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"};