Support multiple interim responses in SimpleClient.
Currently, SimpleClient and descendants support 100 Continue responses (an
interim response) by reading in 100 Continue to response_headers(). Then once
100 Continue is populated and verified, the client calls
ClearPreliminaryResponse() so that response_headers() can be reused for the
final response.
This CL updates SimpleClient and descendants to support multiple interim
responses by introducing interim_headers(), an ordered sequence of BalsaHeaders
for interim responses. This CL removes ClearPreliminaryResponse() and adds
WaitForInterimResponse(), which in the future may be called multiple times.
SimpleHttpClient also calls BalsaFrame::set_use_interim_headers_callback(true),
which gives some additional testing confidence in this new code path.
This CL simplifies some 100-specific logic in the test clients. However, the
client continues to treat 101 as a final response (even in SimpleHttp2Client,
for consistency with SimpleHttpClient) due to the issue in cl/528583898.
One part of this CL involves updates to the QUIC (test) client ecosystem. I
decided to replace the interim/preliminary headers functionality in
QuicTestClient, as updating interim headers on querying (rather than on
receipt) can interact strangely with stream resets and timeouts. This CL
instead plumbs a callback to update interim headers with the flow
SimpleQuicClient --> GfeMockableQuicClient --> QuicSimpleClientSession -->
QuicSimpleClientStream, which can immediately update interim headers on
receipt. A comparison:
$ blaze test //gfe/gfe2/e2e:expect_100_continue_test_ietf_quic
Before: "PASSED in 307.2s" (http://sponge2/2c1eb63c-2bcc-4399-a329-98c638f562ba)
After: "PASSED in 11.0s" (http://sponge2/6c178f44-0aa3-4684-a4b0-e52fdbe61ae2)
I also verified with ++haoyuewang@ that the QuicSpdyClientStream changes do not
have a production impact on Hyperloop.
Huge inspiration from and thanks to ++bnc@'s foundational cl/379795244, which
has made this CL possible!
PiperOrigin-RevId: 529847861
diff --git a/quiche/quic/core/http/quic_spdy_client_stream.cc b/quiche/quic/core/http/quic_spdy_client_stream.cc
index 4eded5e..2f6eb8d 100644
--- a/quiche/quic/core/http/quic_spdy_client_stream.cc
+++ b/quiche/quic/core/http/quic_spdy_client_stream.cc
@@ -28,8 +28,7 @@
response_code_(0),
header_bytes_read_(0),
header_bytes_written_(0),
- session_(session),
- has_preliminary_headers_(false) {}
+ session_(session) {}
QuicSpdyClientStream::QuicSpdyClientStream(PendingStream* pending,
QuicSpdyClientSession* session)
@@ -38,8 +37,7 @@
response_code_(0),
header_bytes_read_(0),
header_bytes_written_(0),
- session_(session),
- has_preliminary_headers_(false) {}
+ session_(session) {}
QuicSpdyClientStream::~QuicSpdyClientStream() = default;
@@ -74,13 +72,7 @@
<< response_headers_[":status"].as_string() << " on stream "
<< id();
set_headers_decompressed(false);
- if (response_code_ == 100 && !has_preliminary_headers_) {
- // This is 100 Continue, save it to enable "Expect: 100-continue".
- has_preliminary_headers_ = true;
- preliminary_headers_ = std::move(response_headers_);
- } else {
- response_headers_.clear();
- }
+ preliminary_headers_.push_back(std::move(response_headers_));
}
return true;
diff --git a/quiche/quic/core/http/quic_spdy_client_stream.h b/quiche/quic/core/http/quic_spdy_client_stream.h
index 9a952aa..02956c6 100644
--- a/quiche/quic/core/http/quic_spdy_client_stream.h
+++ b/quiche/quic/core/http/quic_spdy_client_stream.h
@@ -6,6 +6,7 @@
#define QUICHE_QUIC_CORE_HTTP_QUIC_SPDY_CLIENT_STREAM_H_
#include <cstddef>
+#include <list>
#include <string>
#include "absl/strings/string_view.h"
@@ -56,7 +57,7 @@
// Returns whatever headers have been received for this stream.
const spdy::Http2HeaderBlock& response_headers() { return response_headers_; }
- const spdy::Http2HeaderBlock& preliminary_headers() {
+ const std::list<spdy::Http2HeaderBlock>& preliminary_headers() {
return preliminary_headers_;
}
@@ -96,11 +97,9 @@
QuicSpdyClientSession* session_;
- // These preliminary headers are used for the 100 Continue headers
- // that may arrive before the response headers when the request has
- // Expect: 100-continue.
- bool has_preliminary_headers_;
- spdy::Http2HeaderBlock preliminary_headers_;
+ // These preliminary headers are used for interim response headers that may
+ // arrive before the final response headers.
+ std::list<spdy::Http2HeaderBlock> preliminary_headers_;
};
} // namespace quic
diff --git a/quiche/quic/core/http/quic_spdy_client_stream_test.cc b/quiche/quic/core/http/quic_spdy_client_stream_test.cc
index b249a98..2735b01 100644
--- a/quiche/quic/core/http/quic_spdy_client_stream_test.cc
+++ b/quiche/quic/core/http/quic_spdy_client_stream_test.cc
@@ -166,7 +166,9 @@
auto headers = AsHeaderList(headers_);
stream_->OnStreamHeaderList(false, headers.uncompressed_header_bytes(),
headers);
- EXPECT_EQ("100", stream_->preliminary_headers().find(":status")->second);
+ ASSERT_EQ(stream_->preliminary_headers().size(), 1);
+ EXPECT_EQ("100",
+ stream_->preliminary_headers().front().find(":status")->second);
EXPECT_EQ(0u, stream_->response_headers().size());
EXPECT_EQ(100, stream_->response_code());
EXPECT_EQ("", stream_->data());
@@ -187,7 +189,9 @@
EXPECT_EQ(200, stream_->response_code());
EXPECT_EQ(body_, stream_->data());
// Make sure the 100 response is still available.
- EXPECT_EQ("100", stream_->preliminary_headers().find(":status")->second);
+ ASSERT_EQ(stream_->preliminary_headers().size(), 1);
+ EXPECT_EQ("100",
+ stream_->preliminary_headers().front().find(":status")->second);
}
TEST_P(QuicSpdyClientStreamTest, TestUnknownInformationalBeforeSuccessful) {
@@ -196,6 +200,9 @@
auto headers = AsHeaderList(headers_);
stream_->OnStreamHeaderList(false, headers.uncompressed_header_bytes(),
headers);
+ ASSERT_EQ(stream_->preliminary_headers().size(), 1);
+ EXPECT_EQ("199",
+ stream_->preliminary_headers().front().find(":status")->second);
EXPECT_EQ(0u, stream_->response_headers().size());
EXPECT_EQ(199, stream_->response_code());
EXPECT_EQ("", stream_->data());
@@ -215,6 +222,63 @@
EXPECT_EQ("200", stream_->response_headers().find(":status")->second);
EXPECT_EQ(200, stream_->response_code());
EXPECT_EQ(body_, stream_->data());
+ // Make sure the 199 response is still available.
+ ASSERT_EQ(stream_->preliminary_headers().size(), 1);
+ EXPECT_EQ("199",
+ stream_->preliminary_headers().front().find(":status")->second);
+}
+
+TEST_P(QuicSpdyClientStreamTest, TestMultipleInformationalBeforeSuccessful) {
+ // First send 100 Continue.
+ headers_[":status"] = "100";
+ auto headers = AsHeaderList(headers_);
+ stream_->OnStreamHeaderList(false, headers.uncompressed_header_bytes(),
+ headers);
+ ASSERT_EQ(stream_->preliminary_headers().size(), 1);
+ EXPECT_EQ("100",
+ stream_->preliminary_headers().front().find(":status")->second);
+ EXPECT_EQ(0u, stream_->response_headers().size());
+ EXPECT_EQ(100, stream_->response_code());
+ EXPECT_EQ("", stream_->data());
+
+ // Then send 199, an unknown Informational (1XX).
+ headers_[":status"] = "199";
+ headers = AsHeaderList(headers_);
+ stream_->OnStreamHeaderList(false, headers.uncompressed_header_bytes(),
+ headers);
+ ASSERT_EQ(stream_->preliminary_headers().size(), 2);
+ EXPECT_EQ("100",
+ stream_->preliminary_headers().front().find(":status")->second);
+ EXPECT_EQ("199",
+ stream_->preliminary_headers().back().find(":status")->second);
+ EXPECT_EQ(0u, stream_->response_headers().size());
+ EXPECT_EQ(199, stream_->response_code());
+ EXPECT_EQ("", stream_->data());
+
+ // Then send 200 OK.
+ headers_[":status"] = "200";
+ headers = AsHeaderList(headers_);
+ stream_->OnStreamHeaderList(false, headers.uncompressed_header_bytes(),
+ headers);
+ quiche::QuicheBuffer header = HttpEncoder::SerializeDataFrameHeader(
+ body_.length(), quiche::SimpleBufferAllocator::Get());
+ std::string data = VersionUsesHttp3(connection_->transport_version())
+ ? absl::StrCat(header.AsStringView(), body_)
+ : body_;
+ stream_->OnStreamFrame(
+ QuicStreamFrame(stream_->id(), /*fin=*/false, /*offset=*/0, data));
+
+ // Make sure the 200 response got parsed correctly.
+ EXPECT_EQ("200", stream_->response_headers().find(":status")->second);
+ EXPECT_EQ(200, stream_->response_code());
+ EXPECT_EQ(body_, stream_->data());
+
+ // Make sure the informational responses are still available.
+ ASSERT_EQ(stream_->preliminary_headers().size(), 2);
+ EXPECT_EQ("100",
+ stream_->preliminary_headers().front().find(":status")->second);
+ EXPECT_EQ("199",
+ stream_->preliminary_headers().back().find(":status")->second);
}
TEST_P(QuicSpdyClientStreamTest, TestReceiving101) {
diff --git a/quiche/quic/test_tools/quic_test_client.cc b/quiche/quic/test_tools/quic_test_client.cc
index 1f17227..5835800 100644
--- a/quiche/quic/test_tools/quic_test_client.cc
+++ b/quiche/quic/test_tools/quic_test_client.cc
@@ -616,7 +616,6 @@
response_ = "";
response_complete_ = false;
response_headers_complete_ = false;
- preliminary_headers_.clear();
response_headers_.clear();
response_trailers_.clear();
bytes_read_ = 0;
@@ -668,18 +667,6 @@
return &response_headers_;
}
-const spdy::Http2HeaderBlock* QuicTestClient::preliminary_headers() const {
- for (std::pair<QuicStreamId, QuicSpdyClientStream*> stream : open_streams_) {
- size_t bytes_read =
- stream.second->stream_bytes_read() + stream.second->header_bytes_read();
- if (bytes_read > 0) {
- preliminary_headers_ = stream.second->preliminary_headers().Clone();
- break;
- }
- }
- return &preliminary_headers_;
-}
-
const spdy::Http2HeaderBlock& QuicTestClient::response_trailers() const {
return response_trailers_;
}
@@ -737,7 +724,6 @@
client_stream->stream_error(), connected(),
client_stream->headers_decompressed(),
client_stream->response_headers(),
- client_stream->preliminary_headers(),
(buffer_body() ? std::string(client_stream->data()) : ""),
client_stream->received_trailers(),
// Use NumBytesConsumed to avoid counting retransmitted stream frames.
@@ -844,7 +830,6 @@
response_complete(other.response_complete),
response_headers_complete(other.response_headers_complete),
response_headers(other.response_headers.Clone()),
- preliminary_headers(other.preliminary_headers.Clone()),
response(other.response),
response_trailers(other.response_trailers.Clone()),
bytes_read(other.bytes_read),
@@ -855,7 +840,6 @@
QuicRstStreamErrorCode stream_error, bool response_complete,
bool response_headers_complete,
const spdy::Http2HeaderBlock& response_headers,
- const spdy::Http2HeaderBlock& preliminary_headers,
const std::string& response,
const spdy::Http2HeaderBlock& response_trailers, uint64_t bytes_read,
uint64_t bytes_written, int64_t response_body_size)
@@ -863,7 +847,6 @@
response_complete(response_complete),
response_headers_complete(response_headers_complete),
response_headers(response_headers.Clone()),
- preliminary_headers(preliminary_headers.Clone()),
response(response),
response_trailers(response_trailers.Clone()),
bytes_read(bytes_read),
@@ -896,7 +879,6 @@
response_ = state.response;
response_complete_ = state.response_complete;
response_headers_complete_ = state.response_headers_complete;
- preliminary_headers_ = state.preliminary_headers.Clone();
response_headers_ = state.response_headers.Clone();
response_trailers_ = state.response_trailers.Clone();
bytes_read_ = state.bytes_read;
diff --git a/quiche/quic/test_tools/quic_test_client.h b/quiche/quic/test_tools/quic_test_client.h
index 9f2a869..4b95046 100644
--- a/quiche/quic/test_tools/quic_test_client.h
+++ b/quiche/quic/test_tools/quic_test_client.h
@@ -176,7 +176,6 @@
// have received a partial response.
bool response_headers_complete() const;
const spdy::Http2HeaderBlock* response_headers() const;
- const spdy::Http2HeaderBlock* preliminary_headers() const;
int64_t response_size() const;
size_t bytes_read() const;
size_t bytes_written() const;
@@ -358,7 +357,6 @@
PerStreamState(QuicRstStreamErrorCode stream_error, bool response_complete,
bool response_headers_complete,
const spdy::Http2HeaderBlock& response_headers,
- const spdy::Http2HeaderBlock& preliminary_headers,
const std::string& response,
const spdy::Http2HeaderBlock& response_trailers,
uint64_t bytes_read, uint64_t bytes_written,
@@ -369,7 +367,6 @@
bool response_complete;
bool response_headers_complete;
spdy::Http2HeaderBlock response_headers;
- spdy::Http2HeaderBlock preliminary_headers;
std::string response;
spdy::Http2HeaderBlock response_trailers;
uint64_t bytes_read;
@@ -402,7 +399,6 @@
bool response_complete_;
bool response_headers_complete_;
- mutable spdy::Http2HeaderBlock preliminary_headers_;
mutable spdy::Http2HeaderBlock response_headers_;
// Parsed response trailers (if present), copied from the stream in OnClose.
diff --git a/quiche/quic/tools/quic_simple_client_session.cc b/quiche/quic/tools/quic_simple_client_session.cc
index eaaa921..0bb1515 100644
--- a/quiche/quic/tools/quic_simple_client_session.cc
+++ b/quiche/quic/tools/quic_simple_client_session.cc
@@ -36,9 +36,11 @@
std::unique_ptr<QuicSpdyClientStream>
QuicSimpleClientSession::CreateClientStream() {
- return std::make_unique<QuicSimpleClientStream>(
+ auto stream = std::make_unique<QuicSimpleClientStream>(
GetNextOutgoingBidirectionalStreamId(), this, BIDIRECTIONAL,
drop_response_body_);
+ stream->set_on_interim_headers(on_interim_headers_);
+ return stream;
}
bool QuicSimpleClientSession::ShouldNegotiateWebTransport() {
diff --git a/quiche/quic/tools/quic_simple_client_session.h b/quiche/quic/tools/quic_simple_client_session.h
index b172afc..2a0db5d 100644
--- a/quiche/quic/tools/quic_simple_client_session.h
+++ b/quiche/quic/tools/quic_simple_client_session.h
@@ -5,9 +5,13 @@
#ifndef QUICHE_QUIC_TOOLS_QUIC_SIMPLE_CLIENT_SESSION_H_
#define QUICHE_QUIC_TOOLS_QUIC_SIMPLE_CLIENT_SESSION_H_
+#include <functional>
+#include <utility>
+
#include "quiche/quic/core/http/quic_spdy_client_session.h"
#include "quiche/quic/tools/quic_client_base.h"
#include "quiche/quic/tools/quic_simple_client_stream.h"
+#include "quiche/spdy/core/http2_header_block.h"
namespace quic {
@@ -42,7 +46,13 @@
std::unique_ptr<QuicPathValidationContext> context) override;
bool drop_response_body() const { return drop_response_body_; }
+ void set_on_interim_headers(
+ std::function<void(const spdy::Http2HeaderBlock&)> on_interim_headers) {
+ on_interim_headers_ = std::move(on_interim_headers);
+ }
+
private:
+ std::function<void(const spdy::Http2HeaderBlock&)> on_interim_headers_;
QuicClientBase::NetworkHelper* network_helper_;
const bool drop_response_body_;
const bool enable_web_transport_;
diff --git a/quiche/quic/tools/quic_simple_client_stream.cc b/quiche/quic/tools/quic_simple_client_stream.cc
index 14de9f0..76dd39f 100644
--- a/quiche/quic/tools/quic_simple_client_stream.cc
+++ b/quiche/quic/tools/quic_simple_client_stream.cc
@@ -4,6 +4,8 @@
#include "quiche/quic/tools/quic_simple_client_stream.h"
+#include "quiche/common/platform/api/quiche_logging.h"
+
namespace quic {
void QuicSimpleClientStream::OnBodyAvailable() {
@@ -26,4 +28,20 @@
}
}
+bool QuicSimpleClientStream::ParseAndValidateStatusCode() {
+ const size_t num_previous_interim_headers = preliminary_headers().size();
+ if (!QuicSpdyClientStream::ParseAndValidateStatusCode()) {
+ return false;
+ }
+ // The base ParseAndValidateStatusCode() may have added a preliminary header.
+ if (preliminary_headers().size() > num_previous_interim_headers) {
+ QUICHE_DCHECK_EQ(preliminary_headers().size(),
+ num_previous_interim_headers + 1);
+ if (on_interim_headers_) {
+ on_interim_headers_(preliminary_headers().back());
+ }
+ }
+ return true;
+}
+
} // namespace quic
diff --git a/quiche/quic/tools/quic_simple_client_stream.h b/quiche/quic/tools/quic_simple_client_stream.h
index 976ceaf..fd7ede4 100644
--- a/quiche/quic/tools/quic_simple_client_stream.h
+++ b/quiche/quic/tools/quic_simple_client_stream.h
@@ -5,6 +5,9 @@
#ifndef QUICHE_QUIC_TOOLS_QUIC_SIMPLE_CLIENT_STREAM_H_
#define QUICHE_QUIC_TOOLS_QUIC_SIMPLE_CLIENT_STREAM_H_
+#include <functional>
+#include <utility>
+
#include "quiche/quic/core/http/quic_spdy_client_stream.h"
namespace quic {
@@ -17,7 +20,16 @@
drop_response_body_(drop_response_body) {}
void OnBodyAvailable() override;
+ void set_on_interim_headers(
+ std::function<void(const spdy::Http2HeaderBlock&)> on_interim_headers) {
+ on_interim_headers_ = std::move(on_interim_headers);
+ }
+
+ protected:
+ bool ParseAndValidateStatusCode() override;
+
private:
+ std::function<void(const spdy::Http2HeaderBlock&)> on_interim_headers_;
const bool drop_response_body_;
};
diff --git a/quiche/quic/tools/quic_spdy_client_base.cc b/quiche/quic/tools/quic_spdy_client_base.cc
index b46e780..248e379 100644
--- a/quiche/quic/tools/quic_spdy_client_base.cc
+++ b/quiche/quic/tools/quic_spdy_client_base.cc
@@ -7,6 +7,7 @@
#include <utility>
#include "absl/strings/numbers.h"
+#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"
#include "quiche/quic/core/crypto/quic_random.h"
#include "quiche/quic/core/http/spdy_utils.h"
@@ -86,8 +87,10 @@
QUIC_LOG(ERROR) << "Invalid :status response header: " << status->second;
}
latest_response_headers_ = response_headers.DebugString();
- preliminary_response_headers_ =
- client_stream->preliminary_headers().DebugString();
+ for (const Http2HeaderBlock& headers :
+ client_stream->preliminary_headers()) {
+ absl::StrAppend(&preliminary_response_headers_, headers.DebugString());
+ }
latest_response_header_block_ = response_headers.Clone();
latest_response_body_ = std::string(client_stream->data());
latest_response_trailers_ =
