From fd1b3a23c42ae69a63dd1dc9c45faecaaeaba4b8 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 27 Aug 2022 10:46:11 +0300 Subject: [PATCH] remove unneeded packet number field from the unpackedPacket --- connection.go | 6 ++--- connection_test.go | 53 +++++++++++++++++++++++++---------------- packet_unpacker.go | 2 -- packet_unpacker_test.go | 2 +- 4 files changed, 37 insertions(+), 26 deletions(-) diff --git a/connection.go b/connection.go index 87768db9..fdde3533 100644 --- a/connection.go +++ b/connection.go @@ -982,11 +982,11 @@ func (s *connection) handleSinglePacket(p *receivedPacket, hdr *wire.Header) boo } if s.logger.Debug() { - s.logger.Debugf("<- Reading packet %d (%d bytes) for connection %s, %s", packet.packetNumber, p.Size(), hdr.DestConnectionID, packet.encryptionLevel) + s.logger.Debugf("<- Reading packet %d (%d bytes) for connection %s, %s", packet.hdr.PacketNumber, p.Size(), hdr.DestConnectionID, packet.encryptionLevel) packet.hdr.Log(s.logger) } - if s.receivedPacketHandler.IsPotentiallyDuplicate(packet.packetNumber, packet.encryptionLevel) { + if s.receivedPacketHandler.IsPotentiallyDuplicate(packet.hdr.PacketNumber, packet.encryptionLevel) { s.logger.Debugf("Dropping (potentially) duplicate packet.") if s.tracer != nil { s.tracer.DroppedPacket(logging.PacketTypeFromHeader(hdr), p.Size(), logging.PacketDropDuplicate) @@ -1214,7 +1214,7 @@ func (s *connection) handleUnpackedPacket( } } - return s.receivedPacketHandler.ReceivedPacket(packet.packetNumber, ecn, packet.encryptionLevel, rcvTime, isAckEliciting) + return s.receivedPacketHandler.ReceivedPacket(packet.hdr.PacketNumber, ecn, packet.encryptionLevel, rcvTime, isAckEliciting) } func (s *connection) handleFrame(f wire.Frame, encLevel protocol.EncryptionLevel, destConnID protocol.ConnectionID) error { diff --git a/connection_test.go b/connection_test.go index 3956616d..bbe49fa0 100644 --- a/connection_test.go +++ b/connection_test.go @@ -731,13 +731,14 @@ var _ = Describe("Connection", func() { PacketNumber: 0x37, PacketNumberLen: protocol.PacketNumberLen1, } + unpackedHdr := *hdr + unpackedHdr.PacketNumber = 0x1337 packet := getPacket(hdr, nil) packet.ecn = protocol.ECNCE rcvTime := time.Now().Add(-10 * time.Second) unpacker.EXPECT().Unpack(gomock.Any(), rcvTime, gomock.Any()).Return(&unpackedPacket{ - packetNumber: 0x1337, encryptionLevel: protocol.EncryptionInitial, - hdr: hdr, + hdr: &unpackedHdr, data: []byte{0}, // one PADDING frame }, nil) rph := mockackhandler.NewMockReceivedPacketHandler(mockCtrl) @@ -748,7 +749,7 @@ var _ = Describe("Connection", func() { conn.receivedPacketHandler = rph packet.rcvTime = rcvTime tracer.EXPECT().StartedConnection(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - tracer.EXPECT().ReceivedPacket(hdr, protocol.ByteCount(len(packet.data)), []logging.Frame{}) + tracer.EXPECT().ReceivedPacket(&unpackedHdr, protocol.ByteCount(len(packet.data)), []logging.Frame{}) Expect(conn.handlePacketImpl(packet)).To(BeTrue()) }) @@ -758,15 +759,16 @@ var _ = Describe("Connection", func() { PacketNumber: 0x37, PacketNumberLen: protocol.PacketNumberLen1, } + unpackedHdr := *hdr + unpackedHdr.PacketNumber = 0x1337 rcvTime := time.Now().Add(-10 * time.Second) b, err := (&wire.PingFrame{}).Append(nil, conn.version) Expect(err).ToNot(HaveOccurred()) packet := getPacket(hdr, nil) packet.ecn = protocol.ECT1 unpacker.EXPECT().Unpack(gomock.Any(), rcvTime, gomock.Any()).Return(&unpackedPacket{ - packetNumber: 0x1337, encryptionLevel: protocol.Encryption1RTT, - hdr: hdr, + hdr: &unpackedHdr, data: b, }, nil) rph := mockackhandler.NewMockReceivedPacketHandler(mockCtrl) @@ -777,7 +779,7 @@ var _ = Describe("Connection", func() { conn.receivedPacketHandler = rph packet.rcvTime = rcvTime tracer.EXPECT().StartedConnection(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - tracer.EXPECT().ReceivedPacket(hdr, protocol.ByteCount(len(packet.data)), []logging.Frame{&logging.PingFrame{}}) + tracer.EXPECT().ReceivedPacket(&unpackedHdr, protocol.ByteCount(len(packet.data)), []logging.Frame{&logging.PingFrame{}}) Expect(conn.handlePacketImpl(packet)).To(BeTrue()) }) @@ -788,12 +790,15 @@ var _ = Describe("Connection", func() { PacketNumberLen: protocol.PacketNumberLen1, } packet := getPacket(hdr, nil) - unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any(), gomock.Any()).Return(&unpackedPacket{ - packetNumber: 0x1337, - encryptionLevel: protocol.Encryption1RTT, - hdr: hdr, - data: []byte("foobar"), - }, nil) + unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(*wire.Header, time.Time, []byte) (*unpackedPacket, error) { + h := *hdr + h.PacketNumber = 0x1337 + return &unpackedPacket{ + encryptionLevel: protocol.Encryption1RTT, + hdr: &h, + data: []byte("foobar"), + }, nil + }) rph := mockackhandler.NewMockReceivedPacketHandler(mockCtrl) rph.EXPECT().IsPotentiallyDuplicate(protocol.PacketNumber(0x1337), protocol.Encryption1RTT).Return(true) conn.receivedPacketHandler = rph @@ -842,8 +847,10 @@ var _ = Describe("Connection", func() { return &unpackedPacket{ data: []byte{0}, // PADDING frame encryptionLevel: protocol.Encryption1RTT, - packetNumber: pn, - hdr: &wire.ExtendedHeader{Header: *hdr}, + hdr: &wire.ExtendedHeader{ + Header: *hdr, + PacketNumber: pn, + }, }, nil }).Times(3) tracer.EXPECT().StartedConnection(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) @@ -887,8 +894,10 @@ var _ = Describe("Connection", func() { return &unpackedPacket{ data: []byte{0}, // PADDING frame encryptionLevel: protocol.Encryption1RTT, - packetNumber: pn, - hdr: &wire.ExtendedHeader{Header: *hdr}, + hdr: &wire.ExtendedHeader{ + Header: *hdr, + PacketNumber: pn, + }, }, nil }).Times(3) tracer.EXPECT().StartedConnection(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) @@ -1160,8 +1169,10 @@ var _ = Describe("Connection", func() { return &unpackedPacket{ encryptionLevel: protocol.EncryptionHandshake, data: []byte{0}, - packetNumber: 1, - hdr: &wire.ExtendedHeader{Header: wire.Header{SrcConnectionID: destConnID}}, + hdr: &wire.ExtendedHeader{ + PacketNumber: 1, + Header: wire.Header{SrcConnectionID: destConnID}, + }, }, nil }) hdrLen2, packet2 := getPacketWithLength(srcConnID, 123) @@ -1170,8 +1181,10 @@ var _ = Describe("Connection", func() { return &unpackedPacket{ encryptionLevel: protocol.EncryptionHandshake, data: []byte{0}, - packetNumber: 2, - hdr: &wire.ExtendedHeader{Header: wire.Header{SrcConnectionID: destConnID}}, + hdr: &wire.ExtendedHeader{ + PacketNumber: 2, + Header: wire.Header{SrcConnectionID: destConnID}, + }, }, nil }) gomock.InOrder( diff --git a/packet_unpacker.go b/packet_unpacker.go index f70d8d07..59f6966f 100644 --- a/packet_unpacker.go +++ b/packet_unpacker.go @@ -27,7 +27,6 @@ func (e *headerParseError) Error() string { } type unpackedPacket struct { - packetNumber protocol.PacketNumber // the decoded packet number hdr *wire.ExtendedHeader encryptionLevel protocol.EncryptionLevel data []byte @@ -105,7 +104,6 @@ func (u *packetUnpacker) Unpack(hdr *wire.Header, rcvTime time.Time, data []byte return &unpackedPacket{ hdr: extHdr, - packetNumber: extHdr.PacketNumber, encryptionLevel: encLevel, data: decrypted, }, nil diff --git a/packet_unpacker_test.go b/packet_unpacker_test.go index 813cf82a..fd8b50b4 100644 --- a/packet_unpacker_test.go +++ b/packet_unpacker_test.go @@ -287,6 +287,6 @@ var _ = Describe("Packet Unpacker", func() { } packet, err := unpacker.Unpack(hdr, time.Now(), data) Expect(err).ToNot(HaveOccurred()) - Expect(packet.packetNumber).To(Equal(protocol.PacketNumber(0x7331))) + Expect(packet.hdr.PacketNumber).To(Equal(protocol.PacketNumber(0x7331))) }) })