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

404 if public/ filename contains a space #10004

Closed
rossdakin opened this issue Jan 9, 2020 · 3 comments · Fixed by #10022
Closed

404 if public/ filename contains a space #10004

rossdakin opened this issue Jan 9, 2020 · 3 comments · Fixed by #10022

Comments

@rossdakin
Copy link

Bug report

Describe the bug

Static assets served from public/ result in 404 if they contain a space character.

Also true if the filename contains %20 rather than a space literal.

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. mkdir test
  2. cd test
  3. npm init -y
  4. npm install --save next react react-dom
  5. mkdir pages public
  6. echo "Foo" > public/hello.txt
  7. echo "Foo" > public/hello\ world.txt
  8. npx next

Expected behavior

curl http://localhost:3000/hello.txt → Foo
curl http://localhost:3000/hello\ world.txt → Foo
curl http://localhost:3000/hello%20world.txt → Foo

Actual behavior

curl http://localhost:3000/hello.txt → Foo
curl http://localhost:3000/hello\ world.txt404
curl http://localhost:3000/hello%20world.txt404

Screenshots

Screen Shot 2020-01-09 at 3 47 36 AM

System information

  • OS: macOS 10.15.2
  • Browser: Chrome 79.0.3945.88
  • Version of Next.js: 9.1.7

Additional context

Does't happen when serving from the deprecated static/ directory.

Have only tried with the next local dev server.

rossdakin added a commit to newjersey/career-network that referenced this issue Jan 9, 2020
Only until filenames with spaces in them are supported by the nextjs dev server: vercel/next.js#10004
@Timer
Copy link
Member

Timer commented Jan 9, 2020

Related #9706

kodiakhq bot pushed a commit that referenced this issue Sep 24, 2020
Prior to this pull request, Next.js would immediately decode all URLs sent to its server (via `path-match`).

This was rarely needed, and Next.js would typically re-encode the incoming request right away (see all the `encodeURIComponent`s removed in PR diff). This adds unnecessary performance overhead.

Long term, this will also help prevent weird encoding edge-cases like #10004, #10022, #11371, et al.

---

No new tests are necessary for this change because we've extensively tested these edge cases with existing tests.
One test was updated to reflect that we skip decoding in a 404 scenario.

Let's see if all the existing tests pass!
HitoriSensei pushed a commit to HitoriSensei/next.js that referenced this issue Sep 26, 2020
Prior to this pull request, Next.js would immediately decode all URLs sent to its server (via `path-match`).

This was rarely needed, and Next.js would typically re-encode the incoming request right away (see all the `encodeURIComponent`s removed in PR diff). This adds unnecessary performance overhead.

Long term, this will also help prevent weird encoding edge-cases like vercel#10004, vercel#10022, vercel#11371, et al.

---

No new tests are necessary for this change because we've extensively tested these edge cases with existing tests.
One test was updated to reflect that we skip decoding in a 404 scenario.

Let's see if all the existing tests pass!
@alonre
Copy link

alonre commented Feb 25, 2021

Not sure if we're doing anything wrong, but we're still getting this bug - we have an asset under /public with the @ character in it (image@2x.png) and and when getting request for path/within/public/image%402x.png the server returns 404.
Would appreciate any clues, thanks.

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants