Skip to content

Commit 72dea2b

Browse files
authored
Fix URL-encoded @ symbols in versioned docs URLs (#57594)
1 parent 98e3dbe commit 72dea2b

File tree

3 files changed

+102
-0
lines changed

3 files changed

+102
-0
lines changed

src/frame/middleware/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ import { MAX_REQUEST_TIMEOUT } from '@/frame/lib/constants'
6666
import { initLoggerContext } from '@/observability/logger/lib/logger-context'
6767
import { getAutomaticRequestLogger } from '@/observability/logger/middleware/get-automatic-request-logger'
6868
import appRouterGateway from './app-router-gateway'
69+
import urlDecode from './url-decode'
6970

7071
const { NODE_ENV } = process.env
7172
const isTest = NODE_ENV === 'test' || process.env.GITHUB_ACTIONS === 'true'
@@ -199,6 +200,7 @@ export default function (app: Express) {
199200
app.set('etag', false) // We will manage our own ETags if desired
200201

201202
// *** Config and context for redirects ***
203+
app.use(urlDecode) // Must come before detectLanguage to decode @ symbols in version segments
202204
app.use(detectLanguage) // Must come before context, breadcrumbs, find-page, handle-errors, homepages
203205
app.use(asyncMiddleware(reloadTree)) // Must come before context
204206
app.use(asyncMiddleware(context)) // Must come before early-access-*, handle-redirects

src/frame/middleware/url-decode.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import type { NextFunction, Response } from 'express'
2+
import type { ExtendedRequest } from '@/types'
3+
4+
/**
5+
* Middleware to decode URL-encoded @ symbols.
6+
*
7+
* SharePoint and other systems automatically encode @ symbols to %40,
8+
* which breaks our versioned URLs like /en/enterprise-cloud@latest.
9+
* This middleware decodes @ symbols anywhere in the URL.
10+
*/
11+
export default function urlDecode(req: ExtendedRequest, res: Response, next: NextFunction) {
12+
const originalUrl = req.url
13+
14+
// Only process URLs that contain %40 (encoded @)
15+
if (!originalUrl.includes('%40')) {
16+
return next()
17+
}
18+
19+
try {
20+
// Decode the entire URL, replacing %40 with @
21+
const decodedUrl = originalUrl.replace(/%40/g, '@')
22+
req.url = decodedUrl
23+
return next()
24+
} catch {
25+
// If decoding fails for any reason, continue with original URL
26+
return next()
27+
}
28+
}

src/frame/tests/url-encoding.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { describe, expect, test } from 'vitest'
2+
import { get } from '@/tests/helpers/e2etest'
3+
4+
describe('URL encoding for version paths', () => {
5+
test('handles URL-encoded @ symbol in enterprise-cloud version', async () => {
6+
// SharePoint encodes @ as %40, so /en/enterprise-cloud@latest becomes /en/enterprise-cloud%40latest
7+
const encodedUrl = '/en/enterprise-cloud%40latest/copilot/concepts/chat'
8+
const res = await get(encodedUrl)
9+
10+
// Should either:
11+
// 1. Work directly (200) - the encoded URL should decode and work
12+
// 2. Redirect (301/302) to the proper decoded URL
13+
// Should NOT return 404
14+
expect([200, 301, 302]).toContain(res.statusCode)
15+
16+
if (res.statusCode === 301 || res.statusCode === 302) {
17+
// If it redirects, it should redirect to the decoded version
18+
expect(res.headers.location).toBe('/en/enterprise-cloud@latest/copilot/concepts/chat')
19+
}
20+
})
21+
22+
test('handles URL-encoded @ symbol in enterprise-server version', async () => {
23+
const encodedUrl =
24+
'/en/enterprise-server%403.17/admin/managing-github-actions-for-your-enterprise'
25+
const res = await get(encodedUrl)
26+
27+
expect([200, 301, 302]).toContain(res.statusCode)
28+
29+
if (res.statusCode === 301 || res.statusCode === 302) {
30+
expect(res.headers.location).toBe(
31+
'/en/enterprise-server@3.17/admin/managing-github-actions-for-your-enterprise',
32+
)
33+
}
34+
})
35+
36+
test('handles URL-encoded @ symbol in second path segment', async () => {
37+
// When no language prefix is present
38+
const encodedUrl = '/enterprise-cloud%40latest/copilot/concepts/chat'
39+
const res = await get(encodedUrl)
40+
41+
// Should redirect to add language prefix and decode
42+
expect([301, 302]).toContain(res.statusCode)
43+
expect(res.headers.location).toBe('/en/enterprise-cloud@latest/copilot/concepts/chat')
44+
})
45+
46+
test('normal @ symbol paths continue to work', async () => {
47+
// Ensure we don't break existing functionality
48+
const normalUrl = '/en/enterprise-cloud@latest/copilot/concepts/chat'
49+
const res = await get(normalUrl)
50+
51+
expect(res.statusCode).toBe(200)
52+
})
53+
54+
test('URL encoding in other parts of URL is preserved', async () => {
55+
// Only @ symbols in version paths should be decoded, other encoding should be preserved
56+
const encodedUrl = '/en/enterprise-cloud@latest/copilot/concepts/some%20page'
57+
const res = await get(encodedUrl)
58+
59+
// This might 404 if the page doesn't exist, but shouldn't break due to encoding
60+
expect(res.statusCode).not.toBe(500)
61+
})
62+
63+
test('Express URL properties are correctly updated after decoding', async () => {
64+
// Test that req.path, req.query, etc. are properly updated when req.url is modified
65+
const encodedUrl = '/en/enterprise-cloud%40latest/copilot/concepts/chat?test=value'
66+
const res = await get(encodedUrl)
67+
68+
// Should work correctly (200 or redirect) - the middleware should properly update
69+
// req.path from '/en/enterprise-cloud%40latest/...' to '/en/enterprise-cloud@latest/...'
70+
expect([200, 301, 302]).toContain(res.statusCode)
71+
})
72+
})

0 commit comments

Comments
 (0)