Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: return 404 for resources requests outside the base path #5657

Merged
merged 12 commits into from Oct 12, 2023
2 changes: 2 additions & 0 deletions docs/guide/migration.md
Expand Up @@ -58,6 +58,8 @@ Also there are other breaking changes which only affect few users.
- Top level `this` was rewritten to `globalThis` by default when building. This behavior is now removed.
- [[#14231] feat!: add extension to internal virtual modules](https://github.com/vitejs/vite/pull/14231)
- Internal virtual modules' id now has an extension (`.js`).
- [[#5657] fix: return 404 for resources requests outside the base path](https://github.com/vitejs/vite/pull/5657)
- In the past, Vite responded to requests outside the base path without `Accept: text/html`, as if they were requested with the base path. Vite no longer does that and responds with 404 instead.

## Migration from v3

Expand Down
22 changes: 16 additions & 6 deletions packages/vite/src/node/server/middlewares/base.ts
Expand Up @@ -33,10 +33,12 @@ export function baseMiddleware({
})
res.end()
return
} else if (req.headers.accept?.includes('text/html')) {
// non-based page visit
const redirectPath =
withTrailingSlash(url) !== base ? joinUrlSegments(base, url) : base
}

// non-based page visit
const redirectPath =
withTrailingSlash(url) !== base ? joinUrlSegments(base, url) : base
if (req.headers.accept?.includes('text/html')) {
res.writeHead(404, {
'Content-Type': 'text/html',
})
Expand All @@ -45,8 +47,16 @@ export function baseMiddleware({
`did you mean to visit <a href="${redirectPath}">${redirectPath}</a> instead?`,
)
return
} else {
// not found for resources
res.writeHead(404, {
'Content-Type': 'text/plain',
})
res.end(
`The server is configured with a public base URL of ${base} - ` +
`did you mean to visit ${redirectPath} instead?`,
)
return
}

next()
}
}
17 changes: 17 additions & 0 deletions playground/assets/__tests__/assets.spec.ts
Expand Up @@ -8,6 +8,7 @@ import {
getBg,
getColor,
isBuild,
isServe,
listAssets,
notifyRebuildComplete,
page,
Expand Down Expand Up @@ -60,6 +61,22 @@ test('should fallback to index.html when accessing non-existant html file', asyn
expect((await fetchPath('doesnt-exist.html')).status).toBe(200)
})

describe.runIf(isServe)('outside base', () => {
test('should get a 404 with html', async () => {
const res = await fetch(new URL('/baz', viteTestUrl), {
headers: { Accept: 'text/html,*/*' },
})
expect(res.status).toBe(404)
expect(res.headers.get('Content-Type')).toBe('text/html')
})

test('should get a 404 with text', async () => {
const res = await fetch(new URL('/baz', viteTestUrl))
expect(res.status).toBe(404)
expect(res.headers.get('Content-Type')).toBe('text/plain')
})
})

describe('injected scripts', () => {
test('@vite/client', async () => {
const hasClient = await page.$(
Expand Down