From 0cc3eabc5652bdba6eb1b8d39ebad53bf81fc10e Mon Sep 17 00:00:00 2001
From: Junqi Wang <junqiw@fb.com>
Date: Wed, 26 Jun 2019 22:25:31 -0700
Subject: [PATCH] Do not retransmit PATH_RESPONSE

Summary: https://github.com/quicwg/base-drafts/issues/2724

Reviewed By: mjoras

Differential Revision: D15719978

fbshipit-source-id: 1c57f3e2b497f1abfcd5fa3b5643d76aa51d11d9
---
 quic/api/test/QuicTransportTest.cpp | 10 +++-------
 quic/state/SimpleFrameFunctions.cpp |  9 +++++----
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp
index 943ca250a..aa96a1a45 100644
--- a/quic/api/test/QuicTransportTest.cpp
+++ b/quic/api/test/QuicTransportTest.cpp
@@ -1283,10 +1283,10 @@ TEST_F(QuicTransportTest, ClonePathResponse) {
                          [&](auto&) { return false; });
                    }) != p.packet.frames.end();
       });
-  EXPECT_EQ(numPathResponsePackets, 3);
+  EXPECT_EQ(numPathResponsePackets, 1);
 }
 
-TEST_F(QuicTransportTest, ResendPathResponseOnLoss) {
+TEST_F(QuicTransportTest, DoNotResendPathResponseOnLoss) {
   auto& conn = transport_->getConnectionState();
 
   EXPECT_EQ(conn.pendingEvents.frames.size(), 0);
@@ -1302,11 +1302,7 @@ TEST_F(QuicTransportTest, ResendPathResponseOnLoss) {
       getLastOutstandingPacket(conn, PacketNumberSpace::AppData)->packet;
 
   markPacketLoss(conn, packet, false, 2);
-  EXPECT_EQ(conn.pendingEvents.frames.size(), 1);
-  EXPECT_TRUE(folly::variant_match(
-      conn.pendingEvents.frames.front(),
-      [&](PathResponseFrame& f) { return f == pathResponse; },
-      [&](auto&) { return false; }));
+  EXPECT_EQ(conn.pendingEvents.frames.size(), 0);
 }
 
 TEST_F(QuicTransportTest, SendNewConnectionIdFrame) {
diff --git a/quic/state/SimpleFrameFunctions.cpp b/quic/state/SimpleFrameFunctions.cpp
index 9ace493bd..674db35f7 100644
--- a/quic/state/SimpleFrameFunctions.cpp
+++ b/quic/state/SimpleFrameFunctions.cpp
@@ -56,8 +56,9 @@ folly::Optional<QuicSimpleFrame> updateSimpleFrameOnPacketClone(
         }
         return QuicSimpleFrame(frame);
       },
-      [&](const PathResponseFrame& frame) -> folly::Optional<QuicSimpleFrame> {
-        return QuicSimpleFrame(frame);
+      [&](const PathResponseFrame&) -> folly::Optional<QuicSimpleFrame> {
+        // Do not clone PATH_RESPONSE to avoid buffering
+        return folly::none;
       },
       [&](const NewConnectionIdFrame& frame)
           -> folly::Optional<QuicSimpleFrame> {
@@ -118,8 +119,8 @@ void updateSimpleFrameOnPacketLoss(
           conn.pendingEvents.pathChallenge = frame;
         }
       },
-      [&](const PathResponseFrame& frame) {
-        conn.pendingEvents.frames.push_back(frame);
+      [&](const PathResponseFrame&) {
+        // Do not retransmit PATH_RESPONSE to avoid buffering
       },
       [&](const NewConnectionIdFrame& frame) {
         conn.pendingEvents.frames.push_back(frame);