Validates request paths according to RFC 3986 Section 3.3.
PiperOrigin-RevId: 535379163
diff --git a/quiche/http2/adapter/header_validator.cc b/quiche/http2/adapter/header_validator.cc
index e474558..2119b64 100644
--- a/quiche/http2/adapter/header_validator.cc
+++ b/quiche/http2/adapter/header_validator.cc
@@ -28,6 +28,10 @@
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._~%!$&'()["
"]*+,;=:";
+const absl::string_view kValidPathChars =
+ "/abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._~%!$&'()"
+ "*+,;=:@?";
+
using CharMap = std::array<bool, 256>;
CharMap BuildValidCharMap(absl::string_view valid_chars) {
@@ -177,8 +181,7 @@
} else if (key == ":authority" && !ValidateAndSetAuthority(value)) {
return HEADER_FIELD_INVALID;
} else if (key == ":path") {
- if (value.empty()) {
- // For now, reject an empty path regardless of scheme.
+ if (value.empty() || !IsValidPath(value)) {
return HEADER_FIELD_INVALID;
}
path_ = std::string(value);
@@ -252,6 +255,11 @@
return AllCharsInMap(authority, valid_chars);
}
+bool HeaderValidator::IsValidPath(absl::string_view path) {
+ static const CharMap valid_chars = BuildValidCharMap(kValidPathChars);
+ return AllCharsInMap(path, valid_chars);
+}
+
HeaderValidator::ContentLengthStatus HeaderValidator::HandleContentLength(
absl::string_view value) {
if (value.empty()) {
diff --git a/quiche/http2/adapter/header_validator.h b/quiche/http2/adapter/header_validator.h
index a6070db..c6eb3a4 100644
--- a/quiche/http2/adapter/header_validator.h
+++ b/quiche/http2/adapter/header_validator.h
@@ -33,6 +33,10 @@
// Returns whether `authority` is valid according to RFC 3986 Section 3.2.
static bool IsValidAuthority(absl::string_view authority);
+ // Returns whether `path` is valid according to RFC 3986 Section 3.3. May
+ // contain the query part of a URI.
+ static bool IsValidPath(absl::string_view path);
+
private:
enum ContentLengthStatus {
CONTENT_LENGTH_OK,
diff --git a/quiche/http2/adapter/header_validator_test.cc b/quiche/http2/adapter/header_validator_test.cc
index 6aaf974..3727003 100644
--- a/quiche/http2/adapter/header_validator_test.cc
+++ b/quiche/http2/adapter/header_validator_test.cc
@@ -457,7 +457,8 @@
for (Header to_add : kSampleRequestPseudoheaders) {
if (to_add.first == ":path") {
EXPECT_EQ(HeaderValidator::HEADER_OK,
- validator.ValidateSingleHeader(to_add.first, value));
+ validator.ValidateSingleHeader(to_add.first, value))
+ << "Problematic char: [" << c << "]";
} else {
EXPECT_EQ(HeaderValidator::HEADER_OK,
validator.ValidateSingleHeader(to_add.first, to_add.second));
@@ -466,7 +467,7 @@
EXPECT_TRUE(validator.FinishHeaderBlock(HeaderType::REQUEST));
}
- // BUG: Various invalid path characters.
+ // Various invalid path characters.
for (const absl::string_view c : {"[", "<", "}", "`", "\\", " ", "\t"}) {
const std::string value = absl::StrCat("/shawa", c, "rma");
@@ -474,14 +475,14 @@
validator.StartHeaderBlock();
for (Header to_add : kSampleRequestPseudoheaders) {
if (to_add.first == ":path") {
- EXPECT_EQ(HeaderValidator::HEADER_OK,
+ EXPECT_EQ(HeaderValidator::HEADER_FIELD_INVALID,
validator.ValidateSingleHeader(to_add.first, value));
} else {
EXPECT_EQ(HeaderValidator::HEADER_OK,
validator.ValidateSingleHeader(to_add.first, to_add.second));
}
}
- EXPECT_TRUE(validator.FinishHeaderBlock(HeaderType::REQUEST));
+ EXPECT_FALSE(validator.FinishHeaderBlock(HeaderType::REQUEST));
}
}
diff --git a/quiche/http2/adapter/oghttp2_adapter_test.cc b/quiche/http2/adapter/oghttp2_adapter_test.cc
index 9b3c193..c6f4d47 100644
--- a/quiche/http2/adapter/oghttp2_adapter_test.cc
+++ b/quiche/http2/adapter/oghttp2_adapter_test.cc
@@ -171,18 +171,19 @@
EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "GET"));
EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https"));
EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com"));
- EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/ fragment"));
- EXPECT_CALL(visitor, OnEndHeadersForStream(1));
- EXPECT_CALL(visitor, OnEndStream(1));
+ EXPECT_CALL(
+ visitor,
+ OnInvalidFrame(1, Http2VisitorInterface::InvalidFrameError::kHttpHeader));
+
// Stream 3
EXPECT_CALL(visitor, OnFrameHeader(3, _, HEADERS, 5));
EXPECT_CALL(visitor, OnBeginHeadersForStream(3));
EXPECT_CALL(visitor, OnHeaderForStream(3, ":method", "GET"));
EXPECT_CALL(visitor, OnHeaderForStream(3, ":scheme", "https"));
EXPECT_CALL(visitor, OnHeaderForStream(3, ":authority", "example.com"));
- EXPECT_CALL(visitor, OnHeaderForStream(3, ":path", "/\tfragment2"));
- EXPECT_CALL(visitor, OnEndHeadersForStream(3));
- EXPECT_CALL(visitor, OnEndStream(3));
+ EXPECT_CALL(
+ visitor,
+ OnInvalidFrame(3, Http2VisitorInterface::InvalidFrameError::kHttpHeader));
const int64_t result = adapter->ProcessBytes(frames);
EXPECT_EQ(frames.size(), static_cast<size_t>(result));
