fix(http): prevent transfer cache key collisions

`makeCacheKey` joined the request fields with `|` before hashing. The url
and the serialized body can contain `|` themselves, so a shifted field
boundary (url `/items/a` + body `b|c` vs url `/items/a|b` + body `c`)
produced the same joined string and the same key, letting two distinct
requests share a transfer cache slot.

Join with `\0` instead, which cannot occur in a valid url or in encoded
params, so the field boundaries cannot be forged by field content.
This commit is contained in:
arshiya tabasum
2026-07-20 15:36:01 +05:30
committed by Pawel Kozlowski
parent 00699f3e66
commit 3192dccaa3
2 changed files with 15 additions and 3 deletions
+4 -1
View File
@@ -421,7 +421,10 @@ function makeCacheKey(
serializedBody = '';
}
const key = [method, responseType, mappedRequestUrl, serializedBody, encodedParams].join('|');
// Joining with `|` lets a shifted field boundary (url `/a` + body `b|c` vs url `/a|b` + body `c`)
// collapse to the same string and thus the same hash. `\0` cannot occur in a valid url or in
// encoded params, so the field boundaries can't be forged by field content.
const key = [method, responseType, mappedRequestUrl, serializedBody, encodedParams].join('\0');
const hash = generateHash(key);
return makeStateKey(hash);
@@ -419,7 +419,7 @@ describe('TransferCache', () => {
const transferState = TestBed.inject(TransferState);
expect(JSON.parse(transferState.toJson()) as Record<string, unknown>).toEqual({
'2da5dfaf112523258ec9c26a0abe9a093b59ed7dbe5f43e4b5ee25a407ac9cf0': {
'd501aa2d57b63a95df74e3b0558782b71b077974e968ed303cd30b27e4b70702': {
[BODY]: 'foo',
[HEADERS]: {},
[STATUS]: 200,
@@ -427,7 +427,7 @@ describe('TransferCache', () => {
[REQ_URL]: '/test-1',
[RESPONSE_TYPE]: 'json',
},
'869485290d9385f3c0a9ba571918c335bbca9e03373bf8260d02f2b7dd335849': {
'ceddc6689dc1f2fc3a0b8c364b6e00a79b99a149f27e84da87cec03d44c150c8': {
[BODY]: 'buzz',
[HEADERS]: {},
[STATUS]: 200,
@@ -764,6 +764,15 @@ describe('TransferCache', () => {
makeRequestAndExpectOne('/test-1', null, {method: 'POST', transferCache: true, body: 'bar'});
});
it('should differentiate POST requests with an ambiguous url/body boundary', () => {
// `/items/a` with body `b|c` and `/items/a|b` with body `c` are different requests, but a
// cache key that concatenates the fields with `|` maps both to the same string. The second
// request must be treated as a cache miss and hit the network.
makeRequestAndExpectOne('/items/a', null, {method: 'POST', transferCache: true, body: 'b|c'});
makeRequestAndExpectNone('/items/a', 'POST', {transferCache: true, body: 'b|c'});
makeRequestAndExpectOne('/items/a|b', null, {method: 'POST', transferCache: true, body: 'c'});
});
it('should cache POST with the differing body in object form', () => {
makeRequestAndExpectOne('/test-1', null, {
method: 'POST',