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

Change url on a page that has encoded query parametr causes page reload and internal error #50055

Closed
1 task done
HamAndRock opened this issue May 19, 2023 · 8 comments
Closed
1 task done
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template. locked Navigation Related to Next.js linking (e.g., <Link>) and navigation.

Comments

@HamAndRock
Copy link

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #22 SMP Tue Jan 10 18:39:00 UTC 2023
    Binaries:
      Node: 18.14.2
      npm: 9.5.0
      Yarn: 1.22.19
      pnpm: N/A
    Relevant packages:
      next: 13.4.3
      eslint-config-next: 13.4.1
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.0.4

Which area(s) of Next.js are affected? (leave empty if unsure)

App directory (appDir: true), Routing (next/router, next/navigation, next/link)

Link to the code that reproduces this issue or a replay of the bug

https://codesandbox.io/p/sandbox/awesome-ishizaka-smm5t

To Reproduce

Go directly to https://smm5t5-3000.csb.app/?test=%C5%99e%C5%99icha%2022 and click on the CTA.

This will cause:

Failed to fetch RSC payload. Falling back to browser navigation. TypeError: Failed to execute 'fetch' on 'Window': Failed to read the 'headers' property from 'RequestInit': String contains non ISO-8859-1 code point.

This is caused because the query string contains non valid chars for headers, however in my code as you can see I am encoding that, so this should not be happening.

 const onClick = () => {
    const query = "řeřicha 22";
    router.push(`/?test=${encodeURIComponent(query)}`);
  };

If you land just on https://smm5t5-3000.csb.app/ and hit the CTA it works just fine.

Describe the Bug

When landing on a URL with already encoded query string and then navigation to diffrent query with again non valid query string that needs decoding, it seems to throw an error.

Expected Behavior

Page should not completly reload and should accept my encodeUriComponent call.

Which browser are you using? (if relevant)

Chrome

How are you deploying your application? (if relevant)

self, codespaces

@HamAndRock HamAndRock added the bug Issue was opened via the bug report template. label May 19, 2023
@github-actions github-actions bot added area: app App directory (appDir: true) Navigation Related to Next.js linking (e.g., <Link>) and navigation. labels May 19, 2023
@HamAndRock
Copy link
Author

HamAndRock commented May 19, 2023

I was able to fix this behaviour by patching the header value internally in next with a normalize() call to strip non valid chars

@HamAndRock
Copy link
Author

@timneutkens Hey, again sorry to bother you but this is breaking our production app right now without using patched next.js version. I assume more people using non-english characters in urls are having same issues and this is a big deal breaker for using the app directory.

@timneutkens
Copy link
Member

timneutkens commented May 24, 2023

Please don't ping me on every issue you're running into. We have hundreds of issues to get back to (see screenshot) with 50+ per day being added, not even including GitHub Discussions.

CleanShot 2023-05-24 at 21 19 46@2x

@HamAndRock
Copy link
Author

Hey sorry about that, I ran into this because we build non-english website and this was a blocker for us and it's not something we can solve with workaround since this is Next internal issue. I will limit my pings, sorry about that I understand you have a lot on your plate. Have a great rest of your day.

@HamAndRock
Copy link
Author

I assume this fixes the issue: #51017, will verify later with canary

@timneutkens
Copy link
Member

Going to close this issue per the comment above 👍 If it's not solved let me know! Couldn't verify as the sandbox is gone.

@HamAndRock
Copy link
Author

I am out of office, but we updated nextjs recently after this went out and the patch was no longer necessary. As far as I am aware. Thank you and the team for all the work!

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot added the locked label Aug 3, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template. locked Navigation Related to Next.js linking (e.g., <Link>) and navigation.
Projects
None yet
Development

No branches or pull requests

2 participants