Merge pull request #1237 from lucas-clemente/fix-1233

use the max_packet_size transport parameter

internal/handshake/transport_parameter_test.go

@@ -132,6 +132,7 @@ var _ = Describe("Transport Parameters", func() {
 					initialMaxStreamIDBiDiParameterID: {0x33, 0x44, 0x55, 0x66},
 					initialMaxStreamIDUniParameterID:  {0x44, 0x55, 0x66, 0x77},
 					idleTimeoutParameterID:            {0x13, 0x37},
+					maxPacketSizeParameterID:          {0x73, 0x31},
 				}
 			})
 			It("reads parameters", func() {
@@ -143,6 +144,7 @@ var _ = Describe("Transport Parameters", func() {
 				Expect(params.MaxUniStreamID).To(Equal(protocol.StreamID(0x44556677)))
 				Expect(params.IdleTimeout).To(Equal(0x1337 * time.Second))
 				Expect(params.OmitConnectionID).To(BeFalse())
+				Expect(params.MaxPacketSize).To(Equal(protocol.ByteCount(0x7331)))
 			})
 
 			It("saves if it should omit the connection ID", func() {
@@ -215,6 +217,18 @@ var _ = Describe("Transport Parameters", func() {
 				Expect(err).To(MatchError("wrong length for omit_connection_id: 1 (expected empty)"))
 			})
 
+			It("rejects the parameters if max_packet_size has the wrong length", func() {
+				parameters[maxPacketSizeParameterID] = []byte{0x11} // should be 2 bytes
+				_, err := readTransportParamters(paramsMapToList(parameters))
+				Expect(err).To(MatchError("wrong length for max_packet_size: 1 (expected 2)"))
+			})
+
+			It("rejects max_packet_sizes smaller than 1200 bytes", func() {
+				parameters[maxPacketSizeParameterID] = []byte{0x4, 0xaf} // 0x4af = 1199
+				_, err := readTransportParamters(paramsMapToList(parameters))
+				Expect(err).To(MatchError("invalid value for max_packet_size: 1199 (minimum 1200)"))
+			})
+
 			It("ignores unknown parameters", func() {
 				parameters[1337] = []byte{42}
 				_, err := readTransportParamters(paramsMapToList(parameters))

internal/handshake/transport_parameters.go

@@ -20,6 +20,8 @@ type TransportParameters struct {
 	StreamFlowControlWindow     protocol.ByteCount
 	ConnectionFlowControlWindow protocol.ByteCount
 
+	MaxPacketSize protocol.ByteCount
+
 	MaxBidiStreamID protocol.StreamID // only used for IETF QUIC
 	MaxUniStreamID  protocol.StreamID // only used for IETF QUIC
 	MaxStreams      uint32            // only used for gQUIC
@@ -137,6 +139,15 @@ func readTransportParamters(paramsList []transportParameter) (*TransportParamete
 				return nil, fmt.Errorf("wrong length for omit_connection_id: %d (expected empty)", len(p.Value))
 			}
 			params.OmitConnectionID = true
+		case maxPacketSizeParameterID:
+			if len(p.Value) != 2 {
+				return nil, fmt.Errorf("wrong length for max_packet_size: %d (expected 2)", len(p.Value))
+			}
+			maxPacketSize := protocol.ByteCount(binary.BigEndian.Uint16(p.Value))
+			if maxPacketSize < 1200 {
+				return nil, fmt.Errorf("invalid value for max_packet_size: %d (minimum 1200)", maxPacketSize)
+			}
+			params.MaxPacketSize = maxPacketSize
 		}
 	}
 

packet_packer.go

@@ -10,6 +10,7 @@ import (
 	"github.com/lucas-clemente/quic-go/internal/ackhandler"
 	"github.com/lucas-clemente/quic-go/internal/handshake"
 	"github.com/lucas-clemente/quic-go/internal/protocol"
+	"github.com/lucas-clemente/quic-go/internal/utils"
 	"github.com/lucas-clemente/quic-go/internal/wire"
 )
 
@@ -518,3 +519,7 @@ func (p *packetPacker) canSendData(encLevel protocol.EncryptionLevel) bool {
 func (p *packetPacker) SetOmitConnectionID() {
 	p.omitConnectionID = true
 }
+
+func (p *packetPacker) SetMaxPacketSize(size protocol.ByteCount) {
+	p.maxPacketSize = utils.MinByteCount(p.maxPacketSize, size)
+}

packet_packer_test.go

@@ -895,7 +895,6 @@ var _ = Describe("Packet packer", func() {
 			Expect(sf2.StreamID).To(Equal(protocol.StreamID(5)))
 			Expect(sf2.DataLenPresent).To(BeFalse())
 		})
-
 	})
 
 	Context("packing ACK packets", func() {
@@ -917,4 +916,38 @@ var _ = Describe("Packet packer", func() {
 			}))
 		})
 	})
+
+	Context("max packet size", func() {
+		It("sets the maximum packet size", func() {
+			for i := 0; i < 10*int(maxPacketSize); i++ {
+				packer.QueueControlFrame(&wire.PingFrame{})
+			}
+			mockStreamFramer.EXPECT().HasCryptoStreamData().AnyTimes()
+			mockStreamFramer.EXPECT().PopStreamFrames(gomock.Any()).AnyTimes()
+			p, err := packer.PackPacket()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(p.raw).To(HaveLen(int(maxPacketSize)))
+			// now reduce the maxPacketSize
+			packer.SetMaxPacketSize(maxPacketSize - 10)
+			p, err = packer.PackPacket()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(p.raw).To(HaveLen(int(maxPacketSize) - 10))
+		})
+
+		It("doesn't increase the max packet size", func() {
+			for i := 0; i < 10*int(maxPacketSize); i++ {
+				packer.QueueControlFrame(&wire.PingFrame{})
+			}
+			mockStreamFramer.EXPECT().HasCryptoStreamData().AnyTimes()
+			mockStreamFramer.EXPECT().PopStreamFrames(gomock.Any()).AnyTimes()
+			p, err := packer.PackPacket()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(p.raw).To(HaveLen(int(maxPacketSize)))
+			// now try to increase the maxPacketSize
+			packer.SetMaxPacketSize(maxPacketSize + 10)
+			p, err = packer.PackPacket()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(p.raw).To(HaveLen(int(maxPacketSize)))
+		})
+	})
 })

session.go

@@ -759,6 +759,9 @@ func (s *session) processTransportParameters(params *handshake.TransportParamete
 	if params.OmitConnectionID {
 		s.packer.SetOmitConnectionID()
 	}
+	if params.MaxPacketSize != 0 {
+		s.packer.SetMaxPacketSize(params.MaxPacketSize)
+	}
 	s.connFlowController.UpdateSendWindow(params.ConnectionFlowControlWindow)
 	// the crypto stream is the only open stream at this moment
 	// so we don't need to update stream flow control windows

session_test.go

@@ -1361,11 +1361,13 @@ var _ = Describe("Session", func() {
 			StreamFlowControlWindow:     0x5000,
 			ConnectionFlowControlWindow: 0x5000,
 			OmitConnectionID:            true,
+			MaxPacketSize:               0x42,
 		}
 		streamManager.EXPECT().UpdateLimits(&params)
 		paramsChan <- params
 		Eventually(func() *handshake.TransportParameters { return sess.peerParams }).Should(Equal(&params))
 		Eventually(func() bool { return sess.packer.omitConnectionID }).Should(BeTrue())
+		Eventually(func() protocol.ByteCount { return sess.packer.maxPacketSize }).Should(Equal(protocol.ByteCount(0x42)))
 		// make the go routine return
 		streamManager.EXPECT().CloseWithError(gomock.Any())
 		Expect(sess.Close(nil)).To(Succeed())