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

fix: stop double encoding query params #459

Merged
merged 1 commit into from
Jul 8, 2023

Conversation

SethFalco
Copy link
Member

@SethFalco SethFalco commented Jul 8, 2023

While working on #458, I noticed something strange with the URLs.

A line break was encoding to %250A, and a space would encode to %2520, which doesn't seem right. Turns out special characters were being double encoded/decoded. This is mostly a waste of resources, but also increases the likelihood of a user running into #437 because it inflates the URL.

It's also why all special characters had an encoding starting with %25. Because they were encoded already, so % is the only possible special character, and then the % got URI encoded to %25.

URLSearchParams already does URI encoding and decoding under the hood in #set and #get. There's no need for us to do it!

Here is a demonstration of the issue:

const char = "<";
const encode = encodeURI(char);

const bad = encodeURI(encode); 
// Result: %253C

const params = new URLSearchParams();
params.set("q", encode); 
const bad2 = params.toString();
// Result: q=%253C

On libretranslate.com, if you insert < in the translation textarea, it currently becomes %253C which is not the expected URI encoded string. It should be %3C!

const query = new URLSearchParams(); 
query.set("q", "<");
const good = query.toString();
// Result: q=%3C

const good2 = query.get("q");
// Result: <

@pierotofy
Copy link
Member

Yep, you're right, looks like we're unnecessarily encoding/decoding here. Good catch!

@pierotofy pierotofy merged commit 57cfdec into LibreTranslate:main Jul 8, 2023
4 checks passed
@SethFalco SethFalco deleted the fix-query-params branch May 14, 2024 19:13
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 this pull request may close these issues.

None yet

2 participants