Skip to content

feat: apply some middlewares before configureServer hook #20222

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

Conversation

sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Jun 16, 2025

Description

This PR moves four middlewares to be applied before configureServer hook. The four middlewares are:

  • timeMiddleware
    • so that the time taken by middlewares injected by configureServer hook is included in the log
  • rejectInvalidRequestMiddleware
    • so that middlewares injected by configureSerever hook does not have to care whether req.url includes # or not
      • normally middlewares have to take care of that theirselves so this is not important in that term.
    • the main reason is for consistency between normal middlewares and post-injected middlewares
  • corsMiddleware
    • normally midddlewares would expect the CORS settings applied in the options to be applied for the requests handled by the injected middlewares
    • there could be a case that a plugin wasn't expecting CORS headers to be applied
      • in that case, they can opt out by calling res.removeHeader('Access-Control-Allow-Origin')
  • hostValidationMiddleware
    • so that middlewares injected by configureSerever hook does not have to care about DNS rebinding attacks

These checks are currently not the responsibility of Vite, but I think it's better to include them.

@sapphi-red
Copy link
Member Author

/ecosystem-ci run

Copy link

pkg-pr-new bot commented Jun 16, 2025

Open in StackBlitz

npm i https://pkg.pr.new/vite@20222

commit: 6e4220a

@vite-ecosystem-ci
Copy link

📝 Ran ecosystem CI on 6e4220a: Open

suite result latest scheduled
histoire failure failure
react-router failure failure
vike failure failure
vite-plugin-cloudflare failure failure
one failure failure
waku failure failure
vite-rsc failure failure

analogjs, astro, ladle, laravel, marko, qwik, storybook, sveltekit, rakkas, vite-plugin-react, unocss, vite-environment-examples, vuepress, vite-plugin-pwa, vite-plugin-vue, nuxt, vite-setup-catalogue, quasar, vite-plugin-svelte, vitest, vitepress

@sapphi-red sapphi-red marked this pull request as ready for review June 16, 2025 08:12
@sapphi-red sapphi-red added this to the 7.0 milestone Jun 16, 2025
Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

Sounds good to me. I don't think there will be a better chance to apply this chance later on.

@sapphi-red sapphi-red merged commit f5cc4c0 into vitejs:main Jun 17, 2025
20 checks passed
@sapphi-red sapphi-red deleted the feat/apply-some-middlewares-before-configure-server-hook branch June 17, 2025 02:23
@sapphi-red sapphi-red added the p2-nice-to-have Not breaking anything but nice to have (priority) label Jun 17, 2025
sapphi-red added a commit to sapphi-red/vite that referenced this pull request Jun 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change p2-nice-to-have Not breaking anything but nice to have (priority) trigger: preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants