mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
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.
This commit is contained in:
committed by
Alex Rickabaugh
parent
184c4797b8
commit
ff02a16749
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user