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

Please encode " | " as %7C #233

Open
WygorFonseca opened this issue Apr 11, 2024 · 3 comments · May be fixed by #279
Open

Please encode " | " as %7C #233

WygorFonseca opened this issue Apr 11, 2024 · 3 comments · May be fixed by #279

Comments

@WygorFonseca
Copy link

Environment

N/A

Reproduction

According to the code below, there is no encoding for vertical bar ( | ).
https://github.com/unjs/ufo/blob/cda2af90fc4aaaa2133d8b0529570ee8955338a6/src/encoding.ts#L59C1-L71C2

Describe the bug

According to the code below, there is no encoding for vertical bar ( | ).
https://github.com/unjs/ufo/blob/cda2af90fc4aaaa2133d8b0529570ee8955338a6/src/encoding.ts#L59C1-L71C2

Additional context

No response

Logs

No response

@JvanderHeide
Copy link

JvanderHeide commented Jan 22, 2025

Looking at the source it would seem that ufo has done this for a long time already for some reason. As a means to, undo what encodeURI apparently does?

See: https://github.com/unjs/ufo/blame/187afb0f954256eb953c1ef447fccf65e6ef5dff/src/encoding.ts#L34C10-L34C10

Somehow later the replacement of the brackets have been removed but the undoing of the pipe encoding has stayed for whatever reason?

Afaik; the pipe should be encoded as %7c and not decoded to |
I for one am encountering this issue with tomcat - which does not behave kindly when passed a | in the query params. Where as our CMS expects the pipe (encoded) with multiple options for it's API requests.

@pi0 I know it's a long stretch but any shot at remembering (2020 😅) why we're doing it like this?

@JvanderHeide
Copy link

I'd have no issue opening a PR to remove said decoding - but as the tests are seemingly not even testing for anything related to | or %7c - I'm a bit hesitant as to what the effect this could be and I would love to hear thoughts.

@JvanderHeide
Copy link

I think an additional question related to this posed here #240 opens up a valid consideration; should ufo not use encodeURIComponent?

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

Successfully merging a pull request may close this issue.

2 participants