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

Quotes in the URL are not percent encoded #46850

Closed
asamuzaK opened this issue Feb 26, 2023 · 5 comments · Fixed by #46853
Closed

Quotes in the URL are not percent encoded #46850

asamuzaK opened this issue Feb 26, 2023 · 5 comments · Fixed by #46853
Assignees
Labels
confirmed-bug Issues with confirmed bugs. whatwg-url Issues and PRs related to the WHATWG URL implementation.

Comments

@asamuzaK
Copy link

asamuzaK commented Feb 26, 2023

Version

19.7.0

Platform

all

Subsystem

No response

What steps will reproduce the bug?

When parsing a url containing double quotes with new URL(), the quotes are not percent encoded.

Test code:

const { href } = new URL('https://example.com/"quoted"');
console.log(href);

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

Expected:

const { href } = new URL('https://example.com/"quoted"');
console.log(href);
// => https://example.com/%22quoted%22

What do you see instead?

Actual:

const { href } = new URL('https://example.com/"quoted"');
console.log(href);
// => https://example.com/"quoted"

Additional information

Last good: Node.js v19.6.1
First bad: Node.js v19.7.0

Seem to be regressed by #46410

@panva
Copy link
Member

panva commented Feb 26, 2023

cc @anonrig

@anonrig
Copy link
Member

anonrig commented Feb 26, 2023

cc @nodejs/url

@anonrig anonrig self-assigned this Feb 26, 2023
@lemire
Copy link
Member

lemire commented Feb 26, 2023

Looks like a bug in ada.

ada-url/ada#242

@anonrig anonrig added whatwg-url Issues and PRs related to the WHATWG URL implementation. confirmed-bug Issues with confirmed bugs. labels Feb 26, 2023
@anonrig anonrig assigned lemire and unassigned anonrig Feb 26, 2023
@anonrig
Copy link
Member

anonrig commented Feb 26, 2023

It seems like web platform tests do not cover this edge case...

@lemire
Copy link
Member

lemire commented Feb 26, 2023

We have a patch release in ada (v1.0.4). Upgrading ada in node should fix the issue.

nodejs-github-bot added a commit that referenced this issue Feb 28, 2023
PR-URL: #46853
Fixes: #46850
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
targos pushed a commit that referenced this issue Mar 13, 2023
PR-URL: #46853
Fixes: #46850
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
targos pushed a commit that referenced this issue Mar 14, 2023
PR-URL: #46853
Fixes: #46850
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
anonrig pushed a commit to anonrig/node that referenced this issue Apr 5, 2023
PR-URL: nodejs#46853
Fixes: nodejs#46850
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
anonrig pushed a commit to anonrig/node that referenced this issue Apr 5, 2023
PR-URL: nodejs#46853
Fixes: nodejs#46850
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
anonrig pushed a commit to anonrig/node that referenced this issue Apr 11, 2023
PR-URL: nodejs#46853
Fixes: nodejs#46850
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
anonrig pushed a commit to anonrig/node that referenced this issue Apr 11, 2023
PR-URL: nodejs#46853
Fixes: nodejs#46850
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
danielleadams pushed a commit that referenced this issue Apr 12, 2023
PR-URL: #46853
Backport-PR-URL: #47435
Fixes: #46850
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants