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.
(cherry picked from commit ff02a16749)
This commit is contained in:
committed by
Alex Rickabaugh
parent
8acc4dba08
commit
b3aa78ab5c
@@ -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