diff --git a/.changeset/chatty-days-lie.md b/.changeset/chatty-days-lie.md new file mode 100644 index 0000000000..aec4f12d03 --- /dev/null +++ b/.changeset/chatty-days-lie.md @@ -0,0 +1,5 @@ +--- +'@sitecore-content-sdk/nextjs': patch +--- + +[nextjs] Short-circuit the proxy chain when a handler returns 403 or a redirect (`redirected` or HTTP 3xx), so upstream redirects (e.g. next-intl locale negotiation) are preserved when composed with `defineProxy` diff --git a/packages/nextjs/src/proxy/proxy.test.ts b/packages/nextjs/src/proxy/proxy.test.ts index 9edf375ce5..7c23e1346d 100644 --- a/packages/nextjs/src/proxy/proxy.test.ts +++ b/packages/nextjs/src/proxy/proxy.test.ts @@ -756,6 +756,98 @@ describe('defineProxy', () => { expect(result).to.equal(forbidden); }); + it('should short-circuit the chain once a proxy sets a location header', async () => { + const redirect = { + status: 307, + headers: new Headers({ location: '/en' }), + } as unknown as NextResponse; + + const nextIntlProxy: ProxyHandler = { + handle: sinon.stub().resolves(redirect), + }; + const localeProxy: ProxyHandler = { + handle: sinon.stub().resolves({ status: 200 } as unknown as NextResponse), + }; + const downstreamProxy: ProxyHandler = { + handle: sinon.stub().resolves({ status: 200 } as unknown as NextResponse), + }; + + const req = {} as NextRequest; + const res = { status: 200, headers: new Headers() } as unknown as NextResponse; + + const result = await defineProxy(nextIntlProxy, localeProxy, downstreamProxy).exec(req, res); + + expect(nextIntlProxy.handle).to.have.been.calledOnce; + expect(localeProxy.handle).to.not.have.been.called; + expect(downstreamProxy.handle).to.not.have.been.called; + expect(result).to.equal(redirect); + expect(result.headers.get('location')).to.equal('/en'); + }); + + it('should short-circuit the chain once a proxy returns a 3xx response without redirected flag', async () => { + const redirect = { + status: 302, + redirected: false, + headers: new Headers({ location: '/target' }), + } as unknown as NextResponse; + + const redirectsProxy: ProxyHandler = { + handle: sinon.stub().resolves(redirect), + }; + const downstreamProxy: ProxyHandler = { + handle: sinon.stub().resolves({ status: 200 } as unknown as NextResponse), + }; + + const req = {} as NextRequest; + const res = { status: 200 } as unknown as NextResponse; + + const result = await defineProxy(redirectsProxy, downstreamProxy).exec(req, res); + + expect(redirectsProxy.handle).to.have.been.calledOnce; + expect(downstreamProxy.handle).to.not.have.been.called; + expect(result).to.equal(redirect); + }); + + it('should short-circuit the chain once a proxy sets redirected on the response', async () => { + const redirect = { + status: 301, + redirected: true, + } as unknown as NextResponse; + + const redirectsProxy: ProxyHandler = { + handle: sinon.stub().resolves(redirect), + }; + const downstreamProxy: ProxyHandler = { + handle: sinon.stub().resolves({ status: 200 } as unknown as NextResponse), + }; + + const req = {} as NextRequest; + const res = { status: 200 } as unknown as NextResponse; + + const result = await defineProxy(redirectsProxy, downstreamProxy).exec(req, res); + + expect(downstreamProxy.handle).to.not.have.been.called; + expect(result).to.equal(redirect); + }); + + it('should preserve redirect responses passed as the initial response', async () => { + const redirect = { + status: 307, + headers: new Headers({ location: '/en' }), + } as unknown as NextResponse; + + const localeProxy: ProxyHandler = { + handle: sinon.stub().resolves({ status: 200 } as unknown as NextResponse), + }; + + const req = {} as NextRequest; + + const result = await defineProxy(localeProxy).exec(req, redirect); + + expect(localeProxy.handle).to.not.have.been.called; + expect(result).to.equal(redirect); + }); + it('should pass context to proxies when generateContext is true', async () => { const proxiesContext: ProxiesContext = new Map(); const successfulExecution: { marker: string } & SuccessfulProxyExecution = { diff --git a/packages/nextjs/src/proxy/proxy.ts b/packages/nextjs/src/proxy/proxy.ts index 090f65712f..ac96b31344 100644 --- a/packages/nextjs/src/proxy/proxy.ts +++ b/packages/nextjs/src/proxy/proxy.ts @@ -283,6 +283,15 @@ export abstract class ProxyBase extends ProxyHandler { } } +/** + * Returns true when the proxy chain should stop (403, redirect, or HTTP 3xx). + * @param {NextResponse} res response from a proxy handler + * @returns {boolean} true when remaining handlers should be skipped + */ +function shouldShortCircuitProxyChain(res: NextResponse): boolean { + return res.status === 403 || !!res.redirected || (res.status >= 300 && res.status <= 399); +} + /** * Define a proxy with a list of proxy handlers * @param {ProxyHandler[]} proxies List of proxy handlers to execute @@ -306,9 +315,9 @@ export const defineProxy = (...proxies: ProxyHandler[]) => { const proxyResponse = await proxies.reduce( (p, proxy) => p.then((res) => { - // Short-circuit the remaining proxies once a previous one - // denied the request (e.g. PreviewProxy returning 403). - if (res.status === 403) return res; + if (shouldShortCircuitProxyChain(res)) { + return res; + } return proxy.handle(req, res, proxiesContext); }),