diff --git a/ackhandler/received_packet_handler.go b/ackhandler/received_packet_handler.go index daebbfb2..f88178fa 100644 --- a/ackhandler/received_packet_handler.go +++ b/ackhandler/received_packet_handler.go @@ -8,13 +8,6 @@ import ( "github.com/lucas-clemente/quic-go/protocol" ) -var ( - // ErrDuplicatePacket occurres when a duplicate packet is received - ErrDuplicatePacket = errors.New("ReceivedPacketHandler: Duplicate Packet") - // ErrPacketSmallerThanLastStopWaiting occurs when a packet arrives with a packet number smaller than the largest LeastUnacked of a StopWaitingFrame. If this error occurs, the packet should be ignored - ErrPacketSmallerThanLastStopWaiting = errors.New("ReceivedPacketHandler: Packet number smaller than highest StopWaiting") -) - var errInvalidPacketNumber = errors.New("ReceivedPacketHandler: Invalid packet number") type receivedPacketHandler struct { @@ -52,19 +45,10 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe return errInvalidPacketNumber } - // if the packet number is smaller than the largest LeastUnacked value of a StopWaiting we received, we cannot detect if this packet has a duplicate number - // the packet has to be ignored anyway - if packetNumber <= h.ignorePacketsBelow { - return ErrPacketSmallerThanLastStopWaiting - } - - if h.packetHistory.IsDuplicate(packetNumber) { - return ErrDuplicatePacket - } - - err := h.packetHistory.ReceivedPacket(packetNumber) - if err != nil { - return err + if packetNumber > h.ignorePacketsBelow { + if err := h.packetHistory.ReceivedPacket(packetNumber); err != nil { + return err + } } if packetNumber > h.largestObserved { diff --git a/ackhandler/received_packet_handler_test.go b/ackhandler/received_packet_handler_test.go index 6e9d1134..cece2df3 100644 --- a/ackhandler/received_packet_handler_test.go +++ b/ackhandler/received_packet_handler_test.go @@ -40,24 +40,6 @@ var _ = Describe("receivedPacketHandler", func() { Expect(err).To(MatchError(errInvalidPacketNumber)) }) - It("rejects a duplicate package", func() { - for i := 1; i < 5; i++ { - err := handler.ReceivedPacket(protocol.PacketNumber(i), true) - Expect(err).ToNot(HaveOccurred()) - } - err := handler.ReceivedPacket(4, true) - Expect(err).To(MatchError(ErrDuplicatePacket)) - }) - - It("ignores a packet with PacketNumber less than the LeastUnacked of a previously received StopWaiting", func() { - err := handler.ReceivedPacket(5, true) - Expect(err).ToNot(HaveOccurred()) - err = handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: 10}) - Expect(err).ToNot(HaveOccurred()) - err = handler.ReceivedPacket(9, true) - Expect(err).To(MatchError(ErrPacketSmallerThanLastStopWaiting)) - }) - It("does not ignore a packet with PacketNumber equal to LeastUnacked of a previously received StopWaiting", func() { err := handler.ReceivedPacket(5, true) Expect(err).ToNot(HaveOccurred()) @@ -274,6 +256,19 @@ var _ = Describe("receivedPacketHandler", func() { Expect(ack.AckRanges[1]).To(Equal(frames.AckRange{FirstPacketNumber: 1, LastPacketNumber: 1})) }) + It("doesn't add delayed packets to the packetHistory", func() { + err := handler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: protocol.PacketNumber(6)}) + Expect(err).ToNot(HaveOccurred()) + err = handler.ReceivedPacket(4, true) + Expect(err).ToNot(HaveOccurred()) + err = handler.ReceivedPacket(10, true) + Expect(err).ToNot(HaveOccurred()) + ack := handler.GetAckFrame() + Expect(ack).ToNot(BeNil()) + Expect(ack.LargestAcked).To(Equal(protocol.PacketNumber(10))) + Expect(ack.LowestAcked).To(Equal(protocol.PacketNumber(10))) + }) + It("deletes packets from the packetHistory after receiving a StopWaiting, after continuously received packets", func() { for i := 1; i <= 12; i++ { err := handler.ReceivedPacket(protocol.PacketNumber(i), true) diff --git a/session.go b/session.go index 7068bcbc..b5124303 100644 --- a/session.go +++ b/session.go @@ -396,19 +396,7 @@ func (s *session) handlePacketImpl(p *receivedPacket) error { // Only do this after decrypting, so we are sure the packet is not attacker-controlled s.largestRcvdPacketNumber = utils.MaxPacketNumber(s.largestRcvdPacketNumber, hdr.PacketNumber) - err = s.receivedPacketHandler.ReceivedPacket(hdr.PacketNumber, packet.IsRetransmittable()) - // ignore duplicate packets - if err == ackhandler.ErrDuplicatePacket { - utils.Infof("Ignoring packet 0x%x due to ErrDuplicatePacket", hdr.PacketNumber) - return nil - } - // ignore packets with packet numbers smaller than the LeastUnacked of a StopWaiting - if err == ackhandler.ErrPacketSmallerThanLastStopWaiting { - utils.Infof("Ignoring packet 0x%x due to ErrPacketSmallerThanLastStopWaiting", hdr.PacketNumber) - return nil - } - - if err != nil { + if err = s.receivedPacketHandler.ReceivedPacket(hdr.PacketNumber, packet.IsRetransmittable()); err != nil { return err } diff --git a/session_test.go b/session_test.go index 228aef3a..e91e5ad5 100644 --- a/session_test.go +++ b/session_test.go @@ -813,7 +813,7 @@ var _ = Describe("Session", func() { Expect(sess.largestRcvdPacketNumber).To(Equal(protocol.PacketNumber(5))) }) - It("ignores duplicate packets", func() { + It("handles duplicate packets", func() { hdr.PacketNumber = 5 err := sess.handlePacketImpl(&receivedPacket{publicHeader: hdr}) Expect(err).ToNot(HaveOccurred()) @@ -821,7 +821,7 @@ var _ = Describe("Session", func() { Expect(err).ToNot(HaveOccurred()) }) - It("ignores packets smaller than the highest LeastUnacked of a StopWaiting", func() { + It("handles packets smaller than the highest LeastUnacked of a StopWaiting", func() { err := sess.receivedPacketHandler.ReceivedStopWaiting(&frames.StopWaitingFrame{LeastUnacked: 10}) Expect(err).ToNot(HaveOccurred()) hdr.PacketNumber = 5