-
Notifications
You must be signed in to change notification settings - Fork 450
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 ParseHostOrURL and enable expect tests #2772
Conversation
* ParseHostOrURL does not accepts ":xxx:123" anymore * pingpongTest.exp is still disabled
62211b0
to
37495da
Compare
network/wsNetwork.go
Outdated
// https://datatracker.ietf.org/doc/html/rfc1123#section-2 | ||
// first character is relaxed to allow either a letter or a digit | ||
if parsed.Host[0] == ':' && (len(parsed.Host) < 2 || parsed.Host[1] != ':') { | ||
return nil, errors.New("host name starts with colon") |
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.
please define this as a global public error object and use it here.
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.
done
Codecov Report
@@ Coverage Diff @@
## master #2772 +/- ##
==========================================
- Coverage 47.12% 47.10% -0.02%
==========================================
Files 349 349
Lines 56326 56328 +2
==========================================
- Hits 26541 26535 -6
- Misses 26814 26820 +6
- Partials 2971 2973 +2
Continue to review full report at Codecov.
|
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.
looks good to me
Summary
Test Plan
Added new test cases for ParseHostOrURL: disallow ":xxx:123" and allow "::11.22.33.44:123"