From a08e9b7274029d24813cd52cadf3e46cd36914bc Mon Sep 17 00:00:00 2001 From: Karim Shehadeh Date: Fri, 7 Jun 2024 06:21:47 -0400 Subject: [PATCH 1/3] Prevent append of trailing slash in cases where path ends with a file extension Canonical URLs specified in the metadata object are being resolved incorrectly when the trailing slash config is set to true. In cases where the path ends with something like `.html` it will append the slash resulting in `.html/` which causes issues for SEO. This will validate that if the path has an extension, no slash will be appended. --- .../next/src/lib/metadata/resolvers/resolve-url.test.ts | 7 +++++++ packages/next/src/lib/metadata/resolvers/resolve-url.ts | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts b/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts index b9e1caf796aa5..4e57ca6f587f9 100644 --- a/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts +++ b/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts @@ -110,6 +110,13 @@ describe('resolveAbsoluteUrlWithPathname', () => { 'https://example.com/foo?bar' ) }) + + it('should not add trailing slash to relative url that ends with a file extension', () => { + expect(resolver('/foo.html')).toBe('https://example.com/foo.html') + expect(resolver(new URL('/foo.html', metadataBase))).toBe( + 'https://example.com/foo.html' + ) + }) }) }) diff --git a/packages/next/src/lib/metadata/resolvers/resolve-url.ts b/packages/next/src/lib/metadata/resolvers/resolve-url.ts index 89edce00fa6db..e4056709549b2 100644 --- a/packages/next/src/lib/metadata/resolvers/resolve-url.ts +++ b/packages/next/src/lib/metadata/resolvers/resolve-url.ts @@ -112,6 +112,8 @@ function resolveAbsoluteUrlWithPathname( let isRelative = resolvedUrl.startsWith('/') let isExternal = false let hasQuery = resolvedUrl.includes('?') + let hasFileExtension = /\.[0-9a-z]+$/i.test(resolvedUrl) + if (!isRelative) { try { const parsedUrl = new URL(resolvedUrl) @@ -121,7 +123,8 @@ function resolveAbsoluteUrlWithPathname( // If it's not a valid URL, treat it as external isExternal = true } - if (!isExternal && !hasQuery) return `${resolvedUrl}/` + if (!hasFileExtension && !isExternal && !hasQuery) + return `${resolvedUrl}/` } } From 54c1781baa50e318ab2add097e4284500d26dd26 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 11 Jun 2024 15:33:55 +0200 Subject: [PATCH 2/3] update trailing slash logic --- .../src/lib/metadata/resolvers/resolve-url.test.ts | 2 +- .../next/src/lib/metadata/resolvers/resolve-url.ts | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts b/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts index 4e57ca6f587f9..31bb85e2aae21 100644 --- a/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts +++ b/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts @@ -111,7 +111,7 @@ describe('resolveAbsoluteUrlWithPathname', () => { ) }) - it('should not add trailing slash to relative url that ends with a file extension', () => { + it('should not add trailing slash to relative url that matches file pattern', () => { expect(resolver('/foo.html')).toBe('https://example.com/foo.html') expect(resolver(new URL('/foo.html', metadataBase))).toBe( 'https://example.com/foo.html' diff --git a/packages/next/src/lib/metadata/resolvers/resolve-url.ts b/packages/next/src/lib/metadata/resolvers/resolve-url.ts index e4056709549b2..0c73df90383d1 100644 --- a/packages/next/src/lib/metadata/resolvers/resolve-url.ts +++ b/packages/next/src/lib/metadata/resolvers/resolve-url.ts @@ -86,6 +86,13 @@ function resolveRelativeUrl(url: string | URL, pathname: string): string | URL { return url } +// The regex is matching logic from packages/next/src/lib/load-custom-routes.ts +const FILE_REGEX = + /^(?:\/((?!\.well-known(?:\/.*)?)(?:[^/]+\/)*[^/]+\.\w+))\/[/#?]?$/i +function isFilePattern(pathname: string): boolean { + return FILE_REGEX.test(pathname) +} + // Resolve `pathname` if `url` is a relative path the compose with `metadataBase`. function resolveAbsoluteUrlWithPathname( url: string | URL, @@ -112,7 +119,8 @@ function resolveAbsoluteUrlWithPathname( let isRelative = resolvedUrl.startsWith('/') let isExternal = false let hasQuery = resolvedUrl.includes('?') - let hasFileExtension = /\.[0-9a-z]+$/i.test(resolvedUrl) + // Do not apply trailing slash for file like urls, aligning with the behavior with `trailingSlash` + let isFileUrl = isFilePattern(resolvedUrl) if (!isRelative) { try { @@ -123,8 +131,7 @@ function resolveAbsoluteUrlWithPathname( // If it's not a valid URL, treat it as external isExternal = true } - if (!hasFileExtension && !isExternal && !hasQuery) - return `${resolvedUrl}/` + if (!isFileUrl && !isExternal && !hasQuery) return `${resolvedUrl}/` } } From 44872db613b91b5acc080256eb564fc51c77787d Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 11 Jun 2024 15:48:19 +0200 Subject: [PATCH 3/3] only check pathname --- .../lib/metadata/resolvers/resolve-url.test.ts | 4 ++++ .../src/lib/metadata/resolvers/resolve-url.ts | 16 +++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts b/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts index 31bb85e2aae21..d70c7bc5dc81a 100644 --- a/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts +++ b/packages/next/src/lib/metadata/resolvers/resolve-url.test.ts @@ -113,6 +113,10 @@ describe('resolveAbsoluteUrlWithPathname', () => { it('should not add trailing slash to relative url that matches file pattern', () => { expect(resolver('/foo.html')).toBe('https://example.com/foo.html') + expect(resolver('/foo.html?q=v')).toBe('https://example.com/foo.html?q=v') + expect(resolver(new URL('/.well-known/bar.jpg', metadataBase))).toBe( + 'https://example.com/.well-known/bar.jpg/' + ) expect(resolver(new URL('/foo.html', metadataBase))).toBe( 'https://example.com/foo.html' ) diff --git a/packages/next/src/lib/metadata/resolvers/resolve-url.ts b/packages/next/src/lib/metadata/resolvers/resolve-url.ts index 0c73df90383d1..6b4a25bebf3e9 100644 --- a/packages/next/src/lib/metadata/resolvers/resolve-url.ts +++ b/packages/next/src/lib/metadata/resolvers/resolve-url.ts @@ -88,7 +88,7 @@ function resolveRelativeUrl(url: string | URL, pathname: string): string | URL { // The regex is matching logic from packages/next/src/lib/load-custom-routes.ts const FILE_REGEX = - /^(?:\/((?!\.well-known(?:\/.*)?)(?:[^/]+\/)*[^/]+\.\w+))\/[/#?]?$/i + /^(?:\/((?!\.well-known(?:\/.*)?)(?:[^/]+\/)*[^/]+\.\w+))(\/?|$)/i function isFilePattern(pathname: string): boolean { return FILE_REGEX.test(pathname) } @@ -117,21 +117,27 @@ function resolveAbsoluteUrlWithPathname( // - Doesn't have query if (trailingSlash && !resolvedUrl.endsWith('/')) { let isRelative = resolvedUrl.startsWith('/') - let isExternal = false let hasQuery = resolvedUrl.includes('?') - // Do not apply trailing slash for file like urls, aligning with the behavior with `trailingSlash` - let isFileUrl = isFilePattern(resolvedUrl) + let isExternal = false + let isFileUrl = false if (!isRelative) { try { const parsedUrl = new URL(resolvedUrl) isExternal = metadataBase != null && parsedUrl.origin !== metadataBase.origin + isFileUrl = isFilePattern(parsedUrl.pathname) } catch { // If it's not a valid URL, treat it as external isExternal = true } - if (!isFileUrl && !isExternal && !hasQuery) return `${resolvedUrl}/` + if ( + // Do not apply trailing slash for file like urls, aligning with the behavior with `trailingSlash` + !isFileUrl && + !isExternal && + !hasQuery + ) + return `${resolvedUrl}/` } }