* 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
Fixes#441.
Issue #441 specified a message separator. This PR has a message terminator. A message terminator can be read immediately following a message. A message separator cannot be read until the start of the next message. The message terminator is more useful when the reader is scanning to the terminator before performing some action.
Sorry for the dumbest PR ever, but this tiny addition of a period changes the formatting of this sentence from a header to a regular paragraph in godoc.
Because the net/http server removes \r\n from multi-line header values,
there's no need to to check for \r or \n when skipping whitespace in
headers (see https://godoc.org/net/textproto#Reader.ReadMIMEHeader).
Given this fact, the whitespace test can be simplified to b == ' ' || b
== '\t'. There's no need for the isSpaceOctet bit field in octetTypes.
The isTokenOctet bit field is the only bit field remaining after the
removal of isSpaceOctet. Simplify the code by replacing the
isTokenOctet bit test in octetTypes with an array of booleans called
isTokenOctet.
Declare isTokenOctet as a composite literal instead of constructing it
at runtime.
Add documentation to core functions for parsing HTTP headers.
Fix bug where connection did not return the write buffer to the pool
after a write error. Add test for the same.
Rename messsageWriter.fatal method to endMessage and consolidate all
message cleanup code there. This ensures that the buffer is returned to
pool on all code paths.
Rename Conn.prepMessage to beginMessage for symmetry with endMessage.
Move some duplicated code at calls to prepMessage to beginMessage.
Bonus improvement: Adjust message and buffer size in TestWriteBufferPool
to test that pool works with fragmented messages.
Add table driven test for handling of host in request URL, request
header and TLS server name. In addition to testing various uses of host
names, this test also confirms that host names are handled the same as
the net/http client.
The new table driven test replaces TestDialTLS, TestDialTLSNoverify,
TestDialTLSBadCert and TestHostHeader.
Eliminate duplicated code for constructing root CA.
Corey Daley