Skip to content

Commit ab45979

Browse files
Yenya030alxhub
authored andcommitted
fix(http): skip TransferCache for cookie-bearing requests by default
Treat requests with a Cookie header like other auth-bearing requests and skip TransferCache caching them by default. This preserves the explicit opt-in path via includeRequestsWithAuthHeaders, adds regression coverage for cookie-bearing requests, and updates the SSR guide to document the behavior.
1 parent 6388675 commit ab45979

3 files changed

Lines changed: 112 additions & 9 deletions

File tree

adev/src/content/guide/ssr.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ To configure this, update your `angular.json` file as follows:
432432
You can customize how Angular caches HTTP responses during server‑side rendering (SSR) and reuses them during hydration by configuring `HttpTransferCacheOptions`.
433433
This configuration is provided globally using `withHttpTransferCacheOptions` inside `provideClientHydration()`.
434434

435-
By default, `HttpClient` caches all `HEAD` and `GET` requests which don't contain `Authorization` or `Proxy-Authorization` headers. You can override those settings by using `withHttpTransferCacheOptions` to the hydration configuration.
435+
By default, `HttpClient` caches all `HEAD` and `GET` requests which don't contain `Authorization`, `Proxy-Authorization`, or `Cookie` headers. You can override those settings by using `withHttpTransferCacheOptions` to the hydration configuration.
436436

437437
```ts
438438
import {bootstrapApplication} from '@angular/platform-browser';
@@ -486,7 +486,7 @@ Use this only when `POST` requests are **idempotent** and safe to reuse between
486486

487487
### `includeRequestsWithAuthHeaders`
488488

489-
Determines whether requests containing `Authorization` or `Proxy‑Authorization` headers are eligible for caching.
489+
Determines whether requests containing `Authorization`, `Proxy‑Authorization`, or `Cookie` headers are eligible for caching.
490490
By default, these are excluded to prevent caching user‑specific responses.
491491

492492
```ts

packages/common/http/src/transfer_cache.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@ import {HttpParams} from './params';
4040
* @param includePostRequests Enables caching for POST requests. By default, only GET and HEAD
4141
* requests are cached. This option can be enabled if POST requests are used to retrieve data
4242
* (for example using GraphQL).
43-
* @param includeRequestsWithAuthHeaders Enables caching of requests containing either `Authorization`
44-
* or `Proxy-Authorization` headers. By default, these requests are excluded from caching.
43+
* @param includeRequestsWithAuthHeaders Enables caching of requests containing `Authorization`,
44+
* `Proxy-Authorization`, or `Cookie` headers. By default, these requests are excluded from
45+
* caching.
4546
*
4647
* @see [Configuring the caching options](guide/ssr#configuring-the-caching-options)
4748
*
@@ -133,7 +134,7 @@ function shouldCacheRequest(req: HttpRequest<unknown>, options: CacheOptions): b
133134
// POST requests are allowed either globally or at request level
134135
(requestMethod === 'POST' && !globalOptions.includePostRequests && !requestOptions) ||
135136
(requestMethod !== 'POST' && !ALLOWED_METHODS.includes(requestMethod)) ||
136-
// Do not cache request that require authorization when includeRequestsWithAuthHeaders is falsey
137+
// Do not cache requests with authentication or cookie headers unless explicitly enabled.
137138
(!globalOptions.includeRequestsWithAuthHeaders && hasAuthHeaders(req)) ||
138139
globalOptions.filter?.(req) === false
139140
) {
@@ -288,9 +289,13 @@ export function transferCacheInterceptorFn(
288289
return event$;
289290
}
290291

291-
/** @returns true when the requests contains autorization related headers. */
292+
/** @returns true when the request contains authentication or cookie headers. */
292293
function hasAuthHeaders(req: HttpRequest<unknown>): boolean {
293-
return req.headers.has('authorization') || req.headers.has('proxy-authorization');
294+
return (
295+
req.headers.has('authorization') ||
296+
req.headers.has('proxy-authorization') ||
297+
req.headers.has('cookie')
298+
);
294299
}
295300

296301
function getFilteredHeaders(

packages/common/http/test/transfer_cache_spec.ts

Lines changed: 100 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,19 @@ import {
1717
} from '@angular/core';
1818
import {TestBed} from '@angular/core/testing';
1919
import {useAutoTick, timeout, withBody} from '@angular/private/testing';
20-
import {BehaviorSubject} from 'rxjs';
20+
import {BehaviorSubject, Observable, of} from 'rxjs';
2121

22-
import {HttpClient, HttpResponse, provideHttpClient} from '../public_api';
22+
import {HttpClient, HttpHeaders, HttpRequest, HttpResponse, provideHttpClient} from '../public_api';
2323
import {
2424
BODY,
25+
CACHE_OPTIONS,
2526
HEADERS,
2627
HTTP_TRANSFER_CACHE_ORIGIN_MAP,
2728
RESPONSE_TYPE,
2829
STATUS,
2930
STATUS_TEXT,
3031
REQ_URL,
32+
transferCacheInterceptorFn,
3133
withHttpTransferCache,
3234
} from '../src/transfer_cache';
3335
import {HttpTestingController, provideHttpClientTesting} from '../testing';
@@ -60,6 +62,102 @@ describe('TransferCache', () => {
6062
})
6163
class SomeComponent {}
6264

65+
describe('transferCacheInterceptorFn', () => {
66+
afterEach(() => {
67+
TestBed.resetTestingModule();
68+
});
69+
70+
function configureInterceptor(options: {includeRequestsWithAuthHeaders?: boolean} = {}): void {
71+
TestBed.resetTestingModule();
72+
TestBed.configureTestingModule({
73+
providers: [
74+
TransferState,
75+
{
76+
provide: CACHE_OPTIONS,
77+
useValue: {
78+
isCacheActive: true,
79+
...options,
80+
},
81+
},
82+
],
83+
});
84+
}
85+
86+
function runOnServer<T>(callback: () => T): T {
87+
const previousServerMode = globalThis['ngServerMode'];
88+
globalThis['ngServerMode'] = true;
89+
try {
90+
return callback();
91+
} finally {
92+
globalThis['ngServerMode'] = previousServerMode;
93+
}
94+
}
95+
96+
function runInterceptor(
97+
req: HttpRequest<unknown>,
98+
next: (req: HttpRequest<unknown>) => Observable<HttpResponse<unknown>>,
99+
): HttpResponse<unknown> {
100+
let response!: HttpResponse<unknown>;
101+
TestBed.runInInjectionContext(() => {
102+
transferCacheInterceptorFn(req, next).subscribe((event) => {
103+
if (event instanceof HttpResponse) {
104+
response = event;
105+
}
106+
});
107+
});
108+
return response;
109+
}
110+
111+
it('should not reuse cached responses for Cookie-bearing requests by default', () => {
112+
configureInterceptor();
113+
114+
const firstRequest = new HttpRequest('GET', '/test-cookie', null, {
115+
headers: new HttpHeaders({Cookie: 'session=user-a'}),
116+
});
117+
const secondRequest = new HttpRequest('GET', '/test-cookie', null, {
118+
headers: new HttpHeaders({Cookie: 'session=user-b'}),
119+
});
120+
121+
const firstNext = jasmine
122+
.createSpy('firstNext')
123+
.and.returnValue(of(new HttpResponse({body: 'user-a-secret'})));
124+
const secondNext = jasmine
125+
.createSpy('secondNext')
126+
.and.returnValue(of(new HttpResponse({body: 'user-b-secret'})));
127+
128+
runOnServer(() => {
129+
expect(runInterceptor(firstRequest, firstNext).body).toBe('user-a-secret');
130+
expect(runInterceptor(secondRequest, secondNext).body).toBe('user-b-secret');
131+
});
132+
133+
expect(firstNext).toHaveBeenCalledTimes(1);
134+
expect(secondNext).toHaveBeenCalledTimes(1);
135+
});
136+
137+
it("should preserve opt-in caching for Cookie-bearing requests when 'includeRequestsWithAuthHeaders' is true", () => {
138+
configureInterceptor({includeRequestsWithAuthHeaders: true});
139+
140+
const request = new HttpRequest('GET', '/test-cookie', null, {
141+
headers: new HttpHeaders({Cookie: 'session=user-a'}),
142+
});
143+
144+
const firstNext = jasmine
145+
.createSpy('firstNext')
146+
.and.returnValue(of(new HttpResponse({body: 'user-a-secret'})));
147+
const secondNext = jasmine
148+
.createSpy('secondNext')
149+
.and.returnValue(of(new HttpResponse({body: 'network-should-not-run'})));
150+
151+
runOnServer(() => {
152+
expect(runInterceptor(request, firstNext).body).toBe('user-a-secret');
153+
expect(runInterceptor(request, secondNext).body).toBe('user-a-secret');
154+
});
155+
156+
expect(firstNext).toHaveBeenCalledTimes(1);
157+
expect(secondNext).not.toHaveBeenCalled();
158+
});
159+
});
160+
63161
describe('withHttpTransferCache', () => {
64162
let isStable: BehaviorSubject<boolean>;
65163

0 commit comments

Comments
 (0)