-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Remove double brackets from the ws url when using raw IPv6 address #2951
Remove double brackets from the ws url when using raw IPv6 address #2951
Conversation
0ddb128
to
1f2c674
Compare
// Need to remove those as url.format blindly adds its own set of brackets | ||
// if the host string contains colons. That would lead to non-working | ||
// double brackets (e.g. [[::]]) host | ||
host = host.replace(/^\[(.*)\]$/, '$1'); |
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.
Just interesting where we got extra brackets?
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 problem case browser is navigated to http://[::]:8080/
Websocket URL is formed by taking the current location.hostname
which includes the brackets. Then in the end native URL.format
method at least on Chrome V8 adds a new set of braces even they already exists.
Feels that this is a V8 URL.format
issue and gets someday fixed there based on nodejs/node#36654
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.
Will be great to add tests, bug not high priority now
1f2c674
to
6f51d10
Compare
Tested manually as I wasn't able to quickly figure out how to add a new snapshot test.
|
6f51d10
to
edb079a
Compare
Codecov Report
@@ Coverage Diff @@
## master #2951 +/- ##
=======================================
Coverage 92.39% 92.39%
=======================================
Files 37 37
Lines 1262 1263 +1
Branches 326 327 +1
=======================================
+ Hits 1166 1167 +1
Misses 91 91
Partials 5 5
Continue to review full report at Codecov.
|
edb079a
to
cca1c7a
Compare
Test failure btw doesn't seem to relate to my changes. |
Running into same issue on Ubuntu 20:
This PR didn't seem to help and it still prints |
Time to merge and move to new release |
For Bugs and Features; did you add new tests?
TBD
Motivation / Use-Case
Fixes: #2943
Breaking Changes
Additional Info