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

Filter files more flexibly #139

Closed
sapphi-red opened this issue Jul 7, 2022 · 4 comments
Closed

Filter files more flexibly #139

sapphi-red opened this issue Jul 7, 2022 · 4 comments

Comments

@sapphi-red
Copy link

Currently Vite filters request before passing to sirv, depending on whether the file that is going to be served is allowed to be served.

But this caused an issue. (vitejs/vite#8498)
This was because that Vite needs to imitate how sirv maps URL to file but Vite didn't it properly.
Now I understand what was different between Vite and sirv so I'm able to fix this.
But to fix this, it needs to 1. decode URL 2. filter it 3. encode URL. This is not performant.

So I wonder if I could pass an option to filter files.
This way Vite won't need to imitate sirv and Vite won't need to re-encode the URL.
For example:

const assets = sirv('public', {
  filter: absolutePath => {
    return filter(absolutePath, viteConfig.filterOptions)
  }
});

This option could be thought as a more flexible version of opt.dotfiles, dir.


If this is out of scope, would you consider:

  • Adding option to skip decodeURIComponent
  • Allow to pass decoded url (for example: req.decodedUrl)
@lukeed
Copy link
Owner

lukeed commented Jul 7, 2022

This fix went in around/before the time that Vite issue popped up: https://github.com/lukeed/sirv/blob/master/packages/sirv/index.js#L43

In dev mode (what Vite uses last I checked), the URL given to sirv won't ever leave the dir provided ("public" in this case). And in non-dev mode, the internal Cache is only populated with files from dir recursively, so any attempt made to leave dir will result in a 404 anyway.

@sapphi-red
Copy link
Author

Thank you for the quick response!
Maybe you are confusing vitejs/vite#8498 with vitejs/vite#2820? It has the same name but vitejs/vite#8498 is created last month.

The behavior of Vite is:

  1. Vite decodes URL
  2. Vite changes URL (to resolve alias)
  3. Vite filters request passed to sirv by ensureServingAccess
  4. sirv is called with the changed req.url which is decoded (because it was manipulated to resolve alias)

The relevant code is https://github.com/vitejs/vite/blob/5c6cf5a2a6577fb8e8ecc66a0411143af3fed042/packages/vite/src/node/server/middlewares/static.ts#L62-L117.

@lukeed
Copy link
Owner

lukeed commented Jul 7, 2022

I'm really not interested in changing anything about sirv, especially related to decoding & encoding URLs. We spent a lot of time in recent years ensuring that the hand-off works correctly.

Vite is a devserver, ultimately, so a little bit of extra decoding work isn't going to be problematic for anyone, especially considering that a single URL encode/decode takes less than 0.5ms. Combined, all the other filtering overhead that I see in this file is probably computationally (and wall-time) more expensive.

If vite really wants to avoid extra decodes, they can use sirv's same URL parser (@polka/url@next, which (1) only decodes if it needs to and (2) can setup a req._parsedUrl dict that can be abused/reused once the request is handed off to sirv.

// vite side
let info = parse(req);
// changes
req.url = req._parsedUrl.raw = redirected;

This requires that vite have full confidence that they updated all _parsedUrl keys correctly, since it'll be reused as is if req.url === req._parsedUrl.raw.


Again, no changes will be made here. What's here works for sirv, its dependents, and is the result of close communications with vite maintainers directly to ensure the above is all true.

@lukeed lukeed closed this as not planned Won't fix, can't repro, duplicate, stale Jul 7, 2022
@sapphi-red
Copy link
Author

That's a good point. Thank you for your response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants