quic: Delay delivering trailers when initial headers aren't read yet
Before this CL, there was a DCHECK to confirm initial headers are
delivered when trailers are available. This assumption was wrong since
the handler may not read initial headers at the time trailers are
available. Remove the DCHECK and return early when initial headers
aren't read yet.
Bug: 1248970
Change-Id: I47351fbe1e67d13f2e8f98561960d467d6320aea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3184063
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/main@{#925535}
diff --git a/net/quic/quic_chromium_client_stream.cc b/net/quic/quic_chromium_client_stream.cc
index 69e2fce..9fc1055 100644
--- a/net/quic/quic_chromium_client_stream.cc
+++ b/net/quic/quic_chromium_client_stream.cc
@@ -735,7 +735,10 @@
if (!trailers_decompressed())
return;
- DCHECK(headers_delivered_);
+ // Notify only after the handle reads initial headers.
+ if (!headers_delivered_)
+ return;
+
// Post an async task to notify handle of the FIN flag.
NotifyHandleOfDataAvailableLater();
handle_->OnTrailingHeadersAvailable();
diff --git a/net/quic/quic_chromium_client_stream_test.cc b/net/quic/quic_chromium_client_stream_test.cc
index 4d5cc610..d0ac1c2 100644
--- a/net/quic/quic_chromium_client_stream_test.cc
+++ b/net/quic/quic_chromium_client_stream_test.cc
@@ -1132,6 +1132,52 @@
base::RunLoop().RunUntilIdle();
}
+// Regression test for https://crbug.com/1248970. Write an Early Hints headers,
+// an initial response headers and trailers in succession without reading in
+// the middle of writings.
+TEST_P(QuicChromiumClientStreamTest, TrailersAfterEarlyHintsWithoutRead) {
+ // Process an Early Hints response headers on the stream.
+ spdy::Http2HeaderBlock hints_headers = CreateResponseHeaders("103");
+ quic::QuicHeaderList hints_header_list = ProcessHeaders(hints_headers);
+
+ // Process an initial response headers on the stream.
+ InitializeHeaders();
+ quic::QuicHeaderList header_list = ProcessHeaders(headers_);
+
+ // Process a trailer headers on the stream. This should not hit any DCHECK.
+ spdy::Http2HeaderBlock trailers;
+ trailers["bar"] = "foo";
+ quic::QuicHeaderList trailer_header_list = ProcessTrailers(trailers);
+ base::RunLoop().RunUntilIdle();
+
+ // Read the Early Hints response from the handle.
+ {
+ spdy::Http2HeaderBlock headers;
+ TestCompletionCallback callback;
+ EXPECT_EQ(static_cast<int>(hints_header_list.uncompressed_header_bytes()),
+ handle_->ReadInitialHeaders(&headers, callback.callback()));
+ EXPECT_EQ(headers, hints_headers);
+ }
+
+ // Read the initial headers from the handle.
+ {
+ spdy::Http2HeaderBlock headers;
+ TestCompletionCallback callback;
+ EXPECT_EQ(static_cast<int>(header_list.uncompressed_header_bytes()),
+ handle_->ReadInitialHeaders(&headers, callback.callback()));
+ EXPECT_EQ(headers, headers_);
+ }
+
+ // Read trailers from the handle.
+ {
+ spdy::Http2HeaderBlock headers;
+ TestCompletionCallback callback;
+ EXPECT_EQ(static_cast<int>(trailer_header_list.uncompressed_header_bytes()),
+ handle_->ReadTrailingHeaders(&headers, callback.callback()));
+ EXPECT_EQ(headers, trailers);
+ }
+}
+
} // namespace
} // namespace test
} // namespace net
