From d77e400f001d69a9e01709c5762564c6d5e20907 Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Sat, 23 Nov 2019 11:30:20 +0700
Subject: [PATCH] don't send application-level errors before completion of the
 handshake

---
 internal/qerr/quic_error.go |  4 ++++
 session.go                  |  4 ++++
 session_test.go             | 19 +++++++++++++++++++
 3 files changed, 27 insertions(+)

diff --git a/internal/qerr/quic_error.go b/internal/qerr/quic_error.go
index 07148c367..139cf6d9b 100644
--- a/internal/qerr/quic_error.go
+++ b/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{
diff --git a/session.go b/session.go
index f15fd193a..d99e2083e 100644
--- a/session.go
+++ b/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() {
diff --git a/session_test.go b/session_test.go
index 80ff56de4..3c762b971 100644
--- a/session_test.go
+++ b/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()