Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Sirv does not serve files with space in filename when using express #1507

Closed
konnextv opened this issue Sep 13, 2020 · 2 comments
Closed

Sirv does not serve files with space in filename when using express #1507

konnextv opened this issue Sep 13, 2020 · 2 comments

Comments

@konnextv
Copy link

Describe the bug
Sirv does not serve files with spaces when using express, instead you get a 404. This issue exists with [email protected] or later.

To Reproduce

  • In the current template, exchange polka with express
  • create ./static/foo bar.txt
  • try to access /foo%20bar.txt in the browser

Expected behavior
I know that filenames containing spaces and other special characters are not best practice. However, you run into them anywhere and I would not expect sapper to just show a 404 without any further info.

Severity
I know about it and fixed it myself. However, that took me some time and either

  • the template should be changed so that both polka and express work out of the box
    or
  • there should be a notice about this behaviour that prevents other users from running into the same issue.

Additional context

// server.js
express()
  .use(
    compression({ threshold: 0 }),
    (req, res, next) => {
      // pass sirv a decoded url
      sirv("static", { dev }).call(
        null,
        { ...req, path: decodeURIComponent(req.path) },
        res,
        next
      );
    },
    sirv("static", { dev }),
    sapper.middleware()
  )
@konnextv konnextv changed the title Sirv does not serve files with spaces when using express Sirv does not serve files with space in filename when using express Sep 13, 2020
@benmccann
Copy link
Member

It doesn't sound to me like there's any issue in Sapper: lukeed/sirv#82 (comment)

I don't think we'd change the template since the template uses Polka

@konnextv
Copy link
Author

I understand. However, other people will run into this in the feature because in the template it just says polka() // You can also use Express.

In my opinion there should be a notice about this somewhere but if you think differently that is also fine.

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

No branches or pull requests

2 participants