-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
src: fix leading backslash bug in URL #36613
Conversation
Why are the tests skipped? 🤔 |
|
bae7d8a
to
d1106fd
Compare
The associated condition mentioned in the URL parsing algorithm of the WHATWG URL Standard is: url is special and c is U+005C (\) So, `special_back_slash` must be updated whenever `special` is updated. Fixes: nodejs#36559
d92483a
to
20a4a87
Compare
cc @nodejs/url |
Landed in b8c15c7 |
The associated condition mentioned in the URL parsing algorithm of the WHATWG URL Standard is: url is special and c is U+005C (\) So, `special_back_slash` must be updated whenever `special` is updated. Fixes: #36559 PR-URL: #36613 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The associated condition mentioned in the URL parsing algorithm of the WHATWG URL Standard is: url is special and c is U+005C (\) So, `special_back_slash` must be updated whenever `special` is updated. Fixes: #36559 PR-URL: #36613 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The associated condition mentioned in the URL parsing algorithm of the WHATWG URL Standard is: url is special and c is U+005C (\) So, `special_back_slash` must be updated whenever `special` is updated. Fixes: #36559 PR-URL: #36613 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Shouldn't this PR be |
It's only breaking if code is relying on the old incorrect behaviour. |
EXPECT_EQ(simple.protocol(), "http:"); | ||
EXPECT_EQ(simple.host(), "x"); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, we should always add WPTs for bug fixes to the WHATWG URL parser, so that other implementations (e.g., browsers) can similarly benefit. I've upstreamed two of these tests in web-platform-tests/wpt#29271.
/cc @joyeecheung
The associated condition mentioned in the URL parsing algorithm of the
WHATWG URL Standard is:
url is special and c is U+005C (\)
So,
special_back_slash
must be updated wheneverspecial
is updated.Fixes: #36559
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes