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.

Sign up for GitHub

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

Jump to bottom

404 if static/ filename contains % #11371

Closed
Timer opened this issue Mar 26, 2020 · 1 comment · Fixed by #11373
Closed

404 if static/ filename contains % #11371

Timer opened this issue Mar 26, 2020 · 1 comment · Fixed by #11373
Milestone
9.3.2

Comments

@Timer
Copy link
Member

Timer commented Mar 26, 2020

Bug report

Describe the bug

Static assets served from the static/ folder will 404 if they contain URL-encodable symbols.

Note: This issue is identical to #10004, but pertains to the static/ folder instead of the public/ folder.

To Reproduce

  1. Create a new Next.js Application 
    yarn create next-app test-static
cd test-static
  2. Create a static file with a unique name: 
    mkdir static/
echo "hello world" > static/hello%20copy.txt
  3. Start the development server and issue a request for the file: 
    yarn dev
curl http://localhost:3000/static/hello%2520copy.txt
  4. See a 404 instead of file contents.

Expected behavior

Running the following:

curl http://localhost:3000/static/hello%2520copy.txt

Should respond with the file contents, not a 404.

Screenshots

image

System information

  • OS: macOS
  • Version of Next.js: 9.3.2

Additional Context

Related issue: #9705
Related PR: #10022
@Timer Timer added this to the 9.3.2 milestone Mar 26, 2020
@Timer Timer mentioned this issue Mar 26, 2020
Merged
@Timer Timer mentioned this issue Sep 23, 2020
Merged
kodiakhq bot pushed a commit that referenced this issue Sep 24, 2020
@Timer
Improve server performance by skipping decode/re-encode (#17323)
c351f61 
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
@Timer
Improve server performance by skipping decode/re-encode (vercel#17323)
2c867b2 
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!
@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 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
9.3.2
Development

Successfully merging a pull request may close this issue.

Fix static/ file name encoding Timer/next.js
2 participants
@Timer @balazsorban44