From 1f01904270a682883fe53a76d6afdd87d2cf70b9 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 6 Mar 2017 13:02:46 +0700 Subject: [PATCH] read packets with the maximum packet size fixes #467 --- buffer_pool.go | 4 ++-- buffer_pool_test.go | 4 ++-- client.go | 4 ++-- client_test.go | 7 +------ protocol/protocol.go | 12 +++++++----- protocol/server_parameters.go | 7 +++++++ server.go | 4 ++-- server_test.go | 2 +- 8 files changed, 24 insertions(+), 20 deletions(-) diff --git a/buffer_pool.go b/buffer_pool.go index bb4feb318..f592d475a 100644 --- a/buffer_pool.go +++ b/buffer_pool.go @@ -13,7 +13,7 @@ func getPacketBuffer() []byte { } func putPacketBuffer(buf []byte) { - if cap(buf) != int(protocol.MaxPacketSize) { + if cap(buf) != int(protocol.MaxReceivePacketSize) { panic("putPacketBuffer called with packet of wrong size!") } bufferPool.Put(buf[:0]) @@ -21,6 +21,6 @@ func putPacketBuffer(buf []byte) { func init() { bufferPool.New = func() interface{} { - return make([]byte, 0, protocol.MaxPacketSize) + return make([]byte, 0, protocol.MaxReceivePacketSize) } } diff --git a/buffer_pool_test.go b/buffer_pool_test.go index 5d64c6b66..888dc5768 100644 --- a/buffer_pool_test.go +++ b/buffer_pool_test.go @@ -11,7 +11,7 @@ var _ = Describe("Buffer Pool", func() { It("returns buffers of correct len and cap", func() { buf := getPacketBuffer() Expect(buf).To(HaveLen(0)) - Expect(buf).To(HaveCap(int(protocol.MaxPacketSize))) + Expect(buf).To(HaveCap(int(protocol.MaxReceivePacketSize))) }) It("zeroes put buffers' length", func() { @@ -20,7 +20,7 @@ var _ = Describe("Buffer Pool", func() { putPacketBuffer(buf[0:10]) buf = getPacketBuffer() Expect(buf).To(HaveLen(0)) - Expect(buf).To(HaveCap(int(protocol.MaxPacketSize))) + Expect(buf).To(HaveCap(int(protocol.MaxReceivePacketSize))) } }) diff --git a/client.go b/client.go index 5729fedef..f32ef49e4 100644 --- a/client.go +++ b/client.go @@ -115,7 +115,7 @@ func (c *client) listen() { var n int var addr net.Addr data := getPacketBuffer() - data = data[:protocol.MaxPacketSize] + data = data[:protocol.MaxReceivePacketSize] n, addr, err = c.conn.Read(data) if err != nil { @@ -141,7 +141,7 @@ func (c *client) listen() { } func (c *client) handlePacket(remoteAddr net.Addr, packet []byte) error { - if protocol.ByteCount(len(packet)) > protocol.MaxPacketSize { + if protocol.ByteCount(len(packet)) > protocol.MaxReceivePacketSize { return qerr.PacketTooLarge } diff --git a/client_test.go b/client_test.go index cc3c51b43..bdef8b316 100644 --- a/client_test.go +++ b/client_test.go @@ -112,11 +112,6 @@ var _ = Describe("Client", func() { Expect(err.(*qerr.QuicError).ErrorCode).To(Equal(qerr.InvalidPacketHeader)) }) - It("errors on large packets", func() { - err := cl.handlePacket(nil, bytes.Repeat([]byte{'a'}, int(protocol.MaxPacketSize)+1)) - Expect(err).To(MatchError(qerr.PacketTooLarge)) - }) - // this test requires a real session (because it calls the close callback) and a real UDP conn (because it unblocks and errors when it is closed) It("properly closes", func(done Done) { udpConn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1)}) @@ -152,7 +147,7 @@ var _ = Describe("Client", func() { Context("handling packets", func() { It("errors on too large packets", func() { - err := cl.handlePacket(nil, bytes.Repeat([]byte{'f'}, int(protocol.MaxPacketSize+1))) + err := cl.handlePacket(nil, bytes.Repeat([]byte{'f'}, int(protocol.MaxReceivePacketSize+1))) Expect(err).To(MatchError(qerr.PacketTooLarge)) }) diff --git a/protocol/protocol.go b/protocol/protocol.go index bb36e3be3..93c669484 100644 --- a/protocol/protocol.go +++ b/protocol/protocol.go @@ -36,12 +36,14 @@ type ByteCount uint64 // MaxByteCount is the maximum value of a ByteCount const MaxByteCount = math.MaxUint64 -// MaxPacketSize is the maximum packet size, including the public header -// This is the value used by Chromium for a QUIC packet sent using IPv6 (for IPv4 it would be 1370) -const MaxPacketSize ByteCount = 1350 +// CryptoStreamID is the ID of the crypto stream +const CryptoStreamID StreamID = 1 -// MaxFrameAndPublicHeaderSize is the maximum size of a QUIC frame plus PublicHeader -const MaxFrameAndPublicHeaderSize = MaxPacketSize - 12 /*crypto signature*/ +// MaxReceivePacketSize maximum packet size of any QUIC packet, based on +// ethernet's max size, minus the IP and UDP headers. IPv6 has a 40 byte header, +// UDP adds an additional 8 bytes. This is a total overhead of 48 bytes. +// Ethernet's max packet size is 1500 bytes, 1500 - 48 = 1452. +const MaxReceivePacketSize ByteCount = 1452 // DefaultTCPMSS is the default maximum packet size used in the Linux TCP implementation. // Used in QUIC for congestion window computations in bytes. diff --git a/protocol/server_parameters.go b/protocol/server_parameters.go index 1b2c9b40e..61b62fc11 100644 --- a/protocol/server_parameters.go +++ b/protocol/server_parameters.go @@ -2,6 +2,13 @@ package protocol import "time" +// MaxPacketSize is the maximum packet size, including the public header, that we use for sending packets +// This is the value used by Chromium for a QUIC packet sent using IPv6 (for IPv4 it would be 1370) +const MaxPacketSize ByteCount = 1350 + +// MaxFrameAndPublicHeaderSize is the maximum size of a QUIC frame plus PublicHeader +const MaxFrameAndPublicHeaderSize = MaxPacketSize - 12 /*crypto signature*/ + // NonForwardSecurePacketSizeReduction is the number of bytes a non forward-secure packet has to be smaller than a forward-secure packet // This makes sure that those packets can always be retransmitted without splitting the contained StreamFrames const NonForwardSecurePacketSizeReduction = 50 diff --git a/server.go b/server.go index 4cd548772..f003bef8c 100644 --- a/server.go +++ b/server.go @@ -83,7 +83,7 @@ func Listen(conn net.PacketConn, config *Config) (Listener, error) { func (s *server) Serve() error { for { data := getPacketBuffer() - data = data[:protocol.MaxPacketSize] + data = data[:protocol.MaxReceivePacketSize] n, remoteAddr, err := s.conn.ReadFrom(data) if err != nil { if strings.HasSuffix(err.Error(), "use of closed network connection") { @@ -122,7 +122,7 @@ func (s *server) Addr() net.Addr { } func (s *server) handlePacket(pconn net.PacketConn, remoteAddr net.Addr, packet []byte) error { - if protocol.ByteCount(len(packet)) > protocol.MaxPacketSize { + if protocol.ByteCount(len(packet)) > protocol.MaxReceivePacketSize { return qerr.PacketTooLarge } diff --git a/server_test.go b/server_test.go index 5ebaa9b46..63001269d 100644 --- a/server_test.go +++ b/server_test.go @@ -253,7 +253,7 @@ var _ = Describe("Server", func() { }) It("errors on large packets", func() { - err := serv.handlePacket(nil, nil, bytes.Repeat([]byte{'a'}, int(protocol.MaxPacketSize)+1)) + err := serv.handlePacket(nil, nil, bytes.Repeat([]byte{'a'}, int(protocol.MaxReceivePacketSize)+1)) Expect(err).To(MatchError(qerr.PacketTooLarge)) })