From a472ac47314d49f943f56f8ef66d49b3520e4882 Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Fri, 8 Nov 2019 13:46:20 +0700
Subject: [PATCH] use application-specific CONNECTION_CLOSE for application
 errors

---
 internal/qerr/quic_error.go      |  6 ++++++
 internal/qerr/quic_error_test.go |  7 ++++++-
 session.go                       |  7 ++++---
 session_test.go                  | 13 +++++++++----
 4 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/internal/qerr/quic_error.go b/internal/qerr/quic_error.go
index c4347afa2..66e8960c5 100644
--- a/internal/qerr/quic_error.go
+++ b/internal/qerr/quic_error.go
@@ -39,6 +39,7 @@ func CryptoError(tlsAlert uint8, errorMessage string) *QuicError {
 	}
 }
 
+// ApplicationError creates a new QuicError instance for an application error
 func ApplicationError(errorCode ErrorCode, errorMessage string) *QuicError {
 	return &QuicError{
 		ErrorCode:          errorCode,
@@ -65,6 +66,11 @@ func (e *QuicError) IsCryptoError() bool {
 	return e.ErrorCode.isCryptoError()
 }
 
+// IsApplicationError says if this error is an application error
+func (e *QuicError) IsApplicationError() bool {
+	return e.isApplicationError
+}
+
 // Temporary says if the error is temporary.
 func (e *QuicError) Temporary() bool {
 	return false
diff --git a/internal/qerr/quic_error_test.go b/internal/qerr/quic_error_test.go
index eeeaac6ce..9c48487e3 100644
--- a/internal/qerr/quic_error_test.go
+++ b/internal/qerr/quic_error_test.go
@@ -11,6 +11,7 @@ var _ = Describe("QUIC Transport Errors", func() {
 	It("has a string representation", func() {
 		err := Error(FlowControlError, "foobar")
 		Expect(err.Timeout()).To(BeFalse())
+		Expect(err.IsApplicationError()).To(BeFalse())
 		Expect(err.Error()).To(Equal("FLOW_CONTROL_ERROR: foobar"))
 	})
 
@@ -38,13 +39,17 @@ var _ = Describe("QUIC Transport Errors", func() {
 
 		It("says if an error is a crypto error", func() {
 			Expect(Error(FlowControlError, "").IsCryptoError()).To(BeFalse())
-			Expect(CryptoError(42, "").IsCryptoError()).To(BeTrue())
+			err := CryptoError(42, "")
+			Expect(err.IsCryptoError()).To(BeTrue())
+			Expect(err.IsApplicationError()).To(BeFalse())
+
 		})
 	})
 
 	Context("application errors", func() {
 		It("has a string representation for errors with a message", func() {
 			err := ApplicationError(0x42, "foobar")
+			Expect(err.IsApplicationError()).To(BeTrue())
 			Expect(err.Error()).To(Equal("Application error 0x42: foobar"))
 		})
 
diff --git a/session.go b/session.go
index 828ee09b9..5353bcecf 100644
--- a/session.go
+++ b/session.go
@@ -1023,7 +1023,7 @@ func (s *session) Close() error {
 }
 
 func (s *session) CloseWithError(code protocol.ApplicationErrorCode, desc string) error {
-	s.closeLocal(qerr.Error(qerr.ErrorCode(code), desc))
+	s.closeLocal(qerr.ApplicationError(qerr.ErrorCode(code), desc))
 	<-s.ctx.Done()
 	return nil
 }
@@ -1250,8 +1250,9 @@ func (s *session) sendConnectionClose(quicErr *qerr.QuicError) ([]byte, error) {
 		reason = quicErr.ErrorMessage
 	}
 	packet, err := s.packer.PackConnectionClose(&wire.ConnectionCloseFrame{
-		ErrorCode:    quicErr.ErrorCode,
-		ReasonPhrase: reason,
+		IsApplicationError: quicErr.IsApplicationError(),
+		ErrorCode:          quicErr.ErrorCode,
+		ReasonPhrase:       reason,
 	})
 	if err != nil {
 		return nil, err
diff --git a/session_test.go b/session_test.go
index 23da44340..ea3e05f25 100644
--- a/session_test.go
+++ b/session_test.go
@@ -449,12 +449,17 @@ var _ = Describe("Session", func() {
 			Expect(sess.Context().Done()).To(BeClosed())
 		})
 
-		It("closes streams with proper error", func() {
+		It("closes with an error", func() {
 			testErr := errors.New("test error")
-			streamManager.EXPECT().CloseWithError(qerr.Error(0x1337, testErr.Error()))
+			streamManager.EXPECT().CloseWithError(qerr.ApplicationError(0x1337, testErr.Error()))
 			expectReplaceWithClosed()
 			cryptoSetup.EXPECT().Close()
-			packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
+			packer.EXPECT().PackConnectionClose(gomock.Any()).DoAndReturn(func(f *wire.ConnectionCloseFrame) (*packedPacket, error) {
+				Expect(f.IsApplicationError).To(BeTrue())
+				Expect(f.ErrorCode).To(BeEquivalentTo(0x1337))
+				Expect(f.ReasonPhrase).To(Equal("test error"))
+				return &packedPacket{}, nil
+			})
 			sess.CloseWithError(0x1337, testErr.Error())
 			Eventually(areSessionsRunning).Should(BeFalse())
 			Expect(sess.Context().Done()).To(BeClosed())
@@ -1203,7 +1208,7 @@ var _ = Describe("Session", func() {
 			defer GinkgoRecover()
 			cryptoSetup.EXPECT().RunHandshake().MaxTimes(1)
 			err := sess.run()
-			Expect(err).To(MatchError(qerr.Error(0x1337, testErr.Error())))
+			Expect(err).To(MatchError(qerr.ApplicationError(0x1337, testErr.Error())))
 			close(done)
 		}()
 		streamManager.EXPECT().CloseWithError(gomock.Any())