The 666c197 added an error handling in `SetCloseHandler()` and peer
stops getting `CloseError` when network issue like `write: broken
pipe` happens because the close handle returns the error.
Hence this patch changes to skip network error handling.
### summary
😅 Every time I use `gorilla/websocket`, I need to look at the source
code and find the default size for readBufferSize and writeBufferSize.
I think the default value should be written in the comment.
Signed-off-by: rfyiamcool <rfyiamcool@163.com>
Co-authored-by: Alex Vulaj <ajvulaj@gmail.com>
<!--
For Work In Progress Pull Requests, please use the Draft PR feature,
see https://github.blog/2019-02-14-introducing-draft-pull-requests/ for
further details.
For a timely review/response, please avoid force-pushing additional
commits if your PR already received reviews or comments.
Before submitting a Pull Request, please ensure that you have:
- 📖 Read the Contributing guide:
https://github.com/gorilla/.github/blob/main/CONTRIBUTING.md
- 📖 Read the Code of Conduct:
https://github.com/gorilla/.github/blob/main/CODE_OF_CONDUCT.md
- Provide tests for your changes.
- Use descriptive commit messages.
- Comment your code where appropriate.
- Squash your commits
- Update any related documentation.
- Add gorilla/pull-request-reviewers as a Reviewer
-->
## What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Optimization
- [ ] Documentation Update
- [ ] Go Version Update
- [ ] Dependency Update
## Description
Noticed a change in default close handler from 1.5.0 to 1.5.1
1.5.1 Now returns the error from `WriteControl` with `CloseMessage`
type. However this results in ErrCloseSent returned if the connection
initiated the close handshake. This is normally correct as the
connection should not be able to write after a close handshake is
initiated, except when calling the close handler. in that case nil
should be returned so that `advanceFrame()` can return the actual close
message.
## Related Tickets & Documents
## Added/updated tests?
- [x] Yes
- [ ] No, and this is why: _please replace this line with details on why
tests
have not been included_
- [ ] I need help with writing tests
## Run verifications and test
- [ ] `make verify` is passing (fails with `GO-2023-2186 Incorrect
detection of reserved device names on Windows in path/filepath` that is
unrelated to this change)
- [x] `make test` is passing
Co-authored-by: Corey Daley <cdaley@redhat.com>
<!--
For Work In Progress Pull Requests, please use the Draft PR feature,
see https://github.blog/2019-02-14-introducing-draft-pull-requests/ for
further details.
For a timely review/response, please avoid force-pushing additional
commits if your PR already received reviews or comments.
Before submitting a Pull Request, please ensure that you have:
- 📖 Read the Contributing guide:
https://github.com/gorilla/.github/blob/main/CONTRIBUTING.md
- 📖 Read the Code of Conduct:
https://github.com/gorilla/.github/blob/main/CODE_OF_CONDUCT.md
- Provide tests for your changes.
- Use descriptive commit messages.
- Comment your code where appropriate.
- Squash your commits
- Update any related documentation.
- Add gorilla/pull-request-reviewers as a Reviewer
-->
## What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Optimization
- [ ] Documentation Update
- [ ] Go Version Update
- [ ] Dependency Update
## Description
## Related Tickets & Documents
<!--
For pull requests that relate or close an issue, please include them
below. We like to follow [Github's guidance on linking issues to pull
requests](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue).
For example having the text: "closes #1234" would connect the current
pull
request to issue 1234. And when we merge the pull request, Github will
automatically close the issue.
-->
- Related Issue #
- Closes #
## Added/updated tests?
- [ ] Yes
- [x] No
- [ ] I need help with writing tests
## Run verifications and test
- [ ] `make verify` is passing
- [ ] `make test` is passing
<!--
For Work In Progress Pull Requests, please use the Draft PR feature,
see https://github.blog/2019-02-14-introducing-draft-pull-requests/ for
further details.
For a timely review/response, please avoid force-pushing additional
commits if your PR already received reviews or comments.
Before submitting a Pull Request, please ensure that you have:
- 📖 Read the Contributing guide:
https://github.com/gorilla/.github/blob/main/CONTRIBUTING.md
- 📖 Read the Code of Conduct:
https://github.com/gorilla/.github/blob/main/CODE_OF_CONDUCT.md
- Provide tests for your changes.
- Use descriptive commit messages.
- Comment your code where appropriate.
- Squash your commits
- Update any related documentation.
- Add gorilla/pull-request-reviewers as a Reviewer
-->
## What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Optimization
- [x] Documentation Update
- [ ] Go Version Update
- [ ] Dependency Update
## Description
## Related Tickets & Documents
<!--
For pull requests that relate or close an issue, please include them
below. We like to follow [Github's guidance on linking issues to pull
requests](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue).
For example having the text: "closes #1234" would connect the current
pull
request to issue 1234. And when we merge the pull request, Github will
automatically close the issue.
-->
- Related Issue #
- Closes #
## Added/updated tests?
- [ ] Yes
- [x] No
- [ ] I need help with writing tests
## Run verifications and test
- [ ] `make verify` is passing
- [ ] `make test` is passing
Fixes#822
**Summary of Changes**
1. Changed the order of subprotocol selection to prefer client one
---------
Co-authored-by: Corey Daley <cdaley@redhat.com>
<!--
For Work In Progress Pull Requests, please use the Draft PR feature,
see https://github.blog/2019-02-14-introducing-draft-pull-requests/ for
further details.
For a timely review/response, please avoid force-pushing additional
commits if your PR already received reviews or comments.
Before submitting a Pull Request, please ensure that you have:
- 📖 Read the Contributing guide:
https://github.com/gorilla/.github/blob/main/CONTRIBUTING.md
- 📖 Read the Code of Conduct:
https://github.com/gorilla/.github/blob/main/CODE_OF_CONDUCT.md
- Provide tests for your changes.
- Use descriptive commit messages.
- Comment your code where appropriate.
- Squash your commits
- Update any related documentation.
- Add gorilla/pull-request-reviewers as a Reviewer
-->
## What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Optimization
- [ ] Documentation Update
- [ ] Go Version Update
- [ ] Dependency Update
## Description
## Related Tickets & Documents
<!--
For pull requests that relate or close an issue, please include them
below. We like to follow [Github's guidance on linking issues to pull
requests](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue).
For example having the text: "closes #1234" would connect the current
pull
request to issue 1234. And when we merge the pull request, Github will
automatically close the issue.
-->
- Related Issue #
- Closes #
## Added/updated tests?
- [ ] Yes
- [ ] No, and this is why: _please replace this line with details on why
tests
have not been included_
- [ ] I need help with writing tests
## Run verifications and test
- [x] `make verify` is passing
- [x] `make test` is passing
<!--
For Work In Progress Pull Requests, please use the Draft PR feature,
see https://github.blog/2019-02-14-introducing-draft-pull-requests/ for
further details.
For a timely review/response, please avoid force-pushing additional
commits if your PR already received reviews or comments.
Before submitting a Pull Request, please ensure that you have:
- 📖 Read the Contributing guide:
https://github.com/gorilla/.github/blob/main/CONTRIBUTING.md
- 📖 Read the Code of Conduct:
https://github.com/gorilla/.github/blob/main/CODE_OF_CONDUCT.md
- Provide tests for your changes.
- Use descriptive commit messages.
- Comment your code where appropriate.
- Squash your commits
- Update any related documentation.
- Add gorilla/pull-request-reviewers as a Reviewer
-->
## What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Optimization
- [ ] Documentation Update
- [ ] Go Version Update
- [x] Dependency Update
## Description
## Related Tickets & Documents
<!--
For pull requests that relate or close an issue, please include them
below. We like to follow [Github's guidance on linking issues to pull
requests](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue).
For example having the text: "closes #1234" would connect the current
pull
request to issue 1234. And when we merge the pull request, Github will
automatically close the issue.
-->
- Related Issue #
- Closes #
## Added/updated tests?
- [ ] Yes
- [ ] No, and this is why: _please replace this line with details on why
tests
have not been included_
- [ ] I need help with writing tests
## Run verifications and test
- [ ] `make verify` is passing
- [ ] `make test` is passing
**Summary of Changes**
1. Add an example that uses the write buffer pool
The loop process of the websocket connection is inner the http handler
at existing examples， This usage will cause the 8k buffer(4k read buffer
+ 4k write buffer) allocated by net.http can't be GC(Observed by heap
profiling, see picture below) . The purpose of saving memory is not
achieved even if the WriteBufferPool is used.
In example bufferpool, server process websocket connection in a new
goroutine, and the goroutine created by the net.http will exit, then the
8k buffer will be GC.
Co-authored-by: hakunaliu <hakunaliu@tencent.com>
Co-authored-by: Corey Daley <cdaley@redhat.com>
* return an error when Dialer.TLSClientConfig.NextProtos contains a protocol that is not http/1.1
* include the likely cause of the error in the error message
* check for nil-ness of Dialer.TLSClientConfig before attempting to run the check
* addressing the review
* move the NextProtos test into a separate file so that it can be run conditionally on go versions >= 1.14
* moving the new error check into existing http response error block to reduce the possibility of false positives
* wrapping the error in %w
* using %v instead of %w for compatibility with older versions of go
* Revert "using %v instead of %w for compatibility with older versions of go"
This reverts commit d34dd940ee.
* move the unit test back into the existing test code since golang build constraint is no longer necessary
Co-authored-by: Chan Kang <chankang@chankang17@gmail.com>
Fixes issue: https://github.com/gorilla/websocket/issues/745
With the previous interface, NetDial and NetDialContext were used for
both TLS and non-TLS TCP connections, and afterwards TLSClientConfig was
used to do the TLS handshake.
While this API works for most cases, it prevents from using more advance
authentication methods during the TLS handshake, as this is out of the
control of the user.
This commits introduces another a new dial method, NetDialTLSContext,
which is used when dialing for TLS/TCP. The code then assumes that the
handshake is done there and TLSClientConfig is not used.
This API change is fully backwards compatible and it better aligns with
net/http.Transport API, which has these two dial flavors. See:
https://pkg.go.dev/net/http#Transport
Signed-off-by: Lluis Campos <lluis.campos@northern.tech>
- Note that a new maintainer is needed.
- Remove comparison with x/net/websocket. There's no need to describe
the issues with that package now that the package's documentation
points people here and elsewhere.
The values of the `Upgrade` and `Connection` response headers can
contain multiple tokens, for example
Connection: upgrade, keep-alive
The WebSocket RFC describes the checking of these as follows:
2. If the response lacks an |Upgrade| header field or the |Upgrade|
header field contains a value that is not an ASCII case-
insensitive match for the value "websocket", the client MUST
_Fail the WebSocket Connection_.
3. If the response lacks a |Connection| header field or the
|Connection| header field doesn't contain a token that is an
ASCII case-insensitive match for the value "Upgrade", the client
MUST _Fail the WebSocket Connection_.
It is careful to note "contains a value", "contains a token".
Previously, the client would reject with "bad handshake" if the header
doesn't contain exactly the value it looks for.
Change the checks to use `tokenListContainsValue` instead, which is
incidentally what the server is already doing for similar checks.
Using empty struct for signaling is more idiomatic
compared to booleans because users might wonder
what happens on false or true. Empty struct removes
this problem.
There is also a side benefit of occupying less memory
but it should be negligible in this case.
This fix addresses a potential denial-of-service (DoS) vector that can cause an integer overflow in the presence of malicious WebSocket frames.
The fix adds additional checks against the remaining bytes on a connection, as well as a test to prevent regression.
Credit to Max Justicz (https://justi.cz/) for discovering and reporting this, as well as providing a robust PoC and review.
* build: go.mod to go1.12
* bugfix: fix DoS vector caused by readLimit bypass
* test: update TestReadLimit sub-test
* bugfix: payload length 127 should read bytes as uint64
* bugfix: defend against readLength overflows
Kenjiro Nakayama
