Skip to content

[nextjs] Fix proxy chain clobbering redirects in multi-language sites#457

Open
RobertoArmas wants to merge 2 commits intoSitecore:devfrom
RobertoArmas:bugfix/defineproxy-redirect-short-circuit
Open

[nextjs] Fix proxy chain clobbering redirects in multi-language sites#457
RobertoArmas wants to merge 2 commits intoSitecore:devfrom
RobertoArmas:bugfix/defineproxy-redirect-short-circuit

Conversation

@RobertoArmas
Copy link
Copy Markdown

Description / Motivation

Fixes a bug in multi-language sites where redirects produced by an earlier proxy in the chain were being clobbered by subsequent proxies. defineProxy(...).exec(...) runs registered proxy handlers (locale, multisite, redirects, personalize, etc.) sequentially against a shared NextResponse.

Previously, when one of those handlers issued a redirect by setting the location header on the response, the remaining handlers in the chain still executed and could mutate the response (rewrite path, change or append headers, overwrite cookies), which in some scenarios resulted in the redirect being lost or the user landing on the wrong locale URL.

This PR makes defineProxy short-circuit the chain as soon as the response carries a location header, so once a proxy decides to redirect, downstream handlers are skipped and the redirect is preserved as-is.

The change is scoped to packages/nextjs/src/proxy/proxy.ts and is non-breaking: the chain continues to execute fully whenever no location header is set, which remains the existing default case.

Real-world use case: composing next-intl with the Content SDK proxy

A common multi-language setup is to compose next-intl's proxy / middleware with the Content SDK proxy chain in a single proxy.ts (formerly middleware.ts before Next.js 16).

next-intl performs locale negotiation based on prefix, cookie, and the accept-language header, and on the very first hit it typically produces a redirect (for example, //en, or us.example.com/frca.example.com/fr for domain-based routing). That redirect is expressed by setting the location header on the NextResponse it returns.

With the current SDK, this composition does not work out of the box: the Content SDK proxies registered after NextIntlProxy keep executing on top of the already-redirecting response and clobber the location header.

To work around this, projects today have to fork defineProxy locally just to add a location-header short-circuit — exactly the change this PR merges upstream.

A real consumer's proxy.ts in this state currently looks like this:

import sites from '.sitecore/sites.json';
import {
  AppRouterMultisiteProxy,
  LocaleProxy,
  PersonalizeProxy,
  ProxyBase,
  RedirectsProxy,
} from '@sitecore-content-sdk/nextjs/proxy';
import { NextIntlProxy } from 'lib/proxies/next-intl-proxy';
import { NextResponse, type NextRequest } from 'next/server';
import scConfig from 'sitecore.config';
import { routing } from './i18n/routing';

// ⚠️ Local fork of `defineProxy` purely to short-circuit when a previous
// proxy has already produced a redirect (e.g. next-intl's `/` -> `/en`).
// This becomes unnecessary once the upstream fix in `defineProxy` ships.
export const customDefineProxy = (...proxies: ProxyBase[]) => {
  return {
    exec: async (req: NextRequest, res?: NextResponse) => {
      const response = res || NextResponse.next();
      const proxyResponse = await proxies.reduce(
        (p, proxy) =>
          p.then((res) => {
            if (res.headers?.get('location')) return res; // 👈 the missing piece
            return proxy.handle(req, res);
          }),
        Promise.resolve(response)
      );
      return proxyResponse;
    },
  };
};

const nextIntlProxy = new NextIntlProxy({ routing, sites, skip: () => false });

const locale = new LocaleProxy({
  sites,
  locales: routing.locales.slice(),
  skip: () => false,
});

const multisite = new AppRouterMultisiteProxy({
  sites,
  ...scConfig.api.edge,
  ...scConfig.multisite,
  skip: () => false,
});

const redirects = new RedirectsProxy({
  sites,
  ...scConfig.api.edge,
  ...scConfig.redirects,
  skip: () => false,
});

const personalize = new PersonalizeProxy({
  sites,
  ...scConfig.api.edge,
  ...scConfig.personalize,
  skip: () => false,
});


export function proxy(req: NextRequest) {
  return customDefineProxy(
    nextIntlProxy,
    locale,
    multisite,
    redirects,
    personalize
  ).exec(req);
}

export const config = {
  matcher: [
    '/',
    '/((?!api/|\\.well-known/|sitemap|robots|llms|_next/|healthz|sitecore/api/|-/|favicon.ico|sc_logo.svg).*)',
  ],
};

Before this fix: when the user requests /, NextIntlProxy returns a 307 Location: /en. Without the short-circuit, LocaleProxy, AppRouterMultisiteProxy, RedirectsProxy, PersonalizeProxy and CSPProxy still run on top of that response, mutate it (rewrite URL, set cookies, set headers) and the location header gets effectively lost — the browser ends up not being redirected to the localized URL on the first hit, and locale negotiation appears broken.

After this fix: defineProxy itself stops the chain as soon as a location header is present on the response, so the next-intl redirect reaches the browser intact. Consumers can drop customDefineProxy and use defineProxy from @sitecore-content-sdk/nextjs/proxy directly:

import { defineProxy } from '@sitecore-content-sdk/nextjs/proxy';
export function proxy(req: NextRequest) {
  return defineProxy(
    nextIntlProxy,
    locale,
    multisite,
    redirects,
    personalize
  ).exec(req);
}

The same situation applies to any composition where an upstream step (auth guard, tenant resolver, custom locale logic, or the SDK's own RedirectsProxy) issues a redirect before later proxies in the chain run.

Testing Details

  • Unit Test Added
    • New test in packages/nextjs/src/proxy/proxy.test.ts:
      defineProxy > should stop executing proxies when a previous proxy sets location header
      — verifies that handlers registered after a redirecting handler are not
      invoked and that the location header on the resulting response is
      preserved.
    • Existing defineProxy tests (chain order, empty response, Next.js 16 style
      exec(req) without NextFetchEvent) continue to pass, confirming the
      non-redirect path is unchanged.
  • Manual Test/Other
    • Reproduced on a multi-language Next.js site using a proxy chain where the
      redirects-proxy issues a 301/302 to a localized URL; before the fix the
      redirect was being overwritten by the following proxy, after the fix the
      browser receives the intended redirect.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant