Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/fix-redirects-malformed-regex.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sitecore-content-sdk/nextjs': patch
---

[nextjs] Skip malformed redirect regex rules instead of failing the entire redirect chain.
86 changes: 86 additions & 0 deletions packages/nextjs/src/proxy/redirects-proxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,92 @@ describe('RedirectsProxy', () => {
expect(redirectUrl).to.include('/example');
expect(redirectUrl).to.not.include('$1');
});

it('should skip malformed regex and still apply a valid redirect rule', async () => {
const consoleWarnStub = sandbox.stub(console, 'warn');
const req = createRequest({
nextUrl: {
pathname: '/old-page',
locale: 'en',
defaultLocale: 'en',
},
});
const res = createResponse();
const redirectRes = createResponse({
redirected: true,
status: 301,
url: 'http://localhost:3000/new-page',
});
nextRedirectStub.returns(redirectRes);

const { proxy } = createProxy({
redirectMaps: [
{
pattern: '^/broken(',
target: '/should-not-match',
redirectType: REDIRECT_TYPE_301,
},
{
pattern: '/old-page',
target: '/new-page',
redirectType: REDIRECT_TYPE_301,
},
],
});

const finalRes = await proxy.handle(req, res);

expect(consoleWarnStub).to.have.been.calledOnce;
expect(consoleWarnStub.firstCall.args[0]).to.include('Invalid redirect regex');
expect(nextRedirectStub).to.have.been.calledOnce;
const redirectUrl = nextRedirectStub.getCall(0).args[0] as string;
expect(redirectUrl).to.include('/new-page');
expect(finalRes).to.deep.equal(redirectRes);

consoleWarnStub.restore();
});

it('should skip malformed regex with capture groups without failing capture substitution', async () => {
const consoleWarnStub = sandbox.stub(console, 'warn');
const req = createRequest({
nextUrl: {
pathname: '/old-page/123',
locale: 'en',
defaultLocale: 'en',
},
});
const res = createResponse();
const redirectRes = createResponse({
redirected: true,
status: 301,
url: 'http://localhost:3000/new-page/123',
});
nextRedirectStub.returns(redirectRes);

const { proxy } = createProxy({
redirectMaps: [
{
pattern: '^/broken(',
target: '/bad',
redirectType: REDIRECT_TYPE_301,
},
{
pattern: '/old-page/(\\d+)',
target: '/new-page/$1',
redirectType: REDIRECT_TYPE_301,
},
],
});

await proxy.handle(req, res);

expect(consoleWarnStub).to.have.been.calledOnce;
expect(nextRedirectStub).to.have.been.calledOnce;
const redirectUrl = nextRedirectStub.getCall(0).args[0] as string;
expect(redirectUrl).to.include('/new-page/123');

consoleWarnStub.restore();
});
});

it('should replace $siteLang token in target', async () => {
Expand Down
57 changes: 49 additions & 8 deletions packages/nextjs/src/proxy/redirects-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,20 @@ import { FailedProxyExecution, ProxiesContext, SuccessfulProxyExecution } from '
const REGEXP_CONTEXT_SITE_LANG = new RegExp(/\$siteLang/, 'i');
const REGEXP_ABSOLUTE_URL = new RegExp('^(?:[a-z]+:)?//', 'i');

type RedirectResult = RedirectInfo & { matchedQueryString?: string; matchedPath?: string };
/**
* Escape a string for safe use inside a RegExp source (e.g. locale segment).
* @param {string} string - The string to escape
* @returns {string} The escaped string
*/
function escapeRegExp(string: string): string {
return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}

type RedirectResult = RedirectInfo & {
matchedQueryString?: string;
matchedPath?: string;
compiledPattern?: RegExp;
};

/**
* Information about executed proxy to be stored in the context
Expand Down Expand Up @@ -267,9 +280,10 @@ export class RedirectsProxy extends ProxyBase {
// Apply regex replacements to the target URL if the pattern is a regex
const sourcePath = existsRedirect.matchedPath || reqUrl.pathname;
const pathForCaptureMatch = sourcePath.replace(/\/*$/gi, '') || '/';
const matched = pathForCaptureMatch.match(
this.getRedirectPatternRegex(existsRedirect.pattern)
);
const redirectRegex =
existsRedirect.compiledPattern ??
this.safeCompileRedirectPattern(existsRedirect.pattern);
const matched = redirectRegex ? pathForCaptureMatch.match(redirectRegex) : null;
if (matched) {
existsRedirect.target = existsRedirect.target.replace(
/\$(\d+)/g,
Expand Down Expand Up @@ -400,7 +414,7 @@ export class RedirectsProxy extends ProxyBase {
const patternParts = patternPath.split('/');
const maybeLocale = patternParts[1].toLowerCase();
// case insensitive lookup of locales
if (new RegExp(this.locales.join('|'), 'i').test(maybeLocale)) {
if (new RegExp(this.locales.map(escapeRegExp).join('|'), 'i').test(maybeLocale)) {
patternPath = patternPath.replace(`/${patternParts[1]}`, `/${maybeLocale}`);
}

Expand All @@ -415,7 +429,10 @@ export class RedirectsProxy extends ProxyBase {
}

// process regex rules
const regex = this.getRedirectPatternRegex(redirect.pattern);
const regex = this.safeCompileRedirectPattern(redirect.pattern);
if (!regex) {
return false;
}
const testRegex = (value: string) => {
regex.lastIndex = 0;
return regex.test(value);
Expand All @@ -437,6 +454,7 @@ export class RedirectsProxy extends ProxyBase {
// Save the matched query string (if found) into the redirect object
redirect.matchedQueryString = matchedQueryString || '';
redirect.matchedPath = matchedPath || matchedPathWithQuery || '';
redirect.compiledPattern = regex;

return (
!!(matchedPath || matchedQueryString) &&
Expand All @@ -459,7 +477,10 @@ export class RedirectsProxy extends ProxyBase {
locale: string,
currentPath: string
): RedirectResult | undefined {
const nonLocalePath = currentPath.replace(new RegExp(`^\/?${locale}\/`, 'i'), '/');
const nonLocalePath = currentPath.replace(
new RegExp(`^/?${escapeRegExp(locale)}/`, 'i'),
'/'
);
return redirects.length
? redirects.find((redirect: RedirectResult) => {
const patternPath = redirect.pattern.replace(/\/*$/g, '').toLowerCase();
Expand Down Expand Up @@ -616,6 +637,26 @@ export class RedirectsProxy extends ProxyBase {
return redirect;
}

/**
* Compiles a redirect pattern to RegExp; returns null if Sitecore produced a malformed rule
* so one bad entry does not fail the entire redirect chain.
* @param {string} pattern redirect pattern from redirect map
* @returns {RegExp | null} normalized regex instance, or null when invalid
* @private
*/
private safeCompileRedirectPattern(pattern: string): RegExp | null {
try {
return this.getRedirectPatternRegex(pattern);
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
console.warn(
`[RedirectsProxy] Invalid redirect regex; skipping rule. pattern=${pattern} (${message})`
);
debug.redirects('invalid redirect regex; skipping rule: %s (%s)', pattern, message);
return null;
}
}

/**
* Converts a redirect pattern string into a RegExp.
* Supports both JS literal form (`/pattern/i`) and plain regex source (`^/path$`).
Expand Down Expand Up @@ -645,7 +686,7 @@ export class RedirectsProxy extends ProxyBase {
if (!urlLocale) {
return path;
}
const localePrefixRegex = new RegExp(`^/${urlLocale}(?=/|$)`, 'i');
const localePrefixRegex = new RegExp(`^/${escapeRegExp(urlLocale)}(?=/|$)`, 'i');
const strippedPath = path.replace(localePrefixRegex, '') || '/';
return strippedPath.startsWith('/') ? strippedPath : `/${strippedPath}`;
}
Expand Down
Loading