-
Notifications
You must be signed in to change notification settings - Fork 37
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
Minq draft 21 #135
Minq draft 21 #135
Conversation
QUIC implementations.
handshake-layer.go
Outdated
@@ -173,10 +173,12 @@ func (h *HandshakeLayer) sendAlert(err Alert) error { | |||
|
|||
func (h *HandshakeLayer) ReadMessage() (*HandshakeMessage, error) { | |||
var hdr, body []byte | |||
err := WouldBlock | |||
err := error(nil) |
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.
Better: var err error
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.
R+ with nits fixed
conn.go
Outdated
return HkdfExpandLabel(c.state.cryptoParams.Hash, tmpSecret, "exporter", hc, keyLength), nil | ||
} | ||
|
||
func (c *Conn) GetConnectionState() ConnectionState { |
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.
Nit: Think State()
would be more aesthetic
HandshakeState string // string representation of the handshake state. | ||
CipherSuite CipherSuiteParams // cipher suite in use (TLS_RSA_WITH_RC4_128_SHA, ...) | ||
PeerCertificates []*x509.Certificate // certificate chain presented by remote peer TODO([email protected]): implement | ||
NextProto string // Selected ALPN proto |
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.
This is starting to look mighty similar to ConnectionParams
. Is there some way to de-dup?
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.
Probably but I think too hard for this PR. See #136
frame-reader.go
Outdated
@@ -2,8 +2,7 @@ | |||
// This is used for both TLS Records and TLS Handshake Messages | |||
package mint | |||
|
|||
import ( | |||
) | |||
import () |
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.
Nit: You can just delete this line.
No description provided.