Switch WebTransport from using tokens to using strings
See https://github.com/ietf-wg-webtrans/draft-ietf-webtrans-http3/pull/181
This is a backwards-incompatible change
PiperOrigin-RevId: 773667376
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc
index cc935f0..7b4ae68 100644
--- a/quiche/quic/core/http/end_to_end_test.cc
+++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -7327,7 +7327,7 @@
WebTransportHttp3* session = CreateWebTransportSession(
"/selected-subprotocol", /*wait_for_server_response=*/true,
- {{webtransport::kSubprotocolRequestHeader, "a, b, c, d"},
+ {{webtransport::kSubprotocolRequestHeader, R"("a", "b", "c", "d")"},
{"subprotocol-index", "1"}});
ASSERT_NE(session, nullptr);
NiceMock<MockWebTransportSessionVisitor>& visitor =
diff --git a/quiche/quic/core/http/quic_spdy_stream_test.cc b/quiche/quic/core/http/quic_spdy_stream_test.cc
index 1536a57..95e3ffb 100644
--- a/quiche/quic/core/http/quic_spdy_stream_test.cc
+++ b/quiche/quic/core/http/quic_spdy_stream_test.cc
@@ -3309,7 +3309,7 @@
quiche::HttpHeaderBlock request_headers;
request_headers[":method"] = "CONNECT";
request_headers[":protocol"] = "webtransport";
- request_headers["wt-available-protocols"] = "moqt-00, moqt-01; foo=bar";
+ request_headers["wt-available-protocols"] = R"("moqt-00", "moqt-01"; a=b)";
stream_->WriteHeaders(std::move(request_headers), /*fin=*/false, nullptr);
ASSERT_TRUE(stream_->web_transport() != nullptr);
EXPECT_EQ(stream_->id(), stream_->web_transport()->id());
@@ -3318,7 +3318,7 @@
quiche::HttpHeaderBlock response_headers;
response_headers[":status"] = "200";
- response_headers["wt-protocol"] = "moqt-01";
+ response_headers["wt-protocol"] = "\"moqt-01\"";
stream_->web_transport()->HeadersReceived(response_headers);
EXPECT_EQ(stream_->web_transport()->rejection_reason(),
WebTransportHttp3RejectionReason::kNone);
@@ -3345,13 +3345,13 @@
quiche::HttpHeaderBlock request_headers;
request_headers[":method"] = "CONNECT";
request_headers[":protocol"] = "webtransport";
- request_headers["wt-available-protocols"] = "moqt-00, moqt-01; foo=bar";
+ request_headers["wt-available-protocols"] = R"("moqt-00", "moqt-01"; a=b)";
stream_->WriteHeaders(std::move(request_headers), /*fin=*/false, nullptr);
ASSERT_TRUE(stream_->web_transport() != nullptr);
quiche::HttpHeaderBlock response_headers;
response_headers[":status"] = "200";
- response_headers["wt-protocol"] = "moqt-02";
+ response_headers["wt-protocol"] = "\"moqt-02\"";
stream_->web_transport()->HeadersReceived(response_headers);
EXPECT_EQ(stream_->web_transport()->rejection_reason(),
WebTransportHttp3RejectionReason::kSubprotocolMismatch);
@@ -3378,7 +3378,7 @@
quiche::HttpHeaderBlock request_headers;
request_headers[":method"] = "CONNECT";
request_headers[":protocol"] = "webtransport";
- request_headers["wt-available-protocols"] = "moqt-00, moqt-01; foo=bar";
+ request_headers["wt-available-protocols"] = R"("moqt-00", "moqt-01"; a=b)";
stream_->WriteHeaders(std::move(request_headers), /*fin=*/false, nullptr);
ASSERT_TRUE(stream_->web_transport() != nullptr);
@@ -3405,7 +3405,7 @@
headers_[":method"] = "CONNECT";
headers_[":protocol"] = "webtransport";
- headers_["wt-available-protocols"] = "moqt-00, moqt-01; foo=bar";
+ headers_["wt-available-protocols"] = R"("moqt-00", "moqt-01"; a=b)";
stream_->OnStreamHeadersPriority(
spdy::SpdyStreamPrecedence(kV3HighestPriority));
@@ -3421,7 +3421,7 @@
.Times(AnyNumber());
quiche::HttpHeaderBlock response_headers;
response_headers[":status"] = "200";
- response_headers["wt-protocol"] = "moqt-01";
+ response_headers["wt-protocol"] = "\"moqt-01\"";
stream_->WriteHeaders(std::move(response_headers), /*fin=*/false, nullptr);
EXPECT_EQ(stream_->web_transport()->GetNegotiatedSubprotocol(), "moqt-01");
}
diff --git a/quiche/quic/test_tools/quic_test_backend.cc b/quiche/quic/test_tools/quic_test_backend.cc
index c40db49..c901984 100644
--- a/quiche/quic/test_tools/quic_test_backend.cc
+++ b/quiche/quic/test_tools/quic_test_backend.cc
@@ -183,10 +183,18 @@
return response;
}
}
+ absl::StatusOr<std::string> response_subprotocol =
+ webtransport::SerializeSubprotocolResponseHeader(
+ (*subprotocols)[subprotocol_index]);
+ if (!response_subprotocol.ok()) {
+ WebTransportResponse response;
+ response.response_headers[":status"] = "400";
+ return response;
+ }
WebTransportResponse response;
response.response_headers[":status"] = "200";
response.response_headers[webtransport::kSubprotocolResponseHeader] =
- (*subprotocols)[subprotocol_index];
+ *response_subprotocol;
response.visitor = std::make_unique<SubprotocolStreamVisitor>(session);
return response;
}
diff --git a/quiche/web_transport/web_transport_headers.cc b/quiche/web_transport/web_transport_headers.cc
index 2ae7bf1..ed99cac 100644
--- a/quiche/web_transport/web_transport_headers.cc
+++ b/quiche/web_transport/web_transport_headers.cc
@@ -11,12 +11,13 @@
#include <utility>
#include <vector>
+#include "absl/algorithm/container.h"
#include "absl/base/attributes.h"
#include "absl/container/flat_hash_set.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
+#include "absl/strings/ascii.h"
#include "absl/strings/str_cat.h"
-#include "absl/strings/str_join.h"
#include "absl/strings/string_view.h"
#include "absl/types/span.h"
#include "quiche/common/quiche_status_utils.h"
@@ -70,7 +71,7 @@
std::vector<std::string> result;
result.reserve(parsed->size());
for (ParameterizedMember& member : *parsed) {
- QUICHE_RETURN_IF_ERROR(CheckMemberType(member, Item::kTokenType));
+ QUICHE_RETURN_IF_ERROR(CheckMemberType(member, Item::kStringType));
result.push_back(std::move(member.member[0].item).TakeString());
}
return result;
@@ -78,14 +79,18 @@
absl::StatusOr<std::string> SerializeSubprotocolRequestHeader(
absl::Span<const std::string> subprotocols) {
- // Serialize tokens manually via a simple StrJoin call; this lets us provide
- // better error messages, and is probably more efficient too.
- for (const std::string& token : subprotocols) {
- if (!quiche::structured_headers::IsValidToken(token)) {
- return absl::InvalidArgumentError(absl::StrCat("Invalid token: ", token));
- }
+ quiche::structured_headers::List list;
+ list.reserve(subprotocols.size());
+ for (const std::string& subprotocol : subprotocols) {
+ list.push_back(ParameterizedMember(Item(subprotocol), {}));
}
- return absl::StrJoin(subprotocols, ", ");
+
+ std::optional<std::string> serialized =
+ quiche::structured_headers::SerializeList(list);
+ if (!serialized.has_value()) {
+ return absl::InvalidArgumentError("Invalid subprotocol list supplied");
+ }
+ return *std::move(serialized);
}
absl::StatusOr<std::string> ParseSubprotocolResponseHeader(
@@ -95,20 +100,24 @@
if (!parsed.has_value()) {
return absl::InvalidArgumentError("Failed to parse sf-item");
}
- QUICHE_RETURN_IF_ERROR(CheckItemType(*parsed, Item::kTokenType));
+ QUICHE_RETURN_IF_ERROR(CheckItemType(*parsed, Item::kStringType));
return std::move(parsed->item).TakeString();
}
absl::StatusOr<std::string> SerializeSubprotocolResponseHeader(
absl::string_view subprotocol) {
- if (!quiche::structured_headers::IsValidToken(subprotocol)) {
- return absl::InvalidArgumentError("Invalid token value supplied");
+ Item item(std::string(subprotocol), Item::kStringType);
+ std::optional<std::string> serialized =
+ quiche::structured_headers::SerializeItem(item);
+ if (!serialized.has_value()) {
+ return absl::InvalidArgumentError("Invalid subprotocol name supplied");
}
- return std::string(subprotocol);
+ return *std::move(serialized);
}
bool ValidateSubprotocolName(absl::string_view name) {
- return !name.empty() && quiche::structured_headers::IsValidToken(name);
+ return !name.empty() &&
+ absl::c_all_of(name, [](char c) { return absl::ascii_isprint(c); });
}
template <typename S>
diff --git a/quiche/web_transport/web_transport_headers_test.cc b/quiche/web_transport/web_transport_headers_test.cc
index d85e1f8..da910d0 100644
--- a/quiche/web_transport/web_transport_headers_test.cc
+++ b/quiche/web_transport/web_transport_headers_test.cc
@@ -22,27 +22,30 @@
using ::testing::HasSubstr;
TEST(WebTransportHeaders, ParseSubprotocolRequestHeader) {
- EXPECT_THAT(ParseSubprotocolRequestHeader("test"),
- IsOkAndHolds(ElementsAre("test")));
- EXPECT_THAT(ParseSubprotocolRequestHeader("moqt-draft01, moqt-draft02"),
- IsOkAndHolds(ElementsAre("moqt-draft01", "moqt-draft02")));
- EXPECT_THAT(ParseSubprotocolRequestHeader("moqt-draft01; a=b, moqt-draft02"),
- IsOkAndHolds(ElementsAre("moqt-draft01", "moqt-draft02")));
- EXPECT_THAT(ParseSubprotocolRequestHeader("moqt-draft01, moqt-draft02; a=b"),
- IsOkAndHolds(ElementsAre("moqt-draft01", "moqt-draft02")));
EXPECT_THAT(ParseSubprotocolRequestHeader("\"test\""),
+ IsOkAndHolds(ElementsAre("test")));
+ EXPECT_THAT(
+ ParseSubprotocolRequestHeader(R"("moqt-draft01", "moqt-draft02")"),
+ IsOkAndHolds(ElementsAre("moqt-draft01", "moqt-draft02")));
+ EXPECT_THAT(
+ ParseSubprotocolRequestHeader(R"("moqt-draft01"; a=b, "moqt-draft02")"),
+ IsOkAndHolds(ElementsAre("moqt-draft01", "moqt-draft02")));
+ EXPECT_THAT(
+ ParseSubprotocolRequestHeader(R"("moqt-draft01", "moqt-draft02"; a=b)"),
+ IsOkAndHolds(ElementsAre("moqt-draft01", "moqt-draft02")));
+ EXPECT_THAT(ParseSubprotocolRequestHeader("test"),
StatusIs(absl::StatusCode::kInvalidArgument,
- HasSubstr("found string instead")));
+ HasSubstr("found token instead")));
EXPECT_THAT(ParseSubprotocolRequestHeader("42"),
StatusIs(absl::StatusCode::kInvalidArgument,
HasSubstr("found integer instead")));
- EXPECT_THAT(ParseSubprotocolRequestHeader("a, (b)"),
+ EXPECT_THAT(ParseSubprotocolRequestHeader("\"a\", (b)"),
StatusIs(absl::StatusCode::kInvalidArgument,
HasSubstr("found a nested list instead")));
- EXPECT_THAT(ParseSubprotocolRequestHeader("a, (b c)"),
+ EXPECT_THAT(ParseSubprotocolRequestHeader("\"a\", (b c)"),
StatusIs(absl::StatusCode::kInvalidArgument,
HasSubstr("found a nested list instead")));
- EXPECT_THAT(ParseSubprotocolRequestHeader("foo, ?1, bar"),
+ EXPECT_THAT(ParseSubprotocolRequestHeader("\"foo\", ?1, bar"),
StatusIs(absl::StatusCode::kInvalidArgument,
HasSubstr("found boolean instead")));
EXPECT_THAT(ParseSubprotocolRequestHeader("(a"),
@@ -52,19 +55,19 @@
TEST(WebTransportHeaders, SerializeSubprotocolRequestHeader) {
EXPECT_THAT(SerializeSubprotocolRequestHeader({"test"}),
- IsOkAndHolds("test"));
+ IsOkAndHolds(R"("test")"));
EXPECT_THAT(SerializeSubprotocolRequestHeader({"foo", "bar"}),
- IsOkAndHolds("foo, bar"));
- EXPECT_THAT(SerializeSubprotocolRequestHeader({"moqt-draft01", "a/b/c"}),
- IsOkAndHolds("moqt-draft01, a/b/c"));
- EXPECT_THAT(
- SerializeSubprotocolRequestHeader({"abcd", "0123", "efgh"}),
- StatusIs(absl::StatusCode::kInvalidArgument, "Invalid token: 0123"));
+ IsOkAndHolds(R"("foo", "bar")"));
+ EXPECT_THAT(SerializeSubprotocolRequestHeader({"a\"b", "a/b/c"}),
+ IsOkAndHolds(R"("a\"b", "a/b/c")"));
+ EXPECT_THAT(SerializeSubprotocolRequestHeader({"abcd", "\n", "efgh"}),
+ StatusIs(absl::StatusCode::kInvalidArgument));
}
TEST(WebTransportHeader, ParseSubprotocolResponseHeader) {
- EXPECT_THAT(ParseSubprotocolResponseHeader("foo"), IsOkAndHolds("foo"));
- EXPECT_THAT(ParseSubprotocolResponseHeader("foo; a=b"), IsOkAndHolds("foo"));
+ EXPECT_THAT(ParseSubprotocolResponseHeader("\"foo\""), IsOkAndHolds("foo"));
+ EXPECT_THAT(ParseSubprotocolResponseHeader("\"foo\"; a=b"),
+ IsOkAndHolds("foo"));
EXPECT_THAT(
ParseSubprotocolResponseHeader("1234"),
StatusIs(absl::StatusCode::kInvalidArgument, HasSubstr("found integer")));
@@ -74,10 +77,11 @@
}
TEST(WebTransportHeader, SerializeSubprotocolResponseHeader) {
- EXPECT_THAT(SerializeSubprotocolResponseHeader("foo"), IsOkAndHolds("foo"));
+ EXPECT_THAT(SerializeSubprotocolResponseHeader("foo"),
+ IsOkAndHolds("\"foo\""));
EXPECT_THAT(SerializeSubprotocolResponseHeader("moqt-draft01"),
- IsOkAndHolds("moqt-draft01"));
- EXPECT_THAT(SerializeSubprotocolResponseHeader("123abc"),
+ IsOkAndHolds("\"moqt-draft01\""));
+ EXPECT_THAT(SerializeSubprotocolResponseHeader("\xff"),
StatusIs(absl::StatusCode::kInvalidArgument));
}
@@ -133,12 +137,13 @@
TEST(WebTransportHeaders, ValidateSubprotocolName) {
EXPECT_TRUE(ValidateSubprotocolName("test"));
- EXPECT_FALSE(ValidateSubprotocolName("123"));
+ EXPECT_TRUE(ValidateSubprotocolName("123"));
+ EXPECT_FALSE(ValidateSubprotocolName("\n"));
EXPECT_FALSE(ValidateSubprotocolName(""));
EXPECT_TRUE(ValidateSubprotocolListHelper({}));
EXPECT_TRUE(ValidateSubprotocolListHelper({"a", "b", "c"}));
- EXPECT_FALSE(ValidateSubprotocolListHelper({"a", "1", "c"}));
+ EXPECT_FALSE(ValidateSubprotocolListHelper({"a", "\n", "c"}));
EXPECT_FALSE(ValidateSubprotocolListHelper({"a", "b", "a"}));
std::vector<std::string> vec = {"a", "b"};
