Skip to content

Commit

Permalink
fix(common): Update Location to support base href containing origin
Browse files Browse the repository at this point in the history
In case `APP_BASE_HREF` is set including `origin` the further usage of it might cause failure

e.g.
If an app is placed on `https://example.com` and bundles are on `https://cdn-example.com` you have to set `APP_BASE_HREF` up as `https://example.com/` and build the app with `--base-href` as `https://cdn-example.com/` but it does not work because of the bug

Fixes #48175
  • Loading branch information
constantant committed Dec 3, 2022
1 parent 7f36221 commit f7b163c
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 10 deletions.
16 changes: 12 additions & 4 deletions packages/common/src/location/location.ts
Expand Up @@ -57,7 +57,7 @@ export class Location implements OnDestroy {
/** @internal */
_subject: EventEmitter<any> = new EventEmitter();
/** @internal */
_baseHref: string;
_basePath: string;
/** @internal */
_locationStrategy: LocationStrategy;
/** @internal */
Expand All @@ -67,8 +67,8 @@ export class Location implements OnDestroy {

constructor(locationStrategy: LocationStrategy) {
this._locationStrategy = locationStrategy;
const browserBaseHref = this._locationStrategy.getBaseHref();
this._baseHref = stripTrailingSlash(_stripIndexHtml(browserBaseHref));
const baseHref = this._locationStrategy.getBaseHref();
this._basePath = _stripOrigin(stripTrailingSlash(_stripIndexHtml(baseHref)));
this._locationStrategy.onPopState((ev) => {
this._subject.emit({
'url': this.path(true),
Expand Down Expand Up @@ -127,7 +127,7 @@ export class Location implements OnDestroy {
* @returns The normalized URL string.
*/
normalize(url: string): string {
return Location.stripTrailingSlash(_stripBaseHref(this._baseHref, _stripIndexHtml(url)));
return Location.stripTrailingSlash(_stripBaseHref(this._basePath, _stripIndexHtml(url)));
}

/**
Expand Down Expand Up @@ -301,3 +301,11 @@ function _stripBaseHref(baseHref: string, url: string): string {
function _stripIndexHtml(url: string): string {
return url.replace(/\/index.html$/, '');
}

function _stripOrigin(baseHref: string): string {
if (/^(https?:)?\/\//.test(baseHref)) {
const [, pathname] = baseHref.split(/\/\/[^\/]+/);
return pathname;
}
return baseHref;
}
77 changes: 75 additions & 2 deletions packages/common/test/location/location_spec.ts
Expand Up @@ -55,7 +55,7 @@ describe('Location Class', () => {
return new MockPlatformLocation();
}
},
{provide: Location, useClass: Location, deps: [LocationStrategy, PlatformLocation]},
{provide: Location, useClass: Location, deps: [LocationStrategy]},
]
});

Expand Down Expand Up @@ -147,7 +147,7 @@ describe('Location Class', () => {
return new MockPlatformLocation();
}
},
{provide: Location, useClass: Location, deps: [LocationStrategy, PlatformLocation]},
{provide: Location, useClass: Location, deps: [LocationStrategy]},
]
});

Expand Down Expand Up @@ -215,4 +215,77 @@ describe('Location Class', () => {
expect(notificationCount).toBe(1);
});
});

describe('location._basePath should contain only base path', () => {
const basePath = '/en';
const getBaseHref = (origin: string, basePath: string) => origin + basePath + '/';

it('in case protocol is http:', () => {
const origin = 'http://example.com';
const baseHref = getBaseHref(origin, basePath);

TestBed.configureTestingModule({
imports: [CommonModule],
providers: [
{provide: LocationStrategy, useValue: {getBaseHref: () => baseHref, onPopState() {}}},
{provide: Location, useClass: Location, deps: [LocationStrategy]},
]
});

const location = TestBed.inject(Location);

expect((location as any)._basePath).toBe(basePath);
});

it('in case protocol is https:', () => {
const origin = 'https://example.com';
const baseHref = getBaseHref(origin, basePath);

TestBed.configureTestingModule({
imports: [CommonModule],
providers: [
{provide: LocationStrategy, useValue: {getBaseHref: () => baseHref, onPopState() {}}},
{provide: Location, useClass: Location, deps: [LocationStrategy]},
]
});

const location = TestBed.inject(Location);

expect((location as any)._basePath).toBe(basePath);
});

it('in case no protocol', () => {
const origin = '//example.com';
const baseHref = getBaseHref(origin, basePath);

TestBed.configureTestingModule({
imports: [CommonModule],
providers: [
{provide: LocationStrategy, useValue: {getBaseHref: () => baseHref, onPopState() {}}},
{provide: Location, useClass: Location, deps: [LocationStrategy]},
]
});

const location = TestBed.inject(Location);

expect((location as any)._basePath).toBe(basePath);
});

it('in case no origin', () => {
const origin = '';
const baseHref = getBaseHref(origin, basePath);

TestBed.configureTestingModule({
imports: [CommonModule],
providers: [
{provide: LocationStrategy, useValue: {getBaseHref: () => baseHref, onPopState() {}}},
{provide: Location, useClass: Location, deps: [LocationStrategy]},
]
});

const location = TestBed.inject(Location);

expect((location as any)._basePath).toBe(basePath);
});
});
});
8 changes: 4 additions & 4 deletions packages/common/testing/src/location_mock.ts
Expand Up @@ -25,7 +25,7 @@ export class SpyLocation implements Location {
/** @internal */
_subject: EventEmitter<any> = new EventEmitter();
/** @internal */
_baseHref: string = '';
_basePath: string = '';
/** @internal */
_locationStrategy: LocationStrategy = null!;
/** @internal */
Expand All @@ -42,8 +42,8 @@ export class SpyLocation implements Location {
this._history[this._historyIndex].path = url;
}

setBaseHref(url: string) {
this._baseHref = url;
setBasePath(url: string) {
this._basePath = url;
}

path(): string {
Expand Down Expand Up @@ -81,7 +81,7 @@ export class SpyLocation implements Location {
if (url.length > 0 && !url.startsWith('/')) {
url = '/' + url;
}
return this._baseHref + url;
return this._basePath + url;
}

go(path: string, query: string = '', state: any = null) {
Expand Down

0 comments on commit f7b163c

Please sign in to comment.