Skip to content

fix(http): preserve redirect provenance in HTTP transfer cache #70946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Adyej999 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(http): preserve redirect provenance in HTTP transfer cache #70946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions packages/common/http/src/transfer_cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ export const HEADERS = 'h';
export const STATUS = 's';
export const STATUS_TEXT = 'st';
export const REQ_URL = 'u';
export const RESPONSE_URL = 'ru';
export const REDIRECTED = 'rd';
export const RESPONSE_TYPE = 'rt';

interface TransferHttpResponse {
Expand All @@ -125,8 +127,12 @@ interface TransferHttpResponse {
[STATUS]: number;
/** statusText */
[STATUS_TEXT]: string;
/** url */
/** request url */
[REQ_URL]: string;
/** final response url when it differs from the request url */
[RESPONSE_URL]?: string;
/** whether the response was redirected */
[REDIRECTED]?: boolean;
/** responseType */
[RESPONSE_TYPE]: HttpRequest<unknown>['responseType'];
}
Expand Down Expand Up @@ -239,7 +245,9 @@ export function retrieveStateFromCache(
[HEADERS]: httpHeaders,
[STATUS]: status,
[STATUS_TEXT]: statusText,
[REQ_URL]: url,
[REQ_URL]: requestUrl,
[RESPONSE_URL]: responseUrl,
[REDIRECTED]: redirected,
} = response;
// Request found in cache. Respond using it.
let body: ArrayBuffer | Blob | string | undefined = undecodedBody;
Expand Down Expand Up @@ -271,7 +279,8 @@ export function retrieveStateFromCache(
headers,
status,
statusText,
url,
url: responseUrl ?? requestUrl,
redirected,
});
}

Expand Down Expand Up @@ -311,7 +320,7 @@ export function transferCacheInterceptorFn(
return event$.pipe(
tap((event: HttpEvent<unknown>) => {
if (event instanceof HttpResponse) {
const {headers, body, status, statusText} = event;
const {headers, body, status, statusText, url, redirected} = event;

// Only cache successful HTTP responses that are not non-cacheable.
if (
Expand All @@ -323,6 +332,7 @@ export function transferCacheInterceptorFn(

const {transferCache: requestOptions, responseType} = req;
const headersToInclude = getHeadersToInclude(options, requestOptions);
const responseUrl = url !== null && originMap ? mapRequestOriginUrl(url, originMap) : url;
Comment thread
Adyej999 marked this conversation as resolved.

transferState.set<TransferHttpResponse>(storeKey, {
[BODY]:
Expand All @@ -331,6 +341,10 @@ export function transferCacheInterceptorFn(
[STATUS]: status,
[STATUS_TEXT]: statusText,
[REQ_URL]: requestUrl,
...(responseUrl !== null && responseUrl !== requestUrl
? {[RESPONSE_URL]: responseUrl}
: {}),
...(redirected === true ? {[REDIRECTED]: true} : {}),
[RESPONSE_TYPE]: responseType,
});
}
Expand Down
129 changes: 129 additions & 0 deletions packages/common/http/test/transfer_cache_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ import {
CACHE_OPTIONS,
HEADERS,
HTTP_TRANSFER_CACHE_ORIGIN_MAP,
REDIRECTED,
RESPONSE_TYPE,
RESPONSE_URL,
STATUS,
STATUS_TEXT,
REQ_URL,
Expand Down Expand Up @@ -297,6 +299,133 @@ describe('TransferCache', () => {
expect(firstNext).toHaveBeenCalledTimes(1);
expect(secondNext).toHaveBeenCalledTimes(1);
});

it('should preserve redirect provenance when replaying a transferred response', () => {
configureInterceptor();

const requestUrl = 'https://app.example/api/runtime-config';
const finalUrl = 'https://attacker.example/runtime-config.json';
const attackerBody = {apiBaseUrl: 'https://attacker.example/api'};
const request = new HttpRequest('GET', requestUrl);

const serverNext = jasmine.createSpy('serverNext').and.returnValue(
of(
new HttpResponse({
body: attackerBody,
url: finalUrl,
redirected: true,
}),
),
);

const serverResponse = runOnServer(() => runInterceptor(request, serverNext));

expect(serverResponse.body).toEqual(attackerBody);
expect(serverResponse.url).toBe(finalUrl);
expect(serverResponse.redirected).toBeTrue();
expect(serverNext).toHaveBeenCalledTimes(1);

const clientNext = jasmine.createSpy('clientNext').and.returnValue(
of(
new HttpResponse({
body: {apiBaseUrl: 'https://trusted.example/api'},
url: requestUrl,
redirected: false,
}),
),
);

const cachedResponse = runInterceptor(request, clientNext);

expect(clientNext).not.toHaveBeenCalled();
expect(cachedResponse.body).toEqual(attackerBody);
expect(cachedResponse.url).toBe(finalUrl);
expect(cachedResponse.redirected).toBeTrue();
});

it('should not serialize redirect provenance for non-redirected responses', () => {
configureInterceptor();

const requestUrl = 'https://app.example/api/data';
const request = new HttpRequest('GET', requestUrl);

const serverNext = jasmine.createSpy('serverNext').and.returnValue(
of(
new HttpResponse({
body: {value: 'public-data'},
url: requestUrl,
redirected: false,
}),
),
);

runOnServer(() => runInterceptor(request, serverNext));

const serializedState = JSON.parse(TestBed.inject(TransferState).toJson()) as Record<
string,
Record<string, unknown>
>;
const entries = Object.values(serializedState);

expect(entries.length).toBe(1);
expect(entries[0][REDIRECTED]).toBeUndefined();
expect(entries[0][RESPONSE_URL]).toBeUndefined();
});

it('should map the final response URL for redirects using HTTP_TRANSFER_CACHE_ORIGIN_MAP', () => {
configureInterceptor();

TestBed.configureTestingModule({
providers: [
{
provide: HTTP_TRANSFER_CACHE_ORIGIN_MAP,
useValue: {
'http://internal-domain.com:1234': 'https://external-domain.net:443',
},
},
],
});

const internalRequestUrl = 'http://internal-domain.com:1234/api/runtime-config';
const publicRequestUrl = 'https://external-domain.net:443/api/runtime-config';
const internalFinalUrl = 'http://internal-domain.com:1234/api/runtime-config-v2';
const publicFinalUrl = 'https://external-domain.net:443/api/runtime-config-v2';
const responseBody = {apiBaseUrl: 'https://api.example'};
const serverRequest = new HttpRequest('GET', internalRequestUrl);

const serverNext = jasmine.createSpy('serverNext').and.returnValue(
of(
new HttpResponse({
body: responseBody,
url: internalFinalUrl,
redirected: true,
}),
),
);

const serverResponse = runOnServer(() => runInterceptor(serverRequest, serverNext));

expect(serverResponse.url).toBe(internalFinalUrl);
expect(serverResponse.redirected).toBeTrue();
expect(serverNext).toHaveBeenCalledTimes(1);

const clientRequest = new HttpRequest('GET', publicRequestUrl);
const clientNext = jasmine.createSpy('clientNext').and.returnValue(
of(
new HttpResponse({
body: {apiBaseUrl: 'https://network.example'},
url: publicRequestUrl,
}),
),
);

const cachedResponse = runInterceptor(clientRequest, clientNext);

expect(clientNext).not.toHaveBeenCalled();
expect(cachedResponse.body).toEqual(responseBody);
expect(cachedResponse.url).toBe(publicFinalUrl);
expect(cachedResponse.redirected).toBeTrue();
});
});

describe('withRequestsMadeViaParent()', () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/core/test/bundling/hydration/bundle.golden_symbols.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@
"REACTIVE_LVIEW_CONSUMER_NODE",
"REACTIVE_NODE",
"REACTIVE_TEMPLATE_CONSUMER",
"REDIRECTED",
"REFERENCE_NODE_BODY",
"REFERENCE_NODE_HOST",
"REF_EXTRACTOR_REGEXP",
Expand All @@ -242,6 +243,7 @@
"RENDERER",
"REQ_URL",
"RESPONSE_TYPE",
"RESPONSE_URL",
"RendererFactory2",
"RendererStyleFlags2",
"ResourceParamsStatus",
Expand Down
Loading