This is an automated email from the ASF dual-hosted git repository.

bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 83886d69b2 Extracted pseudo header validation logic to use in both h2 
and h3 (#10139)
83886d69b2 is described below

commit 83886d69b2625124ac9dffa541f0d774a0ba2bee
Author: Zhengxi Li <lzx404...@hotmail.com>
AuthorDate: Thu Aug 3 13:01:49 2023 -0400

    Extracted pseudo header validation logic to use in both h2 and h3 (#10139)
    
    This resolves #9994. Note that in h3, the request with the header failing 
the validation won't be forwarded to the server as expected, but ATS doesn't 
send out a error response as it should due to #10138 .
---
 proxy/hdrs/CMakeLists.txt                     |   1 +
 proxy/hdrs/HeaderValidator.cc                 | 135 ++++++++++++++
 proxy/hdrs/HeaderValidator.h                  |  32 ++++
 proxy/hdrs/Makefile.am                        |   7 +-
 proxy/hdrs/unit_tests/test_HeaderValidator.cc | 250 ++++++++++++++++++++++++++
 proxy/http2/HTTP2.cc                          | 107 +----------
 proxy/http3/Http3HeaderVIOAdaptor.cc          |   8 +
 7 files changed, 434 insertions(+), 106 deletions(-)

diff --git a/proxy/hdrs/CMakeLists.txt b/proxy/hdrs/CMakeLists.txt
index e4ae8ca60d..b27ac2dd20 100644
--- a/proxy/hdrs/CMakeLists.txt
+++ b/proxy/hdrs/CMakeLists.txt
@@ -28,6 +28,7 @@ add_library(hdrs STATIC
         VersionConverter.cc
         HuffmanCodec.cc
         XPACK.cc
+        HeaderValidator.cc
 )
 add_library(ts::hdrs ALIAS hdrs)
 
diff --git a/proxy/hdrs/HeaderValidator.cc b/proxy/hdrs/HeaderValidator.cc
new file mode 100644
index 0000000000..77abec7d2a
--- /dev/null
+++ b/proxy/hdrs/HeaderValidator.cc
@@ -0,0 +1,135 @@
+/** @file
+ *
+ *  A brief file description
+ *
+ *  @section license License
+ *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+#include "HeaderValidator.h"
+#include "HTTP.h"
+
+bool
+HeaderValidator::is_h2_h3_header_valid(const HTTPHdr &hdr, bool is_response, 
bool is_trailing_header)
+{
+  const MIMEField *field = nullptr;
+  const char *name       = nullptr;
+  int name_len           = 0;
+  const char *value      = nullptr;
+  int value_len          = 0;
+  MIMEFieldIter iter;
+  auto method_field       = hdr.field_find(PSEUDO_HEADER_METHOD.data(), 
PSEUDO_HEADER_METHOD.size());
+  bool has_connect_method = false;
+  if (method_field) {
+    int method_len;
+    const char *method_value = method_field->value_get(&method_len);
+    has_connect_method       = method_len == HTTP_LEN_CONNECT && 
strncmp(HTTP_METHOD_CONNECT, method_value, HTTP_LEN_CONNECT) == 0;
+  }
+  unsigned int expected_pseudo_header_count = is_response ? 1 : 
has_connect_method ? 2 : 4;
+  unsigned int pseudo_header_count          = 0;
+
+  if (is_trailing_header) {
+    expected_pseudo_header_count = 0;
+  }
+  for (auto &field : hdr) {
+    name = field.name_get(&name_len);
+    // Pseudo headers must appear before regular headers
+    if (name_len && name[0] == ':') {
+      ++pseudo_header_count;
+      if (pseudo_header_count > expected_pseudo_header_count) {
+        return false;
+      }
+    } else if (name_len <= 0) {
+      return false;
+    } else {
+      if (pseudo_header_count != expected_pseudo_header_count) {
+        return false;
+      }
+    }
+  }
+
+  // rfc7540,sec8.1.2.2 and rfc9114,sec4.2: Any message containing
+  // connection-specific header fields MUST be treated as malformed.
+  if (hdr.field_find(MIME_FIELD_CONNECTION, MIME_LEN_CONNECTION) != nullptr ||
+      hdr.field_find(MIME_FIELD_KEEP_ALIVE, MIME_LEN_KEEP_ALIVE) != nullptr ||
+      hdr.field_find(MIME_FIELD_PROXY_CONNECTION, MIME_LEN_PROXY_CONNECTION) 
!= nullptr ||
+      hdr.field_find(MIME_FIELD_UPGRADE, MIME_LEN_UPGRADE) != nullptr) {
+    return false;
+  }
+
+  // :path pseudo header MUST NOT empty for http or https URIs
+  field = hdr.field_find(PSEUDO_HEADER_PATH.data(), PSEUDO_HEADER_PATH.size());
+  if (field) {
+    field->value_get(&value_len);
+    if (value_len == 0) {
+      return false;
+    }
+  }
+
+  // when The TE header field is received, it MUST NOT contain any
+  // value other than "trailers".
+  field = hdr.field_find(MIME_FIELD_TE, MIME_LEN_TE);
+  if (field) {
+    value = field->value_get(&value_len);
+    if (!(value_len == 8 && memcmp(value, "trailers", 8) == 0)) {
+      return false;
+    }
+  }
+
+  if (is_trailing_header) {
+    // Done with validation for trailing headers, which doesn't have any 
pseudo headers.
+    return true;
+  }
+  // Check pseudo headers
+  if (is_response) {
+    if (hdr.fields_count() >= 1) {
+      if (hdr.field_find(PSEUDO_HEADER_STATUS.data(), 
PSEUDO_HEADER_STATUS.size()) == nullptr) {
+        return false;
+      }
+    } else {
+      // There should at least be :status pseudo header.
+      return false;
+    }
+  } else {
+    // This is a request.
+    if (!has_connect_method && hdr.fields_count() >= 4) {
+      if (hdr.field_find(PSEUDO_HEADER_SCHEME.data(), 
PSEUDO_HEADER_SCHEME.size()) == nullptr ||
+          hdr.field_find(PSEUDO_HEADER_METHOD.data(), 
PSEUDO_HEADER_METHOD.size()) == nullptr ||
+          hdr.field_find(PSEUDO_HEADER_PATH.data(), PSEUDO_HEADER_PATH.size()) 
== nullptr ||
+          hdr.field_find(PSEUDO_HEADER_AUTHORITY.data(), 
PSEUDO_HEADER_AUTHORITY.size()) == nullptr ||
+          hdr.field_find(PSEUDO_HEADER_STATUS.data(), 
PSEUDO_HEADER_STATUS.size()) != nullptr) {
+        // Decoded header field is invalid
+        return false;
+      }
+    } else if (has_connect_method && hdr.fields_count() >= 2) {
+      if (hdr.field_find(PSEUDO_HEADER_SCHEME.data(), 
PSEUDO_HEADER_SCHEME.size()) != nullptr ||
+          hdr.field_find(PSEUDO_HEADER_METHOD.data(), 
PSEUDO_HEADER_METHOD.size()) == nullptr ||
+          hdr.field_find(PSEUDO_HEADER_PATH.data(), PSEUDO_HEADER_PATH.size()) 
!= nullptr ||
+          hdr.field_find(PSEUDO_HEADER_AUTHORITY.data(), 
PSEUDO_HEADER_AUTHORITY.size()) == nullptr ||
+          hdr.field_find(PSEUDO_HEADER_STATUS.data(), 
PSEUDO_HEADER_STATUS.size()) != nullptr) {
+        // Decoded header field is invalid
+        return false;
+      }
+
+    } else {
+      // Pseudo headers is insufficient
+      return false;
+    }
+  }
+  return true;
+}
diff --git a/proxy/hdrs/HeaderValidator.h b/proxy/hdrs/HeaderValidator.h
new file mode 100644
index 0000000000..859a88b92e
--- /dev/null
+++ b/proxy/hdrs/HeaderValidator.h
@@ -0,0 +1,32 @@
+/** @file
+ *
+ *  A brief file description
+ *
+ *  @section license License
+ *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+#pragma once
+
+class HTTPHdr;
+
+class HeaderValidator
+{
+public:
+  static bool is_h2_h3_header_valid(const HTTPHdr &hdr, bool is_response, bool 
is_trailing_header);
+};
diff --git a/proxy/hdrs/Makefile.am b/proxy/hdrs/Makefile.am
index 70b9c2e118..ad7bc804ad 100644
--- a/proxy/hdrs/Makefile.am
+++ b/proxy/hdrs/Makefile.am
@@ -50,7 +50,9 @@ libhdrs_a_SOURCES = \
        HuffmanCodec.cc \
        HuffmanCodec.h \
        XPACK.cc \
-       XPACK.h
+       XPACK.h \
+       HeaderValidator.cc \
+       HeaderValidator.h
 
 load_http_hdr_SOURCES = \
        HTTP.h \
@@ -80,7 +82,8 @@ test_proxy_hdrs_SOURCES = \
        unit_tests/test_Hdrs.cc \
        unit_tests/test_HdrUtils.cc \
        unit_tests/test_URL.cc \
-       unit_tests/test_mime.cc
+       unit_tests/test_mime.cc \
+       unit_tests/test_HeaderValidator.cc
 
 test_proxy_hdrs_LDFLAGS = @AM_LDFLAGS@ @SWOC_LDFLAGS@ @YAMLCPP_LDFLAGS@ 
@OPENSSL_LDFLAGS@
 test_proxy_hdrs_LDADD = \
diff --git a/proxy/hdrs/unit_tests/test_HeaderValidator.cc 
b/proxy/hdrs/unit_tests/test_HeaderValidator.cc
new file mode 100644
index 0000000000..0675ffa4cf
--- /dev/null
+++ b/proxy/hdrs/unit_tests/test_HeaderValidator.cc
@@ -0,0 +1,250 @@
+/** @file
+ *
+ *  Catch-based unit tests for the HeaderValidator class
+ *
+ *  @section license License
+ *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+#include "catch.hpp"
+
+#include "HTTP.h"
+#include "HeaderValidator.h"
+#include <string_view>
+#include <vector>
+
+namespace
+{
+using Fields_type              = std::vector<std::pair<std::string, 
std::string>>;
+constexpr bool IS_VALID_HEADER = true;
+
+void
+add_field_value_to_hdr(HTTPHdr &hdr, std::string_view field_name, 
std::string_view field_value)
+{
+  MIMEField *new_field = hdr.field_create(field_name.data(), 
field_name.size());
+  new_field->value_set(hdr.m_heap, hdr.m_mime, field_value.data(), 
field_value.size());
+  hdr.field_attach(new_field);
+}
+
+void
+check_header(const Fields_type &fields, HTTPHdr &hdr, bool expectation, bool 
is_trailer = false)
+{
+  for (auto &field : fields) {
+    add_field_value_to_hdr(hdr, field.first, field.second);
+  }
+  auto ret = HeaderValidator::is_h2_h3_header_valid(hdr, hdr.type_get() == 
HTTP_TYPE_RESPONSE, is_trailer);
+  REQUIRE(ret == expectation);
+}
+} // end anonymous namespace
+
+TEST_CASE("testIsHeaderValid", "[proxy][hdrtest]")
+{
+  HTTPHdr hdr;
+  // extra to prevent proxy allocation.
+  HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64);
+
+  SECTION("Test (valid) request with 4 required pseudo headers")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method",    "GET"         },
+      {":scheme",    "https"       },
+      {":authority", "www.this.com"},
+      {":path",      "/some/path"  },
+    };
+    check_header(fields, hdr, IS_VALID_HEADER);
+  }
+  SECTION("Test request with missing method field")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":scheme",    "https"       },
+      {":authority", "www.this.com"},
+      {":path",      "/some/path"  },
+    };
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  SECTION("Test request with missing authority field")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method", "GET"       },
+      {":scheme", "https"     },
+      {":path",   "/some/path"},
+    };
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  SECTION("Test request with missing scheme field")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method",    "GET"         },
+      {":authority", "www.this.com"},
+      {":path",      "/some/path"  },
+    };
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  SECTION("Test request with missing path field")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method",    "GET"         },
+      {":scheme",    "https"       },
+      {":authority", "www.this.com"},
+    };
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  SECTION("Test request with extra pseudo headers")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method",    "GET"         },
+      {":scheme",    "https"       },
+      {":authority", "www.this.com"},
+      {":path",      "/some/path"  },
+      {":extra",     "abc"         },
+    };
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  SECTION("Test CONNECT request with all required fields")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method",    "CONNECT"     },
+      {":authority", "www.this.com"},
+      {"extra",      "abc"         }
+    };
+    check_header(fields, hdr, IS_VALID_HEADER);
+  }
+  SECTION("Test CONNECT request with disallowed :scheme field")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method",    "CONNECT"     },
+      {":authority", "www.this.com"},
+      {":scheme",    "https"       },
+      {"extra",      "abc"         }
+    };
+    // :scheme and :path should be omitted in CONNECT requests.
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  SECTION("Test CONNECT request with disallowed :path field")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method",    "CONNECT"     },
+      {":authority", "www.this.com"},
+      {":path",      "/some/path"  },
+      {"extra",      "abc"         }
+    };
+    // :scheme and :path should be omitted in CONNECT requests.
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  SECTION("Test (valid) response with only the status field")
+  {
+    hdr.create(HTTP_TYPE_RESPONSE, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":status", "200"},
+    };
+    check_header(fields, hdr, IS_VALID_HEADER);
+  }
+
+  SECTION("Test response with more than the status field")
+  {
+    hdr.create(HTTP_TYPE_RESPONSE, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":status", "200"},
+      {":method", "GET"},
+    };
+    // Response headers cannot have pseudo headers other than :status.
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  SECTION("Test response with no status field")
+  {
+    hdr.create(HTTP_TYPE_RESPONSE, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method", "GET"},
+    };
+    // Response headers must contain :status.
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  SECTION("Test (invalid) trailer header with pseudo-header field")
+  {
+    hdr.create(HTTP_TYPE_RESPONSE, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":status", "500"},
+    };
+    static constexpr bool IS_TRAILER = true;
+    // Trailer headers may not contain any pseudo-header field.
+    check_header(fields, hdr, !IS_VALID_HEADER, IS_TRAILER);
+  }
+  SECTION("Test request with Connection headers")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method",    "GET"         },
+      {":scheme",    "https"       },
+      {":authority", "www.this.com"},
+      {":path",      "/some/path"  },
+      {"Connection", "Keep-Alive"  },
+    };
+    // Connection-specific headers are not allowed.
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  SECTION("Test request with Keep-Alive headers")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method",    "GET"                },
+      {":scheme",    "https"              },
+      {":authority", "www.this.com"       },
+      {":path",      "/some/path"         },
+      {"Keep-Alive", "timeout=5, max=1000"},
+    };
+    // Connection-specific headers are not allowed.
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  SECTION("Test request with Proxy-Connection headers")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method",          "GET"         },
+      {":scheme",          "https"       },
+      {":authority",       "www.this.com"},
+      {":path",            "/some/path"  },
+      {"Proxy-Connection", "Keep-Alive"  },
+    };
+    // Connection-specific headers are not allowed.
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  SECTION("Test request with Upgrade headers")
+  {
+    hdr.create(HTTP_TYPE_REQUEST, HTTP_1_1, heap);
+    Fields_type fields = {
+      {":method",    "GET"         },
+      {":scheme",    "https"       },
+      {":authority", "www.this.com"},
+      {":path",      "/some/path"  },
+      {"Upgrade",    "HTTP/2.0"    },
+    };
+    // Connection-specific headers are not allowed.
+    check_header(fields, hdr, !IS_VALID_HEADER);
+  }
+  // teardown
+  hdr.destroy();
+}
diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
index fb822769e0..454d040b61 100644
--- a/proxy/http2/HTTP2.cc
+++ b/proxy/http2/HTTP2.cc
@@ -22,6 +22,7 @@
  */
 
 #include "hdrs/VersionConverter.h"
+#include "hdrs/HeaderValidator.h"
 #include "HTTP2.h"
 #include "HPACK.h"
 
@@ -470,11 +471,6 @@ Http2ErrorCode
 http2_decode_header_blocks(HTTPHdr *hdr, const uint8_t *buf_start, const 
uint32_t buf_len, uint32_t *len_read, HpackHandle &handle,
                            bool is_trailing_header, uint32_t 
maximum_table_size, bool is_outbound)
 {
-  const MIMEField *field = nullptr;
-  const char *name       = nullptr;
-  int name_len           = 0;
-  const char *value      = nullptr;
-  int value_len          = 0;
   int64_t result = hpack_decode_header_block(handle, hdr, buf_start, buf_len, 
Http2::max_header_list_size, maximum_table_size);
 
   if (result < 0) {
@@ -489,105 +485,8 @@ http2_decode_header_blocks(HTTPHdr *hdr, const uint8_t 
*buf_start, const uint32_
   if (len_read) {
     *len_read = result;
   }
-
-  MIMEFieldIter iter;
-  auto method_field       = hdr->field_find(PSEUDO_HEADER_METHOD.data(), 
PSEUDO_HEADER_METHOD.size());
-  bool has_connect_method = false;
-  if (method_field) {
-    int method_len;
-    const char *method_value = method_field->value_get(&method_len);
-    has_connect_method       = method_len == HTTP_LEN_CONNECT && 
strncmp(HTTP_METHOD_CONNECT, method_value, HTTP_LEN_CONNECT) == 0;
-  }
-  unsigned int expected_pseudo_header_count = is_outbound ? 1 : 
has_connect_method ? 2 : 4;
-  unsigned int pseudo_header_count          = 0;
-
-  if (is_trailing_header) {
-    expected_pseudo_header_count = 0;
-  }
-  for (auto &field : *hdr) {
-    name = field.name_get(&name_len);
-    // Pseudo headers must appear before regular headers
-    if (name_len && name[0] == ':') {
-      ++pseudo_header_count;
-      if (pseudo_header_count > expected_pseudo_header_count) {
-        return Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR;
-      }
-    } else if (name_len <= 0) {
-      return Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR;
-    } else {
-      if (pseudo_header_count != expected_pseudo_header_count) {
-        return Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR;
-      }
-    }
-  }
-
-  // rfc7540,sec8.1.2.2: Any message containing connection-specific header
-  // fields MUST be treated as malformed
-  if (hdr->field_find(MIME_FIELD_CONNECTION, MIME_LEN_CONNECTION) != nullptr ||
-      hdr->field_find(MIME_FIELD_KEEP_ALIVE, MIME_LEN_KEEP_ALIVE) != nullptr ||
-      hdr->field_find(MIME_FIELD_PROXY_CONNECTION, MIME_LEN_PROXY_CONNECTION) 
!= nullptr ||
-      hdr->field_find(MIME_FIELD_UPGRADE, MIME_LEN_UPGRADE) != nullptr) {
-    return Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR;
-  }
-
-  // :path pseudo header MUST NOT empty for http or https URIs
-  field = hdr->field_find(PSEUDO_HEADER_PATH.data(), 
PSEUDO_HEADER_PATH.size());
-  if (field) {
-    field->value_get(&value_len);
-    if (value_len == 0) {
-      return Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR;
-    }
-  }
-
-  // when The TE header field is received, it MUST NOT contain any
-  // value other than "trailers".
-  field = hdr->field_find(MIME_FIELD_TE, MIME_LEN_TE);
-  if (field) {
-    value = field->value_get(&value_len);
-    if (!(value_len == 8 && memcmp(value, "trailers", 8) == 0)) {
-      return Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR;
-    }
-  }
-
-  if (!is_trailing_header) {
-    // Check pseudo headers
-    if (is_outbound) {
-      if (hdr->fields_count() >= 1) {
-        if (hdr->field_find(PSEUDO_HEADER_STATUS.data(), 
PSEUDO_HEADER_STATUS.size()) == nullptr) {
-          return Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR;
-        }
-      } else {
-        // There should at least be :status pseudo header.
-        return Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR;
-      }
-    } else {
-      if (!has_connect_method && hdr->fields_count() >= 4) {
-        if (hdr->field_find(PSEUDO_HEADER_SCHEME.data(), 
PSEUDO_HEADER_SCHEME.size()) == nullptr ||
-            hdr->field_find(PSEUDO_HEADER_METHOD.data(), 
PSEUDO_HEADER_METHOD.size()) == nullptr ||
-            hdr->field_find(PSEUDO_HEADER_PATH.data(), 
PSEUDO_HEADER_PATH.size()) == nullptr ||
-            hdr->field_find(PSEUDO_HEADER_AUTHORITY.data(), 
PSEUDO_HEADER_AUTHORITY.size()) == nullptr ||
-            hdr->field_find(PSEUDO_HEADER_STATUS.data(), 
PSEUDO_HEADER_STATUS.size()) != nullptr) {
-          // Decoded header field is invalid
-          return Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR;
-        }
-      } else if (has_connect_method && hdr->fields_count() >= 2) {
-        if (hdr->field_find(PSEUDO_HEADER_SCHEME.data(), 
PSEUDO_HEADER_SCHEME.size()) != nullptr ||
-            hdr->field_find(PSEUDO_HEADER_METHOD.data(), 
PSEUDO_HEADER_METHOD.size()) == nullptr ||
-            hdr->field_find(PSEUDO_HEADER_PATH.data(), 
PSEUDO_HEADER_PATH.size()) != nullptr ||
-            hdr->field_find(PSEUDO_HEADER_AUTHORITY.data(), 
PSEUDO_HEADER_AUTHORITY.size()) == nullptr ||
-            hdr->field_find(PSEUDO_HEADER_STATUS.data(), 
PSEUDO_HEADER_STATUS.size()) != nullptr) {
-          // Decoded header field is invalid
-          return Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR;
-        }
-
-      } else {
-        // Pseudo headers is insufficient
-        return Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR;
-      }
-    }
-  }
-
-  return Http2ErrorCode::HTTP2_ERROR_NO_ERROR;
+  return HeaderValidator::is_h2_h3_header_valid(*hdr, is_outbound, 
is_trailing_header) ? Http2ErrorCode::HTTP2_ERROR_NO_ERROR :
+                                                                               
          Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR;
 }
 
 // Initialize this subsystem with librecords configs (for now)
diff --git a/proxy/http3/Http3HeaderVIOAdaptor.cc 
b/proxy/http3/Http3HeaderVIOAdaptor.cc
index 4d449af0b3..ffe59c85f3 100644
--- a/proxy/http3/Http3HeaderVIOAdaptor.cc
+++ b/proxy/http3/Http3HeaderVIOAdaptor.cc
@@ -22,6 +22,7 @@
  */
 
 #include "Http3HeaderVIOAdaptor.h"
+#include "hdrs/HeaderValidator.h"
 
 #include "I_VIO.h"
 #include "HTTP.h"
@@ -95,6 +96,13 @@ Http3HeaderVIOAdaptor::event_handler(int event, Event *data)
 int
 Http3HeaderVIOAdaptor::_on_qpack_decode_complete()
 {
+  // Currently trailer support for h3 is not implemented.
+  constexpr static bool NON_TRAILER = false;
+  if (!HeaderValidator::is_h2_h3_header_valid(this->_header, 
http_hdr_type_get(this->_header.m_http) == HTTP_TYPE_RESPONSE,
+                                              NON_TRAILER)) {
+    Debug("http3", "Header is invalid");
+    return -1;
+  }
   int res = this->_hvc.convert(this->_header, 3, 1);
   if (res != 0) {
     Debug("http3", "PARSE_RESULT_ERROR");

Reply via email to