From caf616670fd20d528aa69e0131cc17d60f0cc27d Mon Sep 17 00:00:00 2001 From: Jaime Burgos <73321943+SkyZeroZx@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:32:14 -0500 Subject: [PATCH] fix(http): run root interceptors in the terminal request chain Represent withRequestsMadeViaParent() with an internal delegating backend so the interceptor handler can distinguish delegated clients from independent child configurations. --- packages/common/http/src/backend.ts | 12 +- packages/common/http/src/provider.ts | 15 +- packages/common/http/test/provider_spec.ts | 176 +++++++++++++++++- .../common/http/test/transfer_cache_spec.ts | 122 ++++++++++++ 4 files changed, 311 insertions(+), 14 deletions(-) diff --git a/packages/common/http/src/backend.ts b/packages/common/http/src/backend.ts index 65dc160a8c4..92d76a9f57c 100644 --- a/packages/common/http/src/backend.ts +++ b/packages/common/http/src/backend.ts @@ -100,11 +100,15 @@ export class HttpInterceptorHandler implements HttpHandler { handle(initialRequest: HttpRequest): Observable> { if (this.chain === null) { + const parentHandler = this.injector.get(HttpHandler, null, {skipSelf: true}); + const isDelegating = parentHandler !== null && this.backend === parentHandler; + const rootInterceptorFns = this.injector.get( + HTTP_ROOT_INTERCEPTOR_FNS, + [], + isDelegating ? {self: true} : undefined, + ); const dedupedInterceptorFns = Array.from( - new Set([ - ...this.injector.get(HTTP_INTERCEPTOR_FNS), - ...this.injector.get(HTTP_ROOT_INTERCEPTOR_FNS, []), - ]), + new Set([...this.injector.get(HTTP_INTERCEPTOR_FNS), ...rootInterceptorFns]), ); // Note: interceptors are wrapped right-to-left so that final execution order is diff --git a/packages/common/http/src/provider.ts b/packages/common/http/src/provider.ts index dfee321420d..2e4a8c94475 100644 --- a/packages/common/http/src/provider.ts +++ b/packages/common/http/src/provider.ts @@ -101,9 +101,16 @@ export function provideHttpClient( featureKinds.has(HttpFeatureKind.CustomXsrfConfiguration) ) { throw new Error( - ngDevMode - ? `Configuration error: found both withXsrfConfiguration() and withNoXsrfProtection() in the same call to provideHttpClient(), which is a contradiction.` - : '', + `Configuration error: found both withXsrfConfiguration() and withNoXsrfProtection() in the same call to provideHttpClient(), which is a contradiction.`, + ); + } + + if ( + featureKinds.has(HttpFeatureKind.RequestsMadeViaParent) && + featureKinds.has(HttpFeatureKind.Fetch) + ) { + throw new Error( + `Configuration error: withRequestsMadeViaParent() cannot be combined with withFetch() in the same call to provideHttpClient().`, ); } } @@ -260,6 +267,8 @@ export function withJsonpSupport(): HttpFeature { * "bubble up" until either reaching the root level or an `HttpClient` which was not configured with * this option. * + * This feature cannot be combined with `withFetch` in the same `provideHttpClient()` call. + * * @see [HTTP client setup](guide/http/setup#withrequestsmadeviaparent) * @see {@link provideHttpClient} * @publicApi 19.0 diff --git a/packages/common/http/test/provider_spec.ts b/packages/common/http/test/provider_spec.ts index 450c2de6018..b198cbddde5 100644 --- a/packages/common/http/test/provider_spec.ts +++ b/packages/common/http/test/provider_spec.ts @@ -34,8 +34,9 @@ import { } from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {EMPTY, Observable, from} from 'rxjs'; +import {tap} from 'rxjs/operators'; -import {HttpInterceptorFn} from '../src/interceptor'; +import {HTTP_ROOT_INTERCEPTOR_FNS, HttpInterceptorFn} from '../src/interceptor'; import { HttpFeature, HttpFeatureKind, @@ -372,6 +373,12 @@ describe('provideHttpClient', () => { }); describe('withRequestsMadeViaParent()', () => { + it('should error when combined with a backend override', () => { + expect(() => provideHttpClient(withRequestsMadeViaParent(), withFetch())).toThrowError( + 'Configuration error: withRequestsMadeViaParent() cannot be combined with withFetch() in the same call to provideHttpClient().', + ); + }); + for (const backend of ['fetch', 'xhr']) { describe(`given '${backend}' backend`, () => { const commonHttpFeatures: HttpFeature[] = []; @@ -449,18 +456,59 @@ describe('provideHttpClient', () => { req.flush(''); }); - it('should be able to connect to a legacy-provided HttpClient context', () => { + it('should include root interceptors in independent child contexts', () => { TestBed.configureTestingModule({ - imports: [HttpClientTestingModule], - providers: [provideLegacyInterceptor('parent')], + providers: [ + provideHttpClient(...commonHttpFeatures), + { + provide: HTTP_ROOT_INTERCEPTOR_FNS, + useValue: makeLiteralTagInterceptorFn('root'), + multi: true, + }, + provideHttpClientTesting(), + ], + }); + + const child = createEnvironmentInjector( + [ + provideHttpClient(withInterceptors([makeLiteralTagInterceptorFn('child')])), + { + provide: HttpBackend, + useFactory: () => inject(HttpBackend, {skipSelf: true}), + }, + ], + TestBed.inject(EnvironmentInjector), + ); + + child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe(); + const req = TestBed.inject(HttpTestingController).expectOne('/test'); + expect(req.request.headers.get('X-Tag')).toEqual('child,root'); + req.flush(''); + }); + + it('should run root interceptors once in the parent request and response chain', () => { + const order: string[] = []; + + TestBed.configureTestingModule({ + providers: [ + provideHttpClient( + ...commonHttpFeatures, + withInterceptors([makeOrderedTagInterceptorFn('parent', order)]), + ), + { + provide: HTTP_ROOT_INTERCEPTOR_FNS, + useValue: makeOrderedTagInterceptorFn('root', order), + multi: true, + }, + provideHttpClientTesting(), + ], }); const child = createEnvironmentInjector( [ provideHttpClient( - ...commonHttpFeatures, withRequestsMadeViaParent(), - withInterceptors([makeLiteralTagInterceptorFn('child')]), + withInterceptors([makeOrderedTagInterceptorFn('child', order)]), ), ], TestBed.inject(EnvironmentInjector), @@ -468,11 +516,112 @@ describe('provideHttpClient', () => { child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe(); const req = TestBed.inject(HttpTestingController).expectOne('/test'); - expect(req.request.headers.get('X-Tag')).toEqual('child,parent'); + expect(req.request.headers.get('X-Tag')).toEqual('child,parent,root'); + expect(order).toEqual(['child:request', 'parent:request', 'root:request']); req.flush(''); + expect(order).toEqual([ + 'child:request', + 'parent:request', + 'root:request', + 'root:response', + 'parent:response', + 'child:response', + ]); }); }); } + + it('should be able to connect to a legacy-provided HttpClient context', () => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [provideLegacyInterceptor('parent')], + }); + + const child = createEnvironmentInjector( + [ + provideHttpClient( + withRequestsMadeViaParent(), + withInterceptors([makeLiteralTagInterceptorFn('child')]), + ), + ], + TestBed.inject(EnvironmentInjector), + ); + + child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe(); + const req = TestBed.inject(HttpTestingController).expectOne('/test'); + expect(req.request.headers.get('X-Tag')).toEqual('child,parent'); + req.flush(''); + }); + + it('should inherit root interceptors in independent child contexts', () => { + TestBed.configureTestingModule({ + providers: [ + provideHttpClient(), + { + provide: HTTP_ROOT_INTERCEPTOR_FNS, + useValue: makeLiteralTagInterceptorFn('root'), + multi: true, + }, + provideHttpClientTesting(), + ], + }); + + const delegatingParent = createEnvironmentInjector( + [provideHttpClient(withRequestsMadeViaParent())], + TestBed.inject(EnvironmentInjector), + ); + const independentChild = createEnvironmentInjector( + [ + provideHttpClient(withInterceptors([makeLiteralTagInterceptorFn('child')])), + {provide: HttpBackend, useValue: TestBed.inject(HttpBackend)}, + ], + delegatingParent, + ); + + try { + independentChild.get(HttpClient).get('/test', {responseType: 'text'}).subscribe(); + const req = TestBed.inject(HttpTestingController).expectOne('/test'); + expect(req.request.headers.get('X-Tag')).toEqual('child,root'); + req.flush(''); + } finally { + independentChild.destroy(); + delegatingParent.destroy(); + } + }); + + it('should inherit root interceptors when a custom provider overrides delegation', () => { + TestBed.configureTestingModule({ + providers: [ + provideHttpClient(), + { + provide: HTTP_ROOT_INTERCEPTOR_FNS, + useValue: makeLiteralTagInterceptorFn('root'), + multi: true, + }, + provideHttpClientTesting(), + ], + }); + + const child = createEnvironmentInjector( + [ + provideHttpClient( + withRequestsMadeViaParent(), + withInterceptors([makeLiteralTagInterceptorFn('child')]), + ), + {provide: HttpBackend, useValue: TestBed.inject(HttpBackend)}, + ], + TestBed.inject(EnvironmentInjector), + ); + + try { + child.get(HttpClient).get('/test', {responseType: 'text'}).subscribe(); + const req = TestBed.inject(HttpTestingController).expectOne('/test'); + expect(req.request.headers.get('X-Tag')).toEqual('child,root'); + req.flush(''); + } finally { + child.destroy(); + } + }); }); describe('compatibility with Http NgModules', () => { @@ -654,6 +803,19 @@ function makeLiteralTagInterceptorFn(tag: string): HttpInterceptorFn { return (req, next) => next(addTagToRequest(req, tag)); } +function makeOrderedTagInterceptorFn(tag: string, order: string[]): HttpInterceptorFn { + return (req, next) => { + order.push(`${tag}:request`); + return next(addTagToRequest(req, tag)).pipe( + tap((event) => { + if (event instanceof HttpResponse) { + order.push(`${tag}:response`); + } + }), + ); + }; +} + function makeTokenTagInterceptorFn(tag: InjectionToken): HttpInterceptorFn { return (req, next) => next(addTagToRequest(req, inject(tag))); } diff --git a/packages/common/http/test/transfer_cache_spec.ts b/packages/common/http/test/transfer_cache_spec.ts index d8b1fccb91b..1392006fa8c 100644 --- a/packages/common/http/test/transfer_cache_spec.ts +++ b/packages/common/http/test/transfer_cache_spec.ts @@ -10,6 +10,8 @@ import {DOCUMENT} from '../../index'; import { ApplicationRef, Component, + createEnvironmentInjector, + EnvironmentInjector, Injectable, PLATFORM_ID, TransferState, @@ -26,6 +28,8 @@ import { HttpRequest, HttpResponse, provideHttpClient, + withInterceptors, + withRequestsMadeViaParent, } from '../public_api'; import { BODY, @@ -295,6 +299,124 @@ describe('TransferCache', () => { }); }); + describe('withRequestsMadeViaParent()', () => { + let childInjector: EnvironmentInjector; + + beforeEach(() => { + globalThis['ngServerMode'] = true; + + TestBed.configureTestingModule({ + providers: [ + TransferState, + withHttpTransferCache({ + filter: (request) => !request.headers.has('X-API-Key'), + }), + provideHttpClient( + withInterceptors([ + (request, next) => { + if (request.url === '/private') { + request = request.clone({ + setHeaders: {Authorization: 'Bearer server-token'}, + }); + } else if (request.url === '/secret') { + request = request.clone({setHeaders: {'X-API-Key': 'secret-a'}}); + } else if (request.url === '/public-alias') { + request = request.clone({url: '/public'}); + } + + return next(request); + }, + ]), + ), + provideHttpClientTesting(), + ], + }); + + childInjector = createEnvironmentInjector( + [ + provideHttpClient( + withInterceptors([ + (request, next) => + next(request.clone({setHeaders: {'X-Trace-Id': 'feature-request'}})), + ]), + withRequestsMadeViaParent(), + ), + ], + TestBed.inject(EnvironmentInjector), + ); + }); + + afterEach(() => { + childInjector.destroy(); + TestBed.inject(HttpTestingController).verify(); + globalThis['ngServerMode'] = undefined; + }); + + it('should evaluate cache eligibility and keys after parent interceptors', () => { + const httpClient = childInjector.get(HttpClient); + const httpTestingController = TestBed.inject(HttpTestingController); + const transferState = TestBed.inject(TransferState); + + let privateResponse: unknown; + httpClient.get('/private').subscribe((response) => (privateResponse = response)); + const privateRequest = httpTestingController.expectOne('/private'); + expect(privateRequest.request.headers.get('Authorization')).toBe('Bearer server-token'); + expect(privateRequest.request.headers.get('X-Trace-Id')).toBe('feature-request'); + privateRequest.flush({internalSecret: 'server-only'}); + + expect(privateResponse).toEqual({internalSecret: 'server-only'}); + expect(JSON.parse(transferState.toJson())).toEqual({}); + + let publicResponse: unknown; + httpClient.get('/public-alias').subscribe((response) => (publicResponse = response)); + httpTestingController.expectOne('/public').flush({message: 'public'}); + + publicResponse = undefined; + httpClient.get('/public-alias').subscribe((response) => (publicResponse = response)); + httpTestingController.expectNone('/public'); + expect(publicResponse).toEqual({message: 'public'}); + }); + + it('should evaluate cache filters after parent interceptors', () => { + const httpClient = childInjector.get(HttpClient); + const httpTestingController = TestBed.inject(HttpTestingController); + const transferState = TestBed.inject(TransferState); + + let response: unknown; + httpClient.get('/secret').subscribe((value) => (response = value)); + const request = httpTestingController.expectOne('/secret'); + expect(request.request.headers.get('X-API-KEY')).toBe('secret-a'); + request.flush({internalSecret: 'secret-only'}); + + expect(response).toEqual({internalSecret: 'secret-only'}); + expect(JSON.parse(transferState.toJson())).toEqual({}); + }); + + it('should evaluate cache eligibility through multiple parent clients', () => { + const grandchildInjector = createEnvironmentInjector( + [provideHttpClient(withRequestsMadeViaParent())], + childInjector, + ); + + try { + const httpClient = grandchildInjector.get(HttpClient); + const httpTestingController = TestBed.inject(HttpTestingController); + const transferState = TestBed.inject(TransferState); + + let privateResponse: unknown; + httpClient.get('/private').subscribe((response) => (privateResponse = response)); + const privateRequest = httpTestingController.expectOne('/private'); + expect(privateRequest.request.headers.get('Authorization')).toBe('Bearer server-token'); + privateRequest.flush({internalSecret: 'server-only'}); + + expect(privateResponse).toEqual({internalSecret: 'server-only'}); + expect(JSON.parse(transferState.toJson())).toEqual({}); + } finally { + grandchildInjector.destroy(); + } + }); + }); + describe('withHttpTransferCache', () => { let isStable: BehaviorSubject;