Merge pull request #2233 from lucas-clemente/connection-close-before-1rtt

don't send application-level errors before completion of the handshake

internal/qerr/quic_error.go

@@ -16,6 +16,10 @@ type QuicError struct {
 
 var _ net.Error = &QuicError{}
 
+// UserCanceledError is used if the application closes the connection
+// before the handshake completes.
+var UserCanceledError = &QuicError{ErrorCode: 0x15a}
+
 // Error creates a new QuicError instance
 func Error(errorCode ErrorCode, errorMessage string) *QuicError {
 	return &QuicError{

session.go

@@ -1282,6 +1282,10 @@ func (s *session) sendPackedPacket(packet *packedPacket) {
 }
 
 func (s *session) sendConnectionClose(quicErr *qerr.QuicError) ([]byte, error) {
+	// don't send application errors in Initial or Handshake packets
+	if quicErr.IsApplicationError() && !s.handshakeComplete {
+		quicErr = qerr.UserCanceledError
+	}
 	var reason string
 	// don't send details of crypto errors
 	if !quicErr.IsCryptoError() {

session_test.go

@@ -427,6 +427,7 @@ var _ = Describe("Session", func() {
 		})
 
 		It("shuts down without error", func() {
+			sess.handshakeComplete = true
 			streamManager.EXPECT().CloseWithError(qerr.ApplicationError(0, ""))
 			expectReplaceWithClosed()
 			cryptoSetup.EXPECT().Close()
@@ -457,6 +458,7 @@ var _ = Describe("Session", func() {
 		})
 
 		It("closes with an error", func() {
+			sess.handshakeComplete = true
 			streamManager.EXPECT().CloseWithError(qerr.ApplicationError(0x1337, "test error"))
 			expectReplaceWithClosed()
 			cryptoSetup.EXPECT().Close()
@@ -472,6 +474,7 @@ var _ = Describe("Session", func() {
 		})
 
 		It("includes the frame type in transport-level close frames", func() {
+			sess.handshakeComplete = true
 			testErr := qerr.ErrorWithFrameType(0x1337, 0x42, "test error")
 			streamManager.EXPECT().CloseWithError(testErr)
 			expectReplaceWithClosed()
@@ -488,6 +491,21 @@ var _ = Describe("Session", func() {
 			Expect(sess.Context().Done()).To(BeClosed())
 		})
 
+		It("doesn't send application-level error before the handshake completes", func() {
+			streamManager.EXPECT().CloseWithError(qerr.ApplicationError(0x1337, "test error"))
+			expectReplaceWithClosed()
+			cryptoSetup.EXPECT().Close()
+			packer.EXPECT().PackConnectionClose(gomock.Any()).DoAndReturn(func(f *wire.ConnectionCloseFrame) (*packedPacket, error) {
+				Expect(f.IsApplicationError).To(BeFalse())
+				Expect(f.ErrorCode).To(BeEquivalentTo(0x15a))
+				Expect(f.ReasonPhrase).To(BeEmpty())
+				return &packedPacket{}, nil
+			})
+			sess.CloseWithError(0x1337, "test error")
+			Eventually(areSessionsRunning).Should(BeFalse())
+			Expect(sess.Context().Done()).To(BeClosed())
+		})
+
 		It("closes the session in order to recreate it", func() {
 			streamManager.EXPECT().CloseWithError(gomock.Any())
 			sessionRunner.EXPECT().Remove(gomock.Any())
@@ -1456,6 +1474,7 @@ var _ = Describe("Session", func() {
 			}()
 			Consistently(sess.Context().Done()).ShouldNot(BeClosed())
 			// make the go routine return
+			sess.handshakeComplete = true
 			expectReplaceWithClosed()
 			cryptoSetup.EXPECT().Close()
 			sess.Close()