From 2ac53843ed009aaa046625896bf2937abf276157 Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Sat, 14 May 2016 17:30:36 +0700
Subject: [PATCH] improve error handling in ConnectionParametersManager

---
 handshake/connection_parameters_manager.go    | 16 +++++++------
 .../connection_parameters_manager_test.go     | 23 ++++++++++++++++++-
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/handshake/connection_parameters_manager.go b/handshake/connection_parameters_manager.go
index f654b06b..92e2ee0c 100644
--- a/handshake/connection_parameters_manager.go
+++ b/handshake/connection_parameters_manager.go
@@ -26,8 +26,10 @@ type ConnectionParametersManager struct {
 	receiveConnectionFlowControlWindow protocol.ByteCount
 }
 
-// ErrTagNotInConnectionParameterMap is returned when a tag is not present in the connection parameters
-var ErrTagNotInConnectionParameterMap = errors.New("Tag not found in ConnectionsParameter map")
+var errTagNotInConnectionParameterMap = errors.New("ConnectionParametersManager: Tag not found in ConnectionsParameter map")
+
+// ErrMalformedTag is returned when the tag value cannot be read
+var ErrMalformedTag = errors.New("ConnectionParametersManager: malformed Tag value")
 
 // NewConnectionParamatersManager creates a new connection parameters manager
 func NewConnectionParamatersManager() *ConnectionParametersManager {
@@ -53,25 +55,25 @@ func (h *ConnectionParametersManager) SetFromMap(params map[Tag][]byte) error {
 		case TagMSPC:
 			clientValue, err := utils.ReadUint32(bytes.NewBuffer(value))
 			if err != nil {
-				return err
+				return ErrMalformedTag
 			}
 			h.maxStreamsPerConnection = h.negotiateMaxStreamsPerConnection(clientValue)
 		case TagICSL:
 			clientValue, err := utils.ReadUint32(bytes.NewBuffer(value))
 			if err != nil {
-				return err
+				return ErrMalformedTag
 			}
 			h.idleConnectionStateLifetime = h.negotiateIdleConnectionStateLifetime(time.Duration(clientValue) * time.Second)
 		case TagSFCW:
 			sendStreamFlowControlWindow, err := utils.ReadUint32(bytes.NewBuffer(value))
 			if err != nil {
-				return err
+				return ErrMalformedTag
 			}
 			h.sendStreamFlowControlWindow = protocol.ByteCount(sendStreamFlowControlWindow)
 		case TagCFCW:
 			sendConnectionFlowControlWindow, err := utils.ReadUint32(bytes.NewBuffer(value))
 			if err != nil {
-				return err
+				return ErrMalformedTag
 			}
 			h.sendConnectionFlowControlWindow = protocol.ByteCount(sendConnectionFlowControlWindow)
 		}
@@ -96,7 +98,7 @@ func (h *ConnectionParametersManager) getRawValue(tag Tag) ([]byte, error) {
 	h.mutex.RUnlock()
 
 	if !ok {
-		return nil, ErrTagNotInConnectionParameterMap
+		return nil, errTagNotInConnectionParameterMap
 	}
 	return rawValue, nil
 }
diff --git a/handshake/connection_parameters_manager_test.go b/handshake/connection_parameters_manager_test.go
index affc91b8..ebbb10a5 100644
--- a/handshake/connection_parameters_manager_test.go
+++ b/handshake/connection_parameters_manager_test.go
@@ -30,7 +30,7 @@ var _ = Describe("ConnectionsParameterManager", func() {
 	It("returns an error for a tag that is not set", func() {
 		_, err := cpm.getRawValue(TagKEXS)
 		Expect(err).To(HaveOccurred())
-		Expect(err).To(Equal(ErrTagNotInConnectionParameterMap))
+		Expect(err).To(Equal(errTagNotInConnectionParameterMap))
 	})
 
 	Context("SHLO", func() {
@@ -115,6 +115,7 @@ var _ = Describe("ConnectionsParameterManager", func() {
 			}
 			err := cpm.SetFromMap(values)
 			Expect(err).To(HaveOccurred())
+			Expect(err).To(Equal(ErrMalformedTag))
 			Expect(cpm.GetSendStreamFlowControlWindow()).To(Equal(protocol.InitialStreamFlowControlWindow))
 		})
 
@@ -133,6 +134,7 @@ var _ = Describe("ConnectionsParameterManager", func() {
 			}
 			err := cpm.SetFromMap(values)
 			Expect(err).To(HaveOccurred())
+			Expect(err).To(Equal(ErrMalformedTag))
 			Expect(cpm.GetSendStreamFlowControlWindow()).To(Equal(protocol.InitialConnectionFlowControlWindow))
 		})
 	})
@@ -160,6 +162,16 @@ var _ = Describe("ConnectionsParameterManager", func() {
 			Expect(cpm.GetIdleConnectionStateLifetime()).To(Equal(10 * time.Second))
 		})
 
+		It("does not change the idle connection state lifetime when given an invalid value", func() {
+			values := map[Tag][]byte{
+				TagSFCW: []byte{0xDE, 0xAD, 0xBE}, // 1 byte too short
+			}
+			err := cpm.SetFromMap(values)
+			Expect(err).To(HaveOccurred())
+			Expect(err).To(Equal(ErrMalformedTag))
+			Expect(cpm.GetIdleConnectionStateLifetime()).To(Equal(protocol.InitialIdleConnectionStateLifetime))
+		})
+
 		It("gets idle connection state lifetime", func() {
 			value := 0xDECAFBAD * time.Second
 			cpm.idleConnectionStateLifetime = value
@@ -186,6 +198,15 @@ var _ = Describe("ConnectionsParameterManager", func() {
 			Expect(cpm.GetMaxStreamsPerConnection()).To(Equal(uint32(2)))
 		})
 
+		It("errors when given an invalid max streams per connection value", func() {
+			values := map[Tag][]byte{
+				TagMSPC: []byte{2, 0, 0}, // 1 byte too short
+			}
+			err := cpm.SetFromMap(values)
+			Expect(err).To(HaveOccurred())
+			Expect(err).To(Equal(ErrMalformedTag))
+		})
+
 		It("gets the max streams per connection value", func() {
 			var value uint32 = 0xDECAFBAD
 			cpm.maxStreamsPerConnection = value