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: normalize html path in transformIndexHtml() #18976

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
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
80 changes: 80 additions & 0 deletions packages/vite/src/node/server/__tests__/transformIndexHtml.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { join } from 'node:path'
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import { normalizePath } from '../../utils'
import { type ViteDevServer, createServer } from '../index'

describe('transformIndexHtml', () => {
const root = join(__dirname, 'fixtures/transformIndexHtml')
let server: ViteDevServer | undefined

let resolveResult: { path: string; filename: string } | undefined

beforeEach(async () => {
server = await createServer({
root,
plugins: [
{
name: 'transformIndexHtml-record-result',
transformIndexHtml: {
order: 'pre',
handler: (html, ctx) => {
expect(html).toBe('')
expect(resolveResult).toBe(undefined)
resolveResult = { path: ctx.path, filename: ctx.filename }
},
},
},
],
})
})
afterEach(async () => {
if (server) {
await server.close()
server = undefined
}
})

async function test(url: string, expPath: string, expFilename: string) {
resolveResult = undefined
await server!.transformIndexHtml(url, '')
expect(resolveResult).toEqual({
path: expPath,
filename: normalizePath(expFilename),
})
}

const p = join(root, 'a', 'b')

it('handles root url', async () => {
await test('/', '/index.html', join(root, 'index.html'))
})

it('handles filename in a subdirectory', async () => {
await test('/a/b/index.html', '/a/b/index.html', join(p, 'index.html'))
})
it('handles no filename in a subdirectory', async () => {
await test('/a/b/', '/a/b/index.html', join(p, 'index.html'))
})
it('handles filename without an extension', async () => {
await test('/a/b/index', '/a/b/index.html', join(p, 'index.html'))
})

it('handles filename other than index.html without extension', async () => {
await test('/a/b/other', '/a/b/other.html', join(p, 'other.html'))
})
it('handles unicode in the path', async () => {
await test(
'/%F0%9F%90%B1/',
'/%F0%9F%90%B1/index.html',
join(root, '🐱', 'index.html'),
)
})

it('handles search parameters', async () => {
await test('/a/b/?c=d', '/a/b/index.html', join(p, 'index.html'))
})

it('handles hash', async () => {
await test('/a/b/#c', '/a/b/index.html', join(p, 'index.html'))
})
})
61 changes: 29 additions & 32 deletions packages/vite/src/node/server/middlewares/htmlFallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,45 +28,42 @@ export function htmlFallbackMiddleware(
return next()
}

const url = cleanUrl(req.url!)
const pathname = decodeURIComponent(url)
const { url, pathname } = normalizeIndexHtmlUrl(req.url!)

// .html files are not handled by serveStaticMiddleware
// so we need to check if the file exists
if (pathname.endsWith('.html')) {
const filePath = path.join(root, pathname)
if (fs.existsSync(filePath)) {
debug?.(`Rewriting ${req.method} ${req.url} to ${url}`)
req.url = url
return next()
}
}
// trailing slash should check for fallback index.html
else if (pathname[pathname.length - 1] === '/') {
const filePath = path.join(root, pathname, 'index.html')
if (fs.existsSync(filePath)) {
const newUrl = url + 'index.html'
debug?.(`Rewriting ${req.method} ${req.url} to ${newUrl}`)
req.url = newUrl
return next()
}
}
// non-trailing slash should check for fallback .html
else {
const filePath = path.join(root, pathname + '.html')
if (fs.existsSync(filePath)) {
const newUrl = url + '.html'
debug?.(`Rewriting ${req.method} ${req.url} to ${newUrl}`)
req.url = newUrl
return next()
}
}

if (spaFallback) {
if (fs.existsSync(path.join(root, pathname))) {
debug?.(`Rewriting ${req.method} ${req.url} to ${url}`)
req.url = url
} else if (spaFallback) {
debug?.(`Rewriting ${req.method} ${req.url} to /index.html`)
req.url = '/index.html'
}

next()
}
}

export function normalizeIndexHtmlUrl(url: string): {
url: string
pathname: string
} {
url = cleanUrl(url)
let pathname = decodeURIComponent(url)

if (pathname.endsWith('.html')) {
return { url, pathname }
}
// trailing slash should check for fallback index.html
else if (pathname[pathname.length - 1] === '/') {
pathname += 'index.html'
url += 'index.html'
return { url, pathname }
}
// non-trailing slash should check for fallback .html
else {
pathname += '.html'
url += '.html'
return { url, pathname }
}
}
24 changes: 12 additions & 12 deletions packages/vite/src/node/server/middlewares/indexHtml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import { isCSSRequest } from '../../plugins/css'
import { getCodeWithSourcemap, injectSourcesContent } from '../sourcemap'
import { cleanUrl, unwrapId, wrapId } from '../../../shared/utils'
import { getNodeAssetAttributes } from '../../assetSource'
import { normalizeIndexHtmlUrl } from './htmlFallback'

interface AssetNode {
start: number
Expand Down Expand Up @@ -86,25 +87,24 @@ export function createDevHtmlTransformFn(
html: string,
originalUrl?: string,
): Promise<string> => {
const normalized = normalizeIndexHtmlUrl(url)
let filename: string
if (normalized.pathname.startsWith(FS_PREFIX)) {
filename = fsPathFromId(normalized.pathname)
} else {
filename = normalizePath(
path.join(server.config.root, normalized.pathname),
)
}
return applyHtmlTransforms(html, transformHooks, {
path: url,
filename: getHtmlFilename(url, server),
path: normalized.url,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we shouldn't normalize path here. We don't know if / is responding with the same HTML file with /index.html when appType: 'custom'.
Instead, I think we should make devHtmlHook function work with path ending with /.

const devHtmlHook: IndexHtmlTransformHook = async (

filename,
server,
originalUrl,
})
}
}

function getHtmlFilename(url: string, server: ViteDevServer) {
if (url.startsWith(FS_PREFIX)) {
return decodeURIComponent(fsPathFromId(url))
} else {
return decodeURIComponent(
normalizePath(path.join(server.config.root, url.slice(1))),
)
}
}

function shouldPreTransform(url: string, config: ResolvedConfig) {
return (
!checkPublicFile(url, config) && (isJSRequest(url) || isCSSRequest(url))
Expand Down
Loading