From 56ad3fe254727defc2272c1c2a7c784e854c38f6 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Thu, 4 Jun 2026 15:11:38 +0200 Subject: [PATCH] fix(http): ensure query parameters are inserted before URL fragments Previously, when making an HTTP request where the URL contained a fragment (`#`) and `HttpParams` were provided, the parameters were appended to the very end of the URL (after the fragment). This resulted in the parameters being treated as part of the fragment rather than query parameters, potentially bypassing server-side logic and validation. This commit updates the URL parsing logic in `HttpRequest` to split the URL by the fragment, correctly inserting the query string before any fragment. --- packages/common/http/src/request.ts | 18 +++++++++++++++--- packages/common/http/test/request_spec.ts | 17 +++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/packages/common/http/src/request.ts b/packages/common/http/src/request.ts index bce1fd1bf0ec..137b7939ce73 100644 --- a/packages/common/http/src/request.ts +++ b/packages/common/http/src/request.ts @@ -408,8 +408,20 @@ export class HttpRequest implements HttpRequestOptions { // No parameters, the visible URL is just the URL given at creation time. this.urlWithParams = url; } else { + // Strip the fragment (e.g. #bypass) from the URL before checking for existing query parameters. + // This ensures that new query parameters are inserted before the fragment, preventing + // situations where parameters are mistakenly appended after the fragment and thus + // only exposed to the frontend, which could bypass security checks. + let urlWithoutFragment = url; + let fragment = ''; + const hashIdx = url.indexOf('#'); + if (hashIdx !== -1) { + fragment = url.substring(hashIdx); + urlWithoutFragment = url.substring(0, hashIdx); + } + // Does the URL already have query parameters? Look for '?'. - const qIdx = url.indexOf('?'); + const qIdx = urlWithoutFragment.indexOf('?'); // There are 3 cases to handle: // 1) No existing parameters -> append '?' followed by params. // 2) '?' exists and is followed by existing query string -> @@ -417,8 +429,8 @@ export class HttpRequest implements HttpRequestOptions { // 3) '?' exists at the end of the url -> append params directly. // This basically amounts to determining the character, if any, with // which to join the URL and parameters. - const sep: string = qIdx === -1 ? '?' : qIdx < url.length - 1 ? '&' : ''; - this.urlWithParams = url + sep + params; + const sep: string = qIdx === -1 ? '?' : qIdx < urlWithoutFragment.length - 1 ? '&' : ''; + this.urlWithParams = urlWithoutFragment + sep + params + fragment; } } } diff --git a/packages/common/http/test/request_spec.ts b/packages/common/http/test/request_spec.ts index 35699f2a6ce3..191f205267f3 100644 --- a/packages/common/http/test/request_spec.ts +++ b/packages/common/http/test/request_spec.ts @@ -286,5 +286,22 @@ describe('HttpRequest', () => { const req = baseReq.clone({setParams: {'test': 'false'}}); expect(req.urlWithParams).toEqual('/test?test=false'); }); + it('appends parameters before a fragment', () => { + const fragmentParams = new HttpParams({fromString: 'auth_token=secret'}); + const req = baseReq.clone({params: fragmentParams, url: '/api/data/123#bypass'}); + expect(req.urlWithParams).toEqual('/api/data/123?auth_token=secret#bypass'); + }); + it('appends parameters before a URL fragment', () => { + const req = baseReq.clone({params, url: '/test#fragment'}); + expect(req.urlWithParams).toEqual('/test?test=true#fragment'); + }); + it('appends parameters before a URL fragment when URL has a query string', () => { + const req = baseReq.clone({params, url: '/test?other=false#fragment'}); + expect(req.urlWithParams).toEqual('/test?other=false&test=true#fragment'); + }); + it('appends parameters before a URL fragment when URL has multiple hash characters', () => { + const req = baseReq.clone({params, url: '/test#frag1#frag2'}); + expect(req.urlWithParams).toEqual('/test?test=true#frag1#frag2'); + }); }); });