From ff02a167495ef411411f0729400faded361cc94f Mon Sep 17 00:00:00 2001 From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com> Date: Sat, 11 Jul 2026 18:52:31 -0500 Subject: [PATCH] fix(http): preserve immutability of materialized clones Prevent lazy HttpHeaders and HttpParams clones from reusing value arrays owned by a materialized source. Append and value-specific delete operations previously mutated those shared arrays, violating the immutable API contract and allowing request metadata to bleed into later requests. Share value arrays until an update mutates a specific header or parameter, then copy only that array. Cover the affected append and delete paths with regression tests that materialize the source first. --- packages/common/http/src/headers.ts | 8 ++++---- packages/common/http/src/params.ts | 10 +++++++--- packages/common/http/test/headers_spec.ts | 9 +++++++++ packages/common/http/test/params_spec.ts | 18 ++++++++++++++++++ 4 files changed, 38 insertions(+), 7 deletions(-) diff --git a/packages/common/http/src/headers.ts b/packages/common/http/src/headers.ts index f1c56b8255f..5aabc1cd245 100644 --- a/packages/common/http/src/headers.ts +++ b/packages/common/http/src/headers.ts @@ -192,10 +192,10 @@ export class HttpHeaders { private copyFrom(other: HttpHeaders) { other.init(); - Array.from(other.headers.keys()).forEach((key) => { - this.headers.set(key, other.headers.get(key)!); + for (const [key, values] of other.headers.entries()) { + this.headers.set(key, values); this.normalizedNames.set(key, other.normalizedNames.get(key)!); - }); + } } private clone(update: Update): HttpHeaders { @@ -218,7 +218,7 @@ export class HttpHeaders { return; } this.maybeSetNormalizedName(update.name, key); - const base = (update.op === 'a' ? this.headers.get(key) : undefined) || []; + const base = update.op === 'a' ? (this.headers.get(key) || []).slice() : []; base.push(...value); this.headers.set(key, base); break; diff --git a/packages/common/http/src/params.ts b/packages/common/http/src/params.ts index b82b6577153..42400a266bb 100644 --- a/packages/common/http/src/params.ts +++ b/packages/common/http/src/params.ts @@ -324,18 +324,22 @@ export class HttpParams { } if (this.cloneFrom !== null) { this.cloneFrom.init(); - this.cloneFrom.keys().forEach((key) => this.map!.set(key, this.cloneFrom!.map!.get(key)!)); + + for (const [key, values] of this.cloneFrom.map!.entries()) { + this.map!.set(key, values); + } + this.updates!.forEach((update) => { switch (update.op) { case 'a': case 's': - const base = (update.op === 'a' ? this.map!.get(update.param) : undefined) || []; + const base = update.op === 'a' ? (this.map!.get(update.param) || []).slice() : []; base.push(valueToString(update.value!)); this.map!.set(update.param, base); break; case 'd': if (update.value !== undefined) { - let base = this.map!.get(update.param) || []; + const base = (this.map!.get(update.param) || []).slice(); const idx = base.indexOf(valueToString(update.value)); if (idx !== -1) { base.splice(idx, 1); diff --git a/packages/common/http/test/headers_spec.ts b/packages/common/http/test/headers_spec.ts index f57e487cd62..4205cc04fcf 100644 --- a/packages/common/http/test/headers_spec.ts +++ b/packages/common/http/test/headers_spec.ts @@ -165,6 +165,15 @@ describe('HttpHeaders', () => { expect(third.getAll('foo')).toEqual(['bar', 'baz']); }); + it('should not mutate a materialized source', () => { + const source = new HttpHeaders().set('foo', 'bar'); + expect(source.getAll('foo')).toEqual(['bar']); + + const clone = source.append('foo', 'baz'); + expect(clone.getAll('foo')).toEqual(['bar', 'baz']); + expect(source.getAll('foo')).toEqual(['bar']); + }); + it('should preserve the case of the first call', () => { const headers = new HttpHeaders(); const second = headers.append('FOO', 'bar'); diff --git a/packages/common/http/test/params_spec.ts b/packages/common/http/test/params_spec.ts index 79989858100..b2e37581928 100644 --- a/packages/common/http/test/params_spec.ts +++ b/packages/common/http/test/params_spec.ts @@ -72,6 +72,15 @@ describe('HttpUrlEncodedParams', () => { expect(mutated.toString()).toEqual('a=b&a=true'); }); + it('should not mutate a materialized source when appending', () => { + const source = new HttpParams().append('a', 'b'); + expect(source.getAll('a')).toEqual(['b']); + + const clone = source.append('a', 'c'); + expect(clone.getAll('a')).toEqual(['b', 'c']); + expect(source.getAll('a')).toEqual(['b']); + }); + it('should allow appending all string parameters', () => { const body = new HttpParams({fromString: 'a=a1&b=b1'}); const mutated = body.appendAll({a: ['a2', 'a3'], b: 'b2'}); @@ -138,6 +147,15 @@ describe('HttpUrlEncodedParams', () => { expect(mutated.getAll('a')).toEqual(['1', '3', '5']); }); + it('should not mutate a materialized source when deleting a value', () => { + const source = new HttpParams().append('a', '1').append('a', '2'); + expect(source.getAll('a')).toEqual(['1', '2']); + + const clone = source.delete('a', '1'); + expect(clone.getAll('a')).toEqual(['2']); + expect(source.getAll('a')).toEqual(['1', '2']); + }); + it('should allow deletion of one value of a number parameter', () => { const body = new HttpParams({fromString: 'a=0&a=1&a=2&a=3&a=4&a=5'}); const mutated = body.delete('a', 0).delete('a', 4);