From 3192dccaa3e1488f8bf351864ea48e44a21246aa Mon Sep 17 00:00:00 2001
From: arshiya tabasum <arshi@bugqore.com>
Date: Mon, 20 Jul 2026 15:36:01 +0530
Subject: [PATCH] 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.
---
 packages/common/http/src/transfer_cache.ts       |  5 ++++-
 packages/common/http/test/transfer_cache_spec.ts | 13 +++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/packages/common/http/src/transfer_cache.ts b/packages/common/http/src/transfer_cache.ts
index b5308632dd0..4c92a84fca1 100644
--- a/packages/common/http/src/transfer_cache.ts
+++ b/packages/common/http/src/transfer_cache.ts
@@ -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);
diff --git a/packages/common/http/test/transfer_cache_spec.ts b/packages/common/http/test/transfer_cache_spec.ts
index f66e7ac3ef6..652665965dd 100644
--- a/packages/common/http/test/transfer_cache_spec.ts
+++ b/packages/common/http/test/transfer_cache_spec.ts
@@ -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',