From 0a79b43e7823aeada576c92f0c954567be1605fe Mon Sep 17 00:00:00 2001 From: Mo Chen Date: Sat, 11 Jul 2026 16:37:10 -0500 Subject: [PATCH 1/9] Add header parsing benchmarks Header parsing needs repeatable measurements before its hot paths can be optimized. Cover zero-copy parsing under the default strict URI mode, copied inputs, modern browser headers, duplicate-heavy responses, and canonical and lowercase WKS names in one harness. Include a profiling loop and file-loaded corpora for investigating representative workloads. --- tools/benchmark/CMakeLists.txt | 6 + tools/benchmark/benchmark_HdrParse.cc | 902 ++++++++++++++++++++++++++ 2 files changed, 908 insertions(+) create mode 100644 tools/benchmark/benchmark_HdrParse.cc diff --git a/tools/benchmark/CMakeLists.txt b/tools/benchmark/CMakeLists.txt index 1456824fb37..b268cdd5081 100644 --- a/tools/benchmark/CMakeLists.txt +++ b/tools/benchmark/CMakeLists.txt @@ -57,3 +57,9 @@ target_include_directories(benchmark_HuffmanDecode PRIVATE ${CMAKE_SOURCE_DIR}/l add_executable(benchmark_ascii_tolower benchmark_ascii_tolower.cc) target_link_libraries(benchmark_ascii_tolower PRIVATE Catch2::Catch2WithMain ts::tscore) + +add_executable(benchmark_HdrParse benchmark_HdrParse.cc) +target_link_libraries( + benchmark_HdrParse PRIVATE Catch2::Catch2 ts::hdrs ts::inkevent libswoc::libswoc lshpack configmanager +) +target_include_directories(benchmark_HdrParse PRIVATE ${CMAKE_SOURCE_DIR}/lib) diff --git a/tools/benchmark/benchmark_HdrParse.cc b/tools/benchmark/benchmark_HdrParse.cc new file mode 100644 index 00000000000..3644ea396be --- /dev/null +++ b/tools/benchmark/benchmark_HdrParse.cc @@ -0,0 +1,902 @@ +/** @file + + Micro-benchmark for HTTP header parsing (src/proxy/hdrs). + + Two modes in one binary: + + * A/B stats mode (default): Catch2 BENCHMARK cases produce mean/median/ + stddev per (target x corpus) so an optimization can be measured before + and after and guarded against regression. Run e.g.: + benchmark_HdrParse "[bench]" --benchmark-samples 100 + + * Profiling mode (--profile ): a tight fixed-count loop with no + Catch2 harness overhead, meant to be wrapped by perf/vtune: + perf stat -e cycles,instructions \ + benchmark_HdrParse --profile request --iters 5000000 + + Targets: request, response, mime, url, wks. + + A realistic + adversarial corpus is built in. Real captured header blocks can + be supplied with --corpus-file FILE or --corpus-dir DIR (blocks split on a + blank line, classified request vs response by the first line); these are added + to both modes, and each loaded request's request-target also feeds the url + target. In stats mode each target also gets a "corpus" benchmark that times one + pass over the loaded cases alone, so its result reflects only the supplied + traffic. + + @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 "proxy/hdrs/HTTP.h" +#include "proxy/hdrs/MIME.h" +#include "proxy/hdrs/URL.h" +#include "proxy/hdrs/HdrToken.h" +#include "proxy/hdrs/HdrHeap.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define CATCH_CONFIG_ENABLE_BENCHMARKING +#include +#include +#include + +// Defined in tscore; disables thread-local proxy allocators so no event-system +// thread setup is required (mirrors the hdrs unit-test main). +extern int cmd_disable_pfreelist; + +namespace +{ +// ---------------------------------------------------------------------------- +// Corpus +// ---------------------------------------------------------------------------- + +struct HeaderCase { + std::string label; + std::string data; // full block including the start line, ends "\r\n\r\n" + bool is_response = false; +}; + +struct UrlCase { + std::string label; + std::string data; // a bare request target +}; + +struct Corpus { + std::vector cases; + std::vector urls; // bare request targets for the url target + std::vector file_urls; // request targets of the file-loaded requests + std::vector wks; // field names (owned) for the wks target + // The same names lowercased. HTTP/2 mandates lowercase field names while the + // well-known-string table holds canonical mixed case, so roughly half of real + // edge traffic reaches the well-known lookup folded. Without this the wks + // target only ever measures the canonical-case path. + std::vector wks_lower; +}; + +// A representative modern browser request and typical responses, alongside the +// classic fixtures used by the hdrs unit tests. +constexpr std::string_view REQ_REALISTIC = + "GET /assets/app.9f2c.js HTTP/1.1\r\n" + "Host: www.example.com\r\n" + "User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) " + "Chrome/126.0.0.0 Safari/537.36\r\n" + "Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8\r\n" + "Accept-Encoding: gzip, deflate, br, zstd\r\n" + "Accept-Language: en-US,en;q=0.9\r\n" + "Cookie: session=8f14e45fceea167a5a36dedd4bea2543; theme=dark; region=us-west-2; ab_bucket=37\r\n" + "Referer: https://www.example.com/\r\n" + "Connection: keep-alive\r\n" + "\r\n"; + +// A 2026 Chrome navigation request. Unlike REQ_REALISTIC (all classic WKS +// names), this carries the modern client-hint / fetch-metadata headers +// (sec-ch-ua*, sec-fetch-*, priority, upgrade-insecure-requests) that are NOT +// well-known strings, so it exercises the non-WKS name paths (tokenize miss, +// name validation, duplicate filter) the way real traffic does. +constexpr std::string_view REQ_MODERN = + "GET /app/feed?tab=home HTTP/1.1\r\n" + "Host: www.example.com\r\n" + "Connection: keep-alive\r\n" + "sec-ch-ua: \"Chromium\";v=\"126\", \"Google Chrome\";v=\"126\", \"Not-A.Brand\";v=\"99\"\r\n" + "sec-ch-ua-mobile: ?0\r\n" + "sec-ch-ua-platform: \"Windows\"\r\n" + "upgrade-insecure-requests: 1\r\n" + "User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) " + "Chrome/126.0.0.0 Safari/537.36\r\n" + "Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8\r\n" + "Sec-Fetch-Site: same-origin\r\n" + "Sec-Fetch-Mode: navigate\r\n" + "Sec-Fetch-User: ?1\r\n" + "Sec-Fetch-Dest: document\r\n" + "Accept-Encoding: gzip, deflate, br, zstd\r\n" + "Accept-Language: en-US,en;q=0.9\r\n" + "Cookie: session=8f14e45fceea167a5a36dedd4bea2543; theme=dark; region=us-west-2; ab_bucket=37\r\n" + "Priority: u=0, i\r\n" + "\r\n"; + +constexpr std::string_view REQ_CLASSIC = "GET http://www.news.com:80/ HTTP/1.0\r\n" + "Proxy-Connection: Keep-Alive\r\n" + "User-Agent: Mozilla/4.04 [en] (X11; I; Linux 2.0.33 i586)\r\n" + "Pragma: no-cache\r\n" + "Host: www.news.com\r\n" + "Accept: image/gif, image/x-xbitmap, image/jpeg, image/pjpeg, image/png, */*\r\n" + "Accept-Language: en\r\n" + "Accept-Charset: iso-8859-1, *, utf-8\r\n" + "\r\n"; + +constexpr std::string_view RESP_REALISTIC = "HTTP/1.1 200 OK\r\n" + "Server: ATS/10.1.0\r\n" + "Date: Mon, 21 Oct 2013 20:13:21 GMT\r\n" + "Content-Type: text/html; charset=utf-8\r\n" + "Content-Length: 12345\r\n" + "Cache-Control: max-age=31536000, public, immutable\r\n" + "Vary: Accept-Encoding\r\n" + "Age: 42\r\n" + "\r\n"; + +constexpr std::string_view RESP_304 = "HTTP/1.1 304 Not Modified\r\n" + "Date: Mon, 21 Oct 2013 20:13:21 GMT\r\n" + "Etag: \"6f2c9a1b\"\r\n" + "Cache-Control: max-age=31536000\r\n" + "\r\n"; + +constexpr std::string_view URL_REALISTIC = "http://www.example.com/images/2026/06/some-article/hero.webp?w=1200&q=75"; + +// Build the adversarial cases programmatically so their worst-case sizes are +// obvious and easy to tune. +std::string +gen_many_fields(int n) +{ + std::string s = "GET /many HTTP/1.1\r\nHost: h\r\n"; + for (int i = 0; i < n; ++i) { + s += "X-Custom-Header-" + std::to_string(i) + ": value-" + std::to_string(i) + "\r\n"; + } + s += "\r\n"; + return s; +} + +std::string +gen_long_value(int len) +{ + std::string s = "GET /long HTTP/1.1\r\nHost: h\r\nX-Blob: "; + s.append(len, 'a'); + s += "\r\n\r\n"; + return s; +} + +// Field names that are guaranteed not to be well-known, forcing the +// hdrtoken_tokenize hash miss + per-char field-name validation path. +std::string +gen_wks_miss(int n) +{ + std::string s = "GET /miss HTTP/1.1\r\nHost: h\r\n"; + for (int i = 0; i < n; ++i) { + s += "X-Zzq-Nonstandard-Field-" + std::to_string(i) + ": v\r\n"; + } + s += "\r\n"; + return s; +} + +// Many duplicates of a well-known, commonly-repeated field: stresses +// mime_hdr_field_attach duplicate chaining. +std::string +gen_dup_fields(int n) +{ + std::string s = "HTTP/1.1 200 OK\r\nDate: Mon, 21 Oct 2013 20:13:21 GMT\r\n"; + for (int i = 0; i < n; ++i) { + s += "Set-Cookie: c" + std::to_string(i) + "=v" + std::to_string(i) + "; Path=/; HttpOnly\r\n"; + } + s += "\r\n"; + return s; +} + +std::string +gen_long_uri(int len) +{ + std::string s = "GET http://www.example.com/"; + s.append(len, 'x'); + s += " HTTP/1.1\r\nHost: www.example.com\r\n\r\n"; + return s; +} + +// The MIME-only target parses a field block with the start line removed. +std::string_view +strip_start_line(std::string_view block) +{ + auto pos = block.find('\n'); + return pos == std::string_view::npos ? block : block.substr(pos + 1); +} + +// Collect the field names in a block (for the wks target). Skips the start line, +// continuation lines, and lines without a colon. +void +collect_field_names(std::string_view block, std::vector &out) +{ + std::string_view rest = strip_start_line(block); + size_t pos = 0; + while (pos < rest.size()) { + size_t eol = rest.find('\n', pos); + std::string_view line = rest.substr(pos, (eol == std::string_view::npos ? rest.size() : eol) - pos); + pos = (eol == std::string_view::npos) ? rest.size() : eol + 1; + if (line.empty() || line == "\r" || line.front() == ' ' || line.front() == '\t') { + continue; // blank terminator or folded continuation + } + size_t colon = line.find(':'); + if (colon != std::string_view::npos && colon > 0) { + out.emplace_back(line.substr(0, colon)); + } + } +} + +// Load raw header blocks from a file. Multiple blocks may be separated by a +// blank line. Line endings are normalized and each block re-terminated with a +// canonical "\r\n\r\n". +void +load_file(const std::filesystem::path &path, std::vector &out) +{ + std::ifstream in(path, std::ios::binary); + if (!in) { + std::fprintf(stderr, "warning: cannot open corpus file %s\n", path.string().c_str()); + return; + } + std::stringstream ss; + ss << in.rdbuf(); + std::string content = ss.str(); + + // Normalize CRLF -> LF, then split blocks on a blank line ("\n\n"). + std::string norm; + norm.reserve(content.size()); + for (char ch : content) { + if (ch != '\r') { + norm += ch; + } + } + + int idx = 0; + size_t pos = 0; + while (pos < norm.size()) { + size_t sep = norm.find("\n\n", pos); + size_t end = (sep == std::string::npos) ? norm.size() : sep; + std::string_view chunk = std::string_view(norm).substr(pos, end - pos); + // Trim leading/trailing newlines. + while (!chunk.empty() && chunk.front() == '\n') { + chunk.remove_prefix(1); + } + while (!chunk.empty() && chunk.back() == '\n') { + chunk.remove_suffix(1); + } + if (!chunk.empty()) { + // Re-insert canonical CRLF line endings and terminator. + std::string block; + for (size_t i = 0; i < chunk.size(); ++i) { + if (chunk[i] == '\n') { + block += "\r\n"; + } else { + block += chunk[i]; + } + } + block += "\r\n\r\n"; + HeaderCase c; + c.label = path.filename().string() + "#" + std::to_string(idx++); + c.is_response = block.rfind("HTTP/", 0) == 0; // status line starts with "HTTP/" + c.data = std::move(block); + out.push_back(std::move(c)); + } + if (sep == std::string::npos) { + break; + } + pos = sep + 2; + } +} + +// The request-target of a request block's "METHOD SP target SP version" start +// line, or empty if the start line doesn't have exactly that shape. +std::string_view +request_target(std::string_view block) +{ + std::string_view const line = block.substr(0, block.find("\r\n")); + size_t const first = line.find(' '); + + if (first == std::string_view::npos) { + return {}; + } + size_t const second = line.find(' ', first + 1); + if (second == std::string_view::npos || second == first + 1 || line.find(' ', second + 1) != std::string_view::npos) { + return {}; + } + return line.substr(first + 1, second - first - 1); +} + +Corpus +build_corpus(const std::vector &files, const std::vector &dirs) +{ + Corpus corp; + auto add = [&](std::string_view label, std::string_view data, bool is_resp) { + corp.cases.push_back({std::string(label), std::string(data), is_resp}); + }; + + // Realistic. + add("req_realistic", REQ_REALISTIC, false); + add("req_modern", REQ_MODERN, false); + add("req_classic", REQ_CLASSIC, false); + add("resp_realistic", RESP_REALISTIC, true); + add("resp_304", RESP_304, true); + + // Adversarial. + add("adv_many_fields", gen_many_fields(100), false); + add("adv_long_value", gen_long_value(8192), false); + add("adv_wks_miss", gen_wks_miss(50), false); + add("adv_dup_fields", gen_dup_fields(50), true); + add("adv_long_uri", gen_long_uri(4000), false); + + // File-loaded. + size_t const builtin_count = corp.cases.size(); + + for (const auto &f : files) { + load_file(f, corp.cases); + } + for (const auto &d : dirs) { + std::error_code ec; + for (auto it = std::filesystem::directory_iterator(d, ec); !ec && it != std::filesystem::directory_iterator(); ++it) { + if (it->is_regular_file()) { + load_file(it->path(), corp.cases); + } + } + } + + corp.urls = {std::string(URL_REALISTIC), "http://www.example.com/" + std::string(4000, 'x')}; + + for (size_t i = builtin_count; i < corp.cases.size(); ++i) { + const auto &c = corp.cases[i]; + if (c.is_response) { + continue; + } + if (auto const target = request_target(c.data); !target.empty()) { + corp.file_urls.push_back({c.label, std::string(target)}); + } + } + + for (const auto &c : corp.cases) { + collect_field_names(c.data, corp.wks); + } + + return corp; +} + +Corpus g_corpus; + +// ---------------------------------------------------------------------------- +// Parse drivers. Each does the minimal realistic setup, one parse, teardown, and +// returns {ParseResult, sink}. +// +// The primary production path is ZERO-COPY: the IOBuffer proxy path +// (HdrTSOnly.cc parse_req(IOBufferReader*)) attaches the socket block to the +// heap and parses with must_copy_strings=false, and it runs under +// strict_uri_parsing=2 (the config default). So the drivers default to +// copy=false and strict=2; pass copy=true to measure the copy path. With +// copy=false the parsed field pointers alias the input buffer, which is a stable +// static corpus string here, so this is safe for the lifetime of the parse. +// ---------------------------------------------------------------------------- + +constexpr bool PROD_COPY = false; // zero-copy IOBuffer proxy path +constexpr int PROD_STRICT = 2; // proxy.config.http.strict_uri_parsing default + +using ParseOutcome = std::pair; + +ParseOutcome +drive_request(std::string_view raw, bool copy = PROD_COPY, int strict = PROD_STRICT) +{ + HTTPParser parser; + http_parser_init(&parser); + HTTPHdr hdr; + HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); // +64 avoids proxy alloc + hdr.create(HTTPType::REQUEST, HTTP_1_1, heap); + const char *start = raw.data(); + ParseResult ret = http_parser_parse_req(&parser, hdr.m_heap, hdr.m_http, &start, raw.data() + raw.size(), copy, + /*eof*/ true, strict, UINT16_MAX, 131070); + uint64_t sink = static_cast(start - raw.data()); + hdr.destroy(); + return {ret, sink}; +} + +ParseOutcome +drive_response(std::string_view raw, bool copy = PROD_COPY) +{ + HTTPParser parser; + http_parser_init(&parser); + HTTPHdr hdr; + HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); + hdr.create(HTTPType::RESPONSE, HTTP_1_1, heap); + const char *start = raw.data(); + ParseResult ret = http_parser_parse_resp(&parser, hdr.m_heap, hdr.m_http, &start, raw.data() + raw.size(), copy, /*eof*/ true); + uint64_t sink = static_cast(start - raw.data()); + hdr.destroy(); + return {ret, sink}; +} + +ParseOutcome +drive_mime(std::string_view fields, bool copy = PROD_COPY) +{ + MIMEParser parser; + mime_parser_init(&parser); + MIMEHdr hdr; + HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); + hdr.create(heap); + const char *start = fields.data(); + ParseResult ret = mime_parser_parse(&parser, hdr.m_heap, hdr.m_mime, &start, fields.data() + fields.size(), copy, + /*eof*/ true, /*remove_ws_from_field_name*/ false); + uint64_t sink = static_cast(start - fields.data()); + hdr.destroy(); + return {ret, sink}; +} + +ParseOutcome +drive_url(std::string_view uri, bool copy = PROD_COPY, int strict = PROD_STRICT) +{ + URL url; + HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); + url.create(heap); + const char *start = uri.data(); + ParseResult ret = url_parse(heap, url.m_url_impl, &start, uri.data() + uri.size(), copy, strict, /*verify_host*/ true); + // Read back parsed state so a successful parse (ParseResult::DONE == 0) cannot + // be optimized away. + uint64_t sink = static_cast(ret) + static_cast(url.host_get().length()) + url.port_get(); + url.destroy(); + return {ret, sink}; +} + +// Isolates the per-field well-known-string hash + lookup (and the miss-path +// validation) without the surrounding MIME machinery. +uint64_t +drive_wks(const std::vector &names) +{ + uint64_t sink = 0; + for (const auto &n : names) { + const char *wks = nullptr; + int idx = hdrtoken_tokenize(n.data(), static_cast(n.size()), &wks); + sink += static_cast(idx + 1) + reinterpret_cast(wks); + } + return sink; +} + +// ---------------------------------------------------------------------------- +// Profiling mode +// ---------------------------------------------------------------------------- + +enum class Target { Request, Response, Mime, Url, Wks, WksLower, Unknown }; + +Target +parse_target(std::string_view s) +{ + if (s == "request") { + return Target::Request; + } + if (s == "response") { + return Target::Response; + } + if (s == "mime") { + return Target::Mime; + } + if (s == "url") { + return Target::Url; + } + if (s == "wks") { + return Target::Wks; + } + if (s == "wkslower" || s == "wks-lower") { + return Target::WksLower; + } + return Target::Unknown; +} + +const char * +profile_target_name(Target t) +{ + switch (t) { + case Target::Request: + return "request"; + case Target::Response: + return "response"; + case Target::Mime: + return "mime"; + case Target::Url: + return "url"; + case Target::WksLower: + return "wks-lower"; + case Target::Wks: + return "wks"; + default: + return "unknown"; + } +} + +int +run_profile(Target target, uint64_t iters) +{ + // Assemble the input set and a per-iteration byte count for throughput. + std::vector inputs; + uint64_t bytes_per_pass = 0; + + auto add_input = [&](std::string_view v) { + inputs.push_back(v); + bytes_per_pass += v.size(); + }; + + switch (target) { + case Target::Request: + for (const auto &c : g_corpus.cases) { + if (!c.is_response) { + add_input(c.data); + } + } + break; + case Target::Response: + for (const auto &c : g_corpus.cases) { + if (c.is_response) { + add_input(c.data); + } + } + break; + case Target::Mime: + for (const auto &c : g_corpus.cases) { + add_input(strip_start_line(c.data)); + } + break; + case Target::Url: + for (const auto &u : g_corpus.urls) { + add_input(u); + } + for (const auto &u : g_corpus.file_urls) { + add_input(u.data); + } + break; + case Target::Wks: + // Handled below (uses the owned name list directly). + for (const auto &n : g_corpus.wks) { + bytes_per_pass += n.size(); + } + break; + case Target::WksLower: + for (const auto &n : g_corpus.wks_lower) { + bytes_per_pass += n.size(); + } + break; + default: + std::fprintf(stderr, "unknown --profile target\n"); + return 2; + } + + if (target != Target::Wks && target != Target::WksLower && inputs.empty()) { + std::fprintf(stderr, "no inputs for the requested target\n"); + return 2; + } + + volatile uint64_t sink = 0; + auto t0 = std::chrono::steady_clock::now(); + uint64_t count = 0; + + if (target == Target::Wks || target == Target::WksLower) { + auto const &names = (target == Target::Wks) ? g_corpus.wks : g_corpus.wks_lower; + for (uint64_t i = 0; i < iters; ++i) { + sink += drive_wks(names); + } + count = iters; // one full pass over all names per iter + } else { + for (uint64_t i = 0; i < iters; ++i) { + std::string_view in = inputs[i % inputs.size()]; + ParseOutcome r; + switch (target) { + case Target::Request: + r = drive_request(in); + break; + case Target::Response: + r = drive_response(in); + break; + case Target::Mime: + r = drive_mime(in); + break; + case Target::Url: + r = drive_url(in); + break; + default: + break; + } + sink += r.second; + } + count = iters; + } + + auto t1 = std::chrono::steady_clock::now(); + double ns = std::chrono::duration(t1 - t0).count(); + double ns_per = ns / static_cast(count); + + // One "op" is one parse for the parse targets, or one full pass over the name + // list for wks. Throughput is over the header bytes actually processed. + double avg_in = inputs.empty() ? 0.0 : static_cast(bytes_per_pass) / static_cast(inputs.size()); + double total_bytes = (target == Target::Wks || target == Target::WksLower) ? + static_cast(bytes_per_pass) * static_cast(iters) : + avg_in * static_cast(iters); + double mibps = (total_bytes / (ns / 1e9)) / (1024.0 * 1024.0); + + std::printf("target=%s iters=%llu %.2f ns/op %.2f Mops/s %.0f MiB/s sink=%llu\n", profile_target_name(target), + static_cast(count), ns_per, 1000.0 / ns_per, mibps, static_cast(sink)); + return 0; +} + +} // namespace + +// ---------------------------------------------------------------------------- +// A/B stats mode (Catch2 BENCHMARK) +// ---------------------------------------------------------------------------- + +namespace +{ +const HeaderCase & +find_case(std::string_view label) +{ + for (const auto &c : g_corpus.cases) { + if (c.label == label) { + return c; + } + } + FAIL("missing corpus case: " << label); + return g_corpus.cases.front(); +} +} // namespace + +// Built-in cases must fully parse (DONE + all bytes consumed); file-loaded cases +// (label carries a '#') only must not ERROR. +bool +is_file_case(const HeaderCase &c) +{ + return c.label.find('#') != std::string::npos; +} + +TEST_CASE("hdr parse: request", "[bench][request]") +{ + std::vector loaded; + + for (const auto &c : g_corpus.cases) { + if (c.is_response) { + continue; + } + CAPTURE(c.label); + auto [ret, consumed] = drive_request(c.data); + if (is_file_case(c)) { + REQUIRE(ret != ParseResult::ERROR); + loaded.push_back(c.data); + } else { + REQUIRE(ret == ParseResult::DONE); + REQUIRE(consumed == c.data.size()); + } + } + + const auto &realistic = find_case("req_realistic"); + const auto &modern = find_case("req_modern"); + const auto &many = find_case("adv_many_fields"); + + BENCHMARK("request: realistic (zero-copy)") + { + return drive_request(realistic.data).second; + }; + BENCHMARK("request: modern browser (zero-copy)") + { + return drive_request(modern.data).second; + }; + BENCHMARK("request: realistic (copy)") + { + return drive_request(realistic.data, /*copy*/ true).second; + }; + BENCHMARK("request: 100 fields") + { + return drive_request(many.data).second; + }; + if (!loaded.empty()) { + BENCHMARK("request: corpus (" + std::to_string(loaded.size()) + " blocks)") + { + uint64_t sink = 0; + for (auto block : loaded) { + sink += drive_request(block).second; + } + return sink; + }; + } +} + +TEST_CASE("hdr parse: response", "[bench][response]") +{ + std::vector loaded; + + for (const auto &c : g_corpus.cases) { + if (!c.is_response) { + continue; + } + CAPTURE(c.label); + auto [ret, consumed] = drive_response(c.data); + if (is_file_case(c)) { + REQUIRE(ret != ParseResult::ERROR); + loaded.push_back(c.data); + } else { + REQUIRE(ret == ParseResult::DONE); + REQUIRE(consumed == c.data.size()); + } + } + + const auto &realistic = find_case("resp_realistic"); + const auto &dups = find_case("adv_dup_fields"); + + BENCHMARK("response: realistic") + { + return drive_response(realistic.data).second; + }; + BENCHMARK("response: 50 dup fields") + { + return drive_response(dups.data).second; + }; + if (!loaded.empty()) { + BENCHMARK("response: corpus (" + std::to_string(loaded.size()) + " blocks)") + { + uint64_t sink = 0; + for (auto block : loaded) { + sink += drive_response(block).second; + } + return sink; + }; + } +} + +TEST_CASE("hdr parse: mime only", "[bench][mime]") +{ + const auto &realistic = find_case("req_realistic"); + const auto &wksmiss = find_case("adv_wks_miss"); + + std::vector loaded; + for (const auto &c : g_corpus.cases) { + if (is_file_case(c)) { + loaded.push_back(strip_start_line(c.data)); + } + } + + BENCHMARK("mime: realistic fields") + { + return drive_mime(strip_start_line(realistic.data)).second; + }; + BENCHMARK("mime: 50 wks-miss fields") + { + return drive_mime(strip_start_line(wksmiss.data)).second; + }; + if (!loaded.empty()) { + BENCHMARK("mime: corpus (" + std::to_string(loaded.size()) + " blocks)") + { + uint64_t sink = 0; + for (auto fields : loaded) { + sink += drive_mime(fields).second; + } + return sink; + }; + } +} + +TEST_CASE("hdr parse: url only", "[bench][url]") +{ + REQUIRE(drive_url(URL_REALISTIC).first != ParseResult::ERROR); + for (const auto &u : g_corpus.file_urls) { + CAPTURE(u.label, u.data); + REQUIRE(drive_url(u.data).first != ParseResult::ERROR); + } + + BENCHMARK("url: realistic") + { + return drive_url(URL_REALISTIC).second; + }; + if (!g_corpus.file_urls.empty()) { + BENCHMARK("url: corpus (" + std::to_string(g_corpus.file_urls.size()) + " targets)") + { + uint64_t sink = 0; + for (const auto &u : g_corpus.file_urls) { + sink += drive_url(u.data).second; + } + return sink; + }; + } +} + +TEST_CASE("hdr parse: wks tokenize", "[bench][wks]") +{ + REQUIRE(!g_corpus.wks.empty()); + + BENCHMARK("wks: tokenize all field names") + { + return drive_wks(g_corpus.wks); + }; + + BENCHMARK("wks: tokenize all field names, lowercased (H2 form)") + { + return drive_wks(g_corpus.wks_lower); + }; +} + +// ---------------------------------------------------------------------------- +// Entry point (own main, like the hdrs unit-test stub, so we can init the WKS +// tables and branch to the profiling loop). +// ---------------------------------------------------------------------------- + +int +main(int argc, char *argv[]) +{ + // No thread setup, forbid thread-local allocators (mirrors unit_test_main.cc). + cmd_disable_pfreelist = true; + // Populate the HTTP well-known strings; parsing depends on them. + http_init(); + + // Pull out our own flags (--profile/--iters/--corpus-*) and pass the rest to + // Catch. --corpus-* feed both modes. + std::vector corpus_files, corpus_dirs; + std::string profile_target; + uint64_t iters = 5'000'000; + std::vector catch_args; + catch_args.push_back(argv[0]); + + for (int i = 1; i < argc; ++i) { + std::string_view a = argv[i]; + if (a == "--profile" && i + 1 < argc) { + profile_target = argv[++i]; + } else if (a == "--iters" && i + 1 < argc) { + iters = std::strtoull(argv[++i], nullptr, 10); + } else if (a == "--corpus-file" && i + 1 < argc) { + corpus_files.emplace_back(argv[++i]); + } else if (a == "--corpus-dir" && i + 1 < argc) { + corpus_dirs.emplace_back(argv[++i]); + } else { + catch_args.push_back(argv[i]); + } + } + + g_corpus = build_corpus(corpus_files, corpus_dirs); + + for (auto const &n : g_corpus.wks) { + std::string lower = n; + for (char &c : lower) { + c = (c >= 'A' && c <= 'Z') ? static_cast(c + 32) : c; + } + g_corpus.wks_lower.push_back(std::move(lower)); + } + + if (!profile_target.empty()) { + Target t = parse_target(profile_target); + if (t == Target::Unknown) { + std::fprintf(stderr, "unknown target '%s' (want: request|response|mime|url|wks|wks-lower)\n", profile_target.c_str()); + return 2; + } + return run_profile(t, iters); + } + + return Catch::Session().run(static_cast(catch_args.size()), catch_args.data()); +} From 590a4c0456fbe586ce3c5b785e95e95716eb03bf Mon Sep 17 00:00:00 2001 From: Mo Chen Date: Sat, 11 Jul 2026 16:41:03 -0500 Subject: [PATCH 2/9] Vectorize URL compliance validation Default URI validation pays for libc character classification on every byte. Use a branchless ASCII range reduction to enable vectorization and make acceptance locale-independent; rejection no longer logs the offending byte. Exhaustive differential tests cover every byte value across vector boundaries and scalar tails. --- src/proxy/hdrs/URL.cc | 13 ++---- src/proxy/hdrs/unit_tests/test_URL.cc | 63 +++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/src/proxy/hdrs/URL.cc b/src/proxy/hdrs/URL.cc index c842dbd29a1..96abbe8cc57 100644 --- a/src/proxy/hdrs/URL.cc +++ b/src/proxy/hdrs/URL.cc @@ -1206,17 +1206,12 @@ url_is_strictly_compliant(const char *start, const char *end) bool url_is_mostly_compliant(const char *start, const char *end) { + unsigned char bad = 0; for (const char *i = start; i < end; ++i) { - if (isspace(*i)) { - Dbg(dbg_ctl_http, "Whitespace character [0x%.2X] found in URL", static_cast(*i)); - return false; - } - if (!isprint(*i)) { - Dbg(dbg_ctl_http, "Non-printable character [0x%.2X] found in URL", static_cast(*i)); - return false; - } + unsigned char const c = static_cast(*i); + bad |= static_cast((c < 0x21) | (c > 0x7E)); } - return true; + return bad == 0; } } // namespace UrlImpl diff --git a/src/proxy/hdrs/unit_tests/test_URL.cc b/src/proxy/hdrs/unit_tests/test_URL.cc index 020b3903f28..2fc62bcdfda 100644 --- a/src/proxy/hdrs/unit_tests/test_URL.cc +++ b/src/proxy/hdrs/unit_tests/test_URL.cc @@ -18,6 +18,7 @@ the License. */ +#include #include #include #include @@ -172,6 +173,68 @@ TEST_CASE("ParseRulesMostlyStrictURI", "[proxy][parseuri]") CHECK(url_is_mostly_compliant(i.uri, i.uri + strlen(i.uri)) == i.valid); } +namespace +{ +// The isspace()/isprint() pair that url_is_mostly_compliant replaced, kept as +// the reference so this compares old behavior against new instead of restating +// the new range test. Bytes are passed as unsigned char; the original passed a +// plain char, which is undefined for the negative values every byte >= 0x80 +// produces. In the C locale, which ATS never leaves (nothing in the tree calls +// setlocale), this accepts exactly 0x21..0x7E. +bool +url_mostly_compliant_reference(const char *start, const char *end) +{ + for (const char *p = start; p < end; ++p) { + unsigned char const c = static_cast(*p); + if (isspace(c) || !isprint(c)) { + return false; + } + } + return true; +} +} // namespace + +TEST_CASE("MostlyCompliantVsScalar", "[proxy][parseuri]") +{ + // The auto-vectorized url_is_mostly_compliant must agree with the libc pair + // it replaced for every input. Exhaustively inject each of the 256 byte values + // at each position across lengths 1..40, which span a sub-vector target, a + // whole vector, and the scalar remainder for both 128- and 256-bit builds. + char buf[64]; + int mismatches = 0, first_len = 0, first_pos = 0, first_byte = 0; + + for (int len = 1; len <= 40; ++len) { + for (int pos = 0; pos < len; ++pos) { + for (int b = 0; b < 256; ++b) { + memset(buf, 'a', len); // 'a' (0x61) is compliant + buf[pos] = static_cast(b); + bool const got = url_is_mostly_compliant(buf, buf + len); + bool const want = url_mostly_compliant_reference(buf, buf + len); + if (got != want) { + if (mismatches == 0) { + first_len = len; + first_pos = pos; + first_byte = b; + } + ++mismatches; + } + } + } + } + CAPTURE(mismatches, first_len, first_pos, first_byte); + CHECK(mismatches == 0); + + // Boundary extremes at every length: all-0x7E accepts, all-0x7F rejects. + for (int len = 0; len <= 40; ++len) { + memset(buf, 0x7E, len); + CHECK(url_is_mostly_compliant(buf, buf + len) == true); + if (len > 0) { + memset(buf, 0x7F, len); + CHECK(url_is_mostly_compliant(buf, buf + len) == false); + } + } +} + struct url_parse_test_case { const std::string input_uri; const std::string expected_printed_url; From d83aaa659c0acf75f896abe30da933f58409e911 Mon Sep 17 00:00:00 2001 From: Mo Chen Date: Sat, 11 Jul 2026 21:53:32 -0500 Subject: [PATCH 3/9] Simplify request-target validation Request-target validation calls three URL getters and follows branches that reduce to a direct test of the stored host and scheme fields. Express that condition directly while preserving the existing treatment of origin, asterisk, absolute, and authority forms. --- src/proxy/hdrs/HTTP.cc | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/src/proxy/hdrs/HTTP.cc b/src/proxy/hdrs/HTTP.cc index 602b1cc8662..9eb0668d602 100644 --- a/src/proxy/hdrs/HTTP.cc +++ b/src/proxy/hdrs/HTTP.cc @@ -1119,29 +1119,18 @@ http_parser_parse_req(HTTPParser *parser, HdrHeap *heap, HTTPHdrImpl *hh, const ParseResult validate_hdr_request_target(int method_wk_idx, URLImpl *url) { - ParseResult ret = ParseResult::DONE; - auto host{url->get_host()}; - auto path{url->get_path()}; - auto scheme{url->get_scheme()}; - - if (host.empty()) { - if (path == "*"sv) { // asterisk-form - // Skip this check for now because URLImpl can't distinguish '*' and '/*' - // if (method_wk_idx != HTTP_WKSIDX_OPTIONS) { - // ret = ParseResult::ERROR; - // } - } else { // origin-form - // Nothing to check here - } - } else if (scheme.empty() && !host.empty()) { // authority-form - if (method_wk_idx != HTTP_WKSIDX_CONNECT) { - ret = ParseResult::ERROR; - } - } else { // absolute-form - // Nothing to check here - } + // The only rejected request-target is authority-form (host present, scheme + // absent) with a method other than CONNECT. A part is empty when its pointer + // is null or its length is zero, matching the getters this replaces; the + // asterisk-form check is intentionally disabled (URLImpl can't distinguish + // '*' from '/*'), so origin-, asterisk-, and absolute-form all accept. + bool const host_present = url->m_ptr_host != nullptr && url->m_len_host != 0; + bool const scheme_absent = url->m_scheme_wks_idx < 0 && (url->m_ptr_scheme == nullptr || url->m_len_scheme == 0); - return ret; + if (host_present && scheme_absent && method_wk_idx != HTTP_WKSIDX_CONNECT) { + return ParseResult::ERROR; + } + return ParseResult::DONE; } bool From 1dba7cd3ad5074106387a8b3229a5b89cd9126bc Mon Sep 17 00:00:00 2001 From: Mo Chen Date: Sat, 11 Jul 2026 21:54:11 -0500 Subject: [PATCH 4/9] Fuse field-name scanning and hashing Field names were walked separately to find the colon, hash the name, and validate its characters. Fuse those passes and reuse the WKS lookup through a prehashed entry point, preserving whitespace normalization and the current table representation. Parity tests check the delimiter position, character validation, and token lookup. --- include/proxy/hdrs/HdrToken.h | 8 +- src/proxy/hdrs/HdrToken.cc | 74 ++++++++++--- src/proxy/hdrs/MIME.cc | 48 +++++++-- src/proxy/hdrs/unit_tests/test_mime.cc | 141 +++++++++++++++++++++++++ 4 files changed, 246 insertions(+), 25 deletions(-) diff --git a/include/proxy/hdrs/HdrToken.h b/include/proxy/hdrs/HdrToken.h index 80fa6121658..04b965b75b7 100644 --- a/include/proxy/hdrs/HdrToken.h +++ b/include/proxy/hdrs/HdrToken.h @@ -132,9 +132,11 @@ extern HdrTokenInfoFlags hdrtoken_str_flags[]; // //////////////////////////////////////////////////////////////////////////// -extern void hdrtoken_init(); -extern int hdrtoken_tokenize(const char *string, int string_len, const char **wks_string_out = nullptr); -extern int hdrtoken_method_tokenize(const char *string, int string_len); +extern void hdrtoken_init(); +extern int hdrtoken_tokenize(const char *string, int string_len, const char **wks_string_out = nullptr); +extern int hdrtoken_tokenize_prehashed(const char *string, int string_len, uint32_t hash, const char **wks_string_out = nullptr); +extern int hdrtoken_field_name_scan(const char *string, int maxlen, uint32_t *hash_out, bool *all_valid_out); +extern int hdrtoken_method_tokenize(const char *string, int string_len); extern const char *hdrtoken_string_to_wks(const char *string); extern const char *hdrtoken_string_to_wks(const char *string, int length); extern c_str_view hdrtoken_string_to_wks_sv(const char *string); diff --git a/src/proxy/hdrs/HdrToken.cc b/src/proxy/hdrs/HdrToken.cc index 4ce08d0037c..d167fb4d6db 100644 --- a/src/proxy/hdrs/HdrToken.cc +++ b/src/proxy/hdrs/HdrToken.cc @@ -323,7 +323,8 @@ hdrtoken_ascii_toupper(unsigned char c) constexpr uint32_t HDRTOKEN_HASH_SEED = 0x811c9dc5u; // FNV-1a 32-bit offset basis // The one hash function, shared by compile-time table construction and hdrtoken_tokenize(), so the -// two can never disagree. +// two can never disagree. hdrtoken_field_name_scan() folds the same steps inline because it +// discovers the length as it scans; a parity unit test pins it to this function. constexpr uint32_t hdrtoken_hash(std::string_view s) { @@ -674,21 +675,15 @@ hdrtoken_method_tokenize(const char *string, int string_len) /*------------------------------------------------------------------------- -------------------------------------------------------------------------*/ +// WKS lookup for a name whose FNV-1a hash the caller has already computed +// (e.g. fused into the field-name scan). Matches by slot, hash, and length like +// hdrtoken_tokenize, but skips the interned-pointer test, so it is only valid +// for a non-interned `string`. int -hdrtoken_tokenize(const char *string, int string_len, const char **wks_string_out) +hdrtoken_tokenize_prehashed(const char *string, int string_len, uint32_t hash, const char **wks_string_out) { ink_assert(string != nullptr); - - if (hdrtoken_is_wks(string)) { - int const wks_idx = hdrtoken_wks_to_index(string); - - if (wks_string_out) { - *wks_string_out = string; - } - return wks_idx; - } - - uint32_t const hash = hdrtoken_hash(std::string_view{string, static_cast(string_len)}); + ink_assert(!hdrtoken_is_wks(string)); HdrTokenHashBucket const &bucket = hdrtoken_hash_table[hash_to_slot(hash)]; @@ -708,6 +703,59 @@ hdrtoken_tokenize(const char *string, int string_len, const char **wks_string_ou return -1; } +/*------------------------------------------------------------------------- + -------------------------------------------------------------------------*/ + +// Single-pass field-name scan for the MIME parser. Scans up to `maxlen` bytes +// of `string` for the ':' delimiter while, in the same pass, accumulating the +// FNV-1a name hash (identical to hdrtoken_hash) and tracking whether every byte +// before ':' is a valid HTTP field-name char. Returns the index of ':' (i.e. +// the field-name length) or -1 if no ':' appears within `maxlen`. `*hash_out` +// and `*all_valid_out` describe the bytes scanned before ':' (or all `maxlen` +// bytes when ':' is absent); both are required. +int +hdrtoken_field_name_scan(const char *string, int maxlen, uint32_t *hash_out, bool *all_valid_out) +{ + uint32_t hval = HDRTOKEN_HASH_SEED; // same FNV-1a name hash as hdrtoken_hash + bool all_valid = true; + int i = 0; + + for (; i < maxlen; ++i) { + unsigned char const uc = static_cast(string[i]); + if (uc == ':') { + break; + } + hval = (hval ^ hdrtoken_ascii_toupper(uc)) * 0x01000193u; + all_valid &= (ParseRules::is_http_field_name(static_cast(uc)) != 0); + } + + *hash_out = hval; + *all_valid_out = all_valid; + return (i < maxlen) ? i : -1; +} + +/*------------------------------------------------------------------------- + -------------------------------------------------------------------------*/ + +int +hdrtoken_tokenize(const char *string, int string_len, const char **wks_string_out) +{ + ink_assert(string != nullptr); + + if (hdrtoken_is_wks(string)) { + int const wks_idx = hdrtoken_wks_to_index(string); + + if (wks_string_out) { + *wks_string_out = string; + } + return wks_idx; + } + + uint32_t const hash = hdrtoken_hash(std::string_view{string, static_cast(string_len)}); + + return hdrtoken_tokenize_prehashed(string, string_len, hash, wks_string_out); +} + /*------------------------------------------------------------------------- -------------------------------------------------------------------------*/ diff --git a/src/proxy/hdrs/MIME.cc b/src/proxy/hdrs/MIME.cc index 26aaeaa410b..466a2459841 100644 --- a/src/proxy/hdrs/MIME.cc +++ b/src/proxy/hdrs/MIME.cc @@ -2536,12 +2536,30 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, MIMEHdrImpl *mh, const char continue; // toss away garbage line } + // A line so long its size cannot be represented as int cannot yield a + // storable field; reject it rather than let the narrowing below wrap. + if (parsed.size() > static_cast(INT_MAX)) { + return ParseResult::ERROR; + } + // find name last - auto field_value = parsed; // need parsed as is later on. - auto field_name = field_value.split_prefix_at(':'); - if (field_name.empty()) { + // + // Fuse the colon scan, FNV-1a name hash, and per-byte field-name validation + // into one pass over the name bytes. hdrtoken_field_name_scan returns the + // colon index (the name length) and, for those bytes, the hash reused below + // by the WKS lookup, plus whether every byte is a valid HTTP field-name + // char. + auto field_value = parsed; // need parsed as is later on. + uint32_t field_name_hash; + bool name_all_valid; + int colon_idx = hdrtoken_field_name_scan(parsed.data(), static_cast(parsed.size()), &field_name_hash, &name_all_valid); + if (colon_idx <= 0) { + // colon_idx < 0: no colon; colon_idx == 0: empty name. Both are garbage, + // matching the old empty-field_name toss. continue; // toss away garbage line } + auto field_name = parsed.prefix(colon_idx); + field_value.remove_prefix(colon_idx + 1); // RFC7230 section 3.2.4: // No whitespace is allowed between the header field-name and colon. In @@ -2553,12 +2571,15 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, MIMEHdrImpl *mh, const char // A proxy MUST remove any such whitespace from a response message before // forwarding the message downstream. bool raw_print_field = true; + bool name_scan_stale = false; if (is_ws(field_name.back())) { if (!remove_ws_from_field_name) { return ParseResult::ERROR; } field_name.rtrim_if(&ParseRules::is_ws); raw_print_field = false; + // The fused scan hashed and validated the untrimmed name; recompute below. + name_scan_stale = true; } else if (parsed.suffix(2) != "\r\n" || (parsed.size() > 2 && parsed[parsed.size() - 3] == '\r')) { // Do not preserve malformed line endings when forwarding the field. raw_print_field = false; @@ -2597,14 +2618,23 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, MIMEHdrImpl *mh, const char // tokenize the name // /////////////////////// - int field_name_wks_idx = hdrtoken_tokenize(field_name.data(), field_name.size()); - - if (field_name_wks_idx < 0) { - for (auto i : field_name) { - if (!ParseRules::is_http_field_name(i)) { - return ParseResult::ERROR; + int field_name_wks_idx; + if (name_scan_stale) { + // BWS trimming shortened the name after the fused scan; redo the WKS + // lookup and byte validation over the trimmed name. + field_name_wks_idx = hdrtoken_tokenize(field_name.data(), field_name.size()); + if (field_name_wks_idx < 0) { + for (auto i : field_name) { + if (!ParseRules::is_http_field_name(i)) { + return ParseResult::ERROR; + } } } + } else { + field_name_wks_idx = hdrtoken_tokenize_prehashed(field_name.data(), static_cast(field_name.size()), field_name_hash); + if ((field_name_wks_idx < 0) && !name_all_valid) { + return ParseResult::ERROR; + } } // RFC 9110 Section 5.5. Field Values diff --git a/src/proxy/hdrs/unit_tests/test_mime.cc b/src/proxy/hdrs/unit_tests/test_mime.cc index 6a6d067a0c1..3d87e36549f 100644 --- a/src/proxy/hdrs/unit_tests/test_mime.cc +++ b/src/proxy/hdrs/unit_tests/test_mime.cc @@ -24,8 +24,11 @@ #include #include #include +#include +#include #include +#include using namespace std::literals; @@ -35,6 +38,8 @@ using namespace std::literals; #include "tscore/ink_platform.h" #include "tscore/Diags.h" #include "tscore/BaseLogFile.h" +#include "tscore/ParseRules.h" +#include "proxy/hdrs/HdrToken.h" #include "proxy/hdrs/MIME.h" #include "proxy/hdrs/HdrHeap.h" @@ -90,6 +95,142 @@ TEST_CASE("Mime", "[proxy][mime]") hdr.destroy(); } +TEST_CASE("MimeParserReuseAcrossHeaders", "[proxy][mimeparser]") +{ + // A parser reused on a different header without an intervening clear must + // still detect duplicates of fields already present in that header. The tail + // append derives its candidate from the header under parse, so a change of + // header cannot carry state over. If it instead trusted parser-held state, a + // wire duplicate of a pre-existing custom field would attach as an + // independent head rather than joining the dup chain. + MIMEParser parser; + mime_parser_init(&parser); + + // First header: parse a non-WKS field so the parser seeds its dup state. + MIMEHdr hdrA; + hdrA.create(nullptr); + { + std::string_view text = "X-Foo: 1\r\n\r\n"sv; + const char *start = text.data(); + REQUIRE(hdrA.parse(&parser, &start, text.data() + text.size(), true, false, false) == ParseResult::DONE); + } + + // Second header (different mh) already carries a live non-WKS field; reuse the + // same parser WITHOUT clearing it and parse a duplicate of that field. + MIMEHdr hdrB; + hdrB.create(nullptr); + MIMEField *pre = hdrB.field_create("X-Baz"sv); + pre->value_set(hdrB.m_heap, hdrB.m_mime, "a"sv); + hdrB.field_attach(pre); + { + std::string_view text = "X-Baz: b\r\n\r\n"sv; + const char *start = text.data(); + REQUIRE(hdrB.parse(&parser, &start, text.data() + text.size(), true, false, false) == ParseResult::DONE); + } + + // The wire field must have joined the pre-existing field's dup chain: exactly + // two X-Baz values reachable from the head. + MIMEField *head = hdrB.field_find("X-Baz"sv); + REQUIRE(head != nullptr); + int count = 0; + for (MIMEField *f = head; f != nullptr; f = f->m_next_dup) { + ++count; + } + CHECK(count == 2); + + mime_parser_clear(&parser); + hdrA.destroy(); + hdrB.destroy(); +} + +TEST_CASE("HdrTokenFusedNameScanParity", "[proxy][hdrtoken]") +{ + // hdrtoken_field_name_scan and hdrtoken_tokenize_prehashed fuse the colon + // scan, the FNV hash, and field-name validation into one pass. Verify the + // fused path agrees with references: the colon position, per-byte validity, + // and -- via the prehashed lookup fed the fused hash -- the well-known index + // the standalone tokenizer returns (which is a proxy for hash parity). + struct Case { + const char *name; + const char *tail; + }; + static const std::vector cases = { + {"Content-Length", ": 5" }, + {"content-length", ":5" }, + {"CONTENT-LENGTH", ":5" }, + {"Host", ": x" }, + {"hOsT", ":x" }, + {"Set-Cookie", ": a=b" }, + {"Cache-Control", ":no" }, + {"Transfer-Encoding", ":chunk" }, + {"@Ats-Internal", ":z" }, + {"X-Custom-Header", ": v" }, + {"sec-ch-ua", ": \"x\""}, + {"sec-fetch-mode", ":cors" }, + {"priority", ":u=1" }, + {"X-My-Header", ":v" }, + {"a", ":b" }, + }; + + for (auto const &c : cases) { + std::string const buf = std::string(c.name) + c.tail; + int const name_len = static_cast(strlen(c.name)); + uint32_t hash = 0; + bool valid = false; + int const colon = hdrtoken_field_name_scan(buf.data(), static_cast(buf.size()), &hash, &valid); + CAPTURE(c.name); + CHECK(colon == name_len); + + bool ref_valid = true; + for (int i = 0; i < name_len; ++i) { + if (!ParseRules::is_http_field_name(c.name[i])) { + ref_valid = false; + break; + } + } + CHECK(valid == ref_valid); + CHECK(hdrtoken_tokenize_prehashed(c.name, name_len, hash) == hdrtoken_tokenize(c.name, name_len)); + } + + // The cases above only pin hash parity where the name is in the token table; + // on a miss both tokenizers return -1 whatever hash they were handed. Sweep + // the whole table, in three case forms, so every entry is a live comparison. + for (int idx = 0; idx < hdrtoken_num_wks; ++idx) { + std::string const wks{hdrtoken_strs[idx], static_cast(hdrtoken_str_lengths[idx])}; + std::string upper{wks}, lower{wks}; + + for (auto &ch : upper) { + ch = static_cast(toupper(static_cast(ch))); + } + for (auto &ch : lower) { + ch = static_cast(tolower(static_cast(ch))); + } + + for (auto const &name : {wks, upper, lower}) { + std::string const buf = name + ":v"; + int const name_len = static_cast(name.size()); + uint32_t hash = 0; + bool valid = false; + int const colon = hdrtoken_field_name_scan(buf.data(), static_cast(buf.size()), &hash, &valid); + + CAPTURE(name); + CHECK(colon == name_len); + CHECK(hdrtoken_tokenize_prehashed(name.data(), name_len, hash) == hdrtoken_tokenize(name.data(), name_len)); + } + } + + // Edge cases: no colon, empty name, and an invalid byte in the name. + uint32_t h = 0; + bool v = false; + CHECK(hdrtoken_field_name_scan("NoColon", 7, &h, &v) < 0); + CHECK(hdrtoken_field_name_scan(":value", 6, &h, &v) == 0); + { + const char bad[] = {'X', '\x01', 'Y', ':', 'v'}; + CHECK(hdrtoken_field_name_scan(bad, 5, &h, &v) == 3); + CHECK(v == false); + } +} + TEST_CASE("MimeGetHostPortValues", "[proxy][mimeport]") { MIMEHdr hdr; From f58829fbc72b1b74d46be06aaa63afd575a1c5c9 Mon Sep 17 00:00:00 2001 From: Mo Chen Date: Sat, 11 Jul 2026 21:56:57 -0500 Subject: [PATCH 5/9] Skip redundant WKS duplicate searches A clear presence bit already proves that a well-known field has no duplicate in the header. Use that result when attaching parsed fields to avoid redundant lookup work. Well-known names without a presence mask and non-well-known names retain the normal duplicate search. --- src/proxy/hdrs/MIME.cc | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/proxy/hdrs/MIME.cc b/src/proxy/hdrs/MIME.cc index 466a2459841..01721b08b69 100644 --- a/src/proxy/hdrs/MIME.cc +++ b/src/proxy/hdrs/MIME.cc @@ -2651,7 +2651,20 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, MIMEHdrImpl *mh, const char MIMEField *field = mime_field_create_for_name(heap, mh, field_name); mime_field_name_value_set(heap, mh, field, field_name_wks_idx, field_name, field_value, raw_print_field, parsed.size(), false); - mime_hdr_field_attach(mh, field, 1, nullptr); + // A clear presence bit guarantees no duplicate exists. Skip the lookup. + // Names without a presence bit still need the normal duplicate check. + // + // mime_hdr_field_attach() uses field->name_get(), which returns an interned + // string for well-known names. mime_hdr_field_find() would then check the + // same presence bit and return nullptr. + int check_for_dups = 1; + if (field_name_wks_idx >= 0) { + uint64_t const mask = hdrtoken_index_to_mask(field_name_wks_idx); + if (mask != 0 && (mh->m_presence_bits & mask) == 0) { + check_for_dups = 0; + } + } + mime_hdr_field_attach(mh, field, check_for_dups, nullptr); } } From 0fe3be7ae8d7f9cc8684488bfb7ad657f2201a45 Mon Sep 17 00:00:00 2001 From: Mo Chen Date: Mon, 17 Aug 2026 16:17:00 -0500 Subject: [PATCH 6/9] Append adjacent duplicate fields in O(1) Consecutive fields such as Set-Cookie repeatedly search an existing duplicate chain even though their predecessor is already its tail. Derive that predecessor from the current header block and append directly, avoiding parser-held pointers that could outlive the header. Tests compare duplicate chains with normal attachment and cover parser reuse after a header is destroyed. --- src/proxy/hdrs/MIME.cc | 49 +++++++- src/proxy/hdrs/unit_tests/test_mime.cc | 155 +++++++++++++++++++++++++ 2 files changed, 203 insertions(+), 1 deletion(-) diff --git a/src/proxy/hdrs/MIME.cc b/src/proxy/hdrs/MIME.cc index 01721b08b69..ef8bca9ceef 100644 --- a/src/proxy/hdrs/MIME.cc +++ b/src/proxy/hdrs/MIME.cc @@ -2651,6 +2651,7 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, MIMEHdrImpl *mh, const char MIMEField *field = mime_field_create_for_name(heap, mh, field_name); mime_field_name_value_set(heap, mh, field, field_name_wks_idx, field_name, field_value, raw_print_field, parsed.size(), false); + // A clear presence bit guarantees no duplicate exists. Skip the lookup. // Names without a presence bit still need the normal duplicate check. // @@ -2664,7 +2665,53 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, MIMEHdrImpl *mh, const char check_for_dups = 0; } } - mime_hdr_field_attach(mh, field, check_for_dups, nullptr); + + // Append an adjacent duplicate in O(1), without searching its chain. + // The previous field must have the same name and be the chain's tail. + // Duplicate chains follow slot order, so the new field belongs after it. + // + // The pointer check below is required: mime_field_create_for_name() can + // reuse an older slot. Only use this shortcut when the new field occupies + // the last allocated slot in the tail block and has a predecessor there. + // Otherwise, fall back to normal attachment. + // + // Get the previous field from the current header. Do not cache it in the + // parser: the parser can be reused after its previous header is destroyed. + bool fast_tail_append = false; + MIMEFieldBlockImpl *const tail_fblock = mh->m_fblock_list_tail; + + if (tail_fblock->m_freetop >= 2 && &tail_fblock->m_field_slots[tail_fblock->m_freetop - 1] == field) { + MIMEField *const last = &tail_fblock->m_field_slots[tail_fblock->m_freetop - 2]; + + if (last->is_live() && last->m_next_dup == nullptr) { + bool name_matches; + + if (field_name_wks_idx >= 0) { + name_matches = (last->m_wks_idx == field_name_wks_idx); + } else { + name_matches = + (last->m_wks_idx < 0) && + ts::iequals(std::string_view{last->m_ptr_name, static_cast(last->m_len_name)}, field_name); + } + + if (name_matches) { + field->m_readiness = MIME_FIELD_SLOT_READINESS_LIVE; + field->m_flags = (field->m_flags & ~MIME_FIELD_SLOT_FLAGS_DUP_HEAD); + field->m_next_dup = nullptr; + last->m_next_dup = field; + // Presence bit and slot accelerator were set by the chain head; a tail + // dup leaves them untouched, matching attach's patch-after-prev branch. + if (field->m_ptr_value && field->is_cooked()) { + mh->recompute_cooked_stuff(field); + } + fast_tail_append = true; + } + } + } + + if (!fast_tail_append) { + mime_hdr_field_attach(mh, field, check_for_dups, nullptr); + } } } diff --git a/src/proxy/hdrs/unit_tests/test_mime.cc b/src/proxy/hdrs/unit_tests/test_mime.cc index 3d87e36549f..f752600db68 100644 --- a/src/proxy/hdrs/unit_tests/test_mime.cc +++ b/src/proxy/hdrs/unit_tests/test_mime.cc @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -143,6 +144,99 @@ TEST_CASE("MimeParserReuseAcrossHeaders", "[proxy][mimeparser]") hdrB.destroy(); } +TEST_CASE("MimeParserTailAppendEquivalence", "[proxy][mimeparser]") +{ + // The O(1) adjacent-duplicate tail append must produce the same field/dup + // structure as attach's full duplicate search. Build the same field sequence + // two ways -- via the parser (which takes the tail-append path) and via + // explicit create+attach (the reference full-attach path) -- and compare the + // dup chain of every name. + struct Field { + const char *name; + const char *value; + }; + auto scenario = GENERATE(from_range(std::vector>{ + {{"X-A", "1"}, {"X-A", "2"}, {"X-A", "3"}}, // consecutive custom dups + {{"X-A", "1"}, {"X-B", "2"}, {"X-A", "3"}}, // interleaved + {{"X-A", "1"}, {"X-A", "2"}, {"X-B", "3"}, {"X-B", "4"}}, // two adjacent runs + {{"Set-Cookie", "a"}, {"Set-Cookie", "b"}, {"Set-Cookie", "c"}, {"Set-Cookie", "d"}}, // well-known dups + {{"X-A", "1"}, {"X-B", "2"}, {"X-C", "3"}}, // no dups + })); + + // Parser-built header (tail-append path). + std::string raw; + for (auto const &f : scenario) { + raw += f.name; + raw += ": "; + raw += f.value; + raw += "\r\n"; + } + raw += "\r\n"; + MIMEParser parser; + mime_parser_init(&parser); + MIMEHdr hdrA; + hdrA.create(nullptr); + { + const char *start = raw.data(); + REQUIRE(hdrA.parse(&parser, &start, raw.data() + raw.size(), true, false, false) == ParseResult::DONE); + } + mime_parser_clear(&parser); + + // Reference header via explicit create+attach (attach's full path, no tail append). + MIMEHdr hdrB; + hdrB.create(nullptr); + for (auto const &f : scenario) { + MIMEField *fld = hdrB.field_create(std::string_view{f.name}); + fld->value_set(hdrB.m_heap, hdrB.m_mime, std::string_view{f.value}); + hdrB.field_attach(fld); + } + + // Compare structure, not just values. The tail append sets the dup-head flag + // and the well-known index itself instead of letting attach do it, and it + // must still raise the presence bit for a well-known name. + struct Slot { + std::string value; + bool dup_head; + int16_t wks_idx; + + bool + operator==(Slot const &o) const + { + return value == o.value && dup_head == o.dup_head && wks_idx == o.wks_idx; + } + }; + + auto collect = [](MIMEHdr &h, std::string_view n) { + std::vector slots; + for (MIMEField *fld = h.field_find(n); fld != nullptr; fld = fld->m_next_dup) { + auto v = fld->value_get(); + slots.push_back(Slot{std::string{v}, fld->is_dup_head() != 0, fld->m_wks_idx}); + } + return slots; + }; + + std::set names; + for (auto const &f : scenario) { + names.insert(f.name); + } + for (auto const &name : names) { + std::vector va = collect(hdrA, name); + std::vector vb = collect(hdrB, name); + CAPTURE(name, va.size(), vb.size()); + REQUIRE(va.size() == vb.size()); + for (size_t i = 0; i < va.size(); i++) { + CAPTURE(i, va[i].value, vb[i].value, va[i].dup_head, vb[i].dup_head, va[i].wks_idx, vb[i].wks_idx); + CHECK(va[i] == vb[i]); + } + } + + CHECK(hdrA.fields_count() == hdrB.fields_count()); + CHECK(hdrA.m_mime->m_presence_bits == hdrB.m_mime->m_presence_bits); + + hdrA.destroy(); + hdrB.destroy(); +} + TEST_CASE("HdrTokenFusedNameScanParity", "[proxy][hdrtoken]") { // hdrtoken_field_name_scan and hdrtoken_tokenize_prehashed fuse the colon @@ -733,3 +827,64 @@ TEST_CASE("MimeSetterBoolReturn", "[proxy][mime]") hdr.destroy(); } } + +TEST_CASE("MimeParserStaleLastAttachedAcrossDestroyedHeader", "[proxy][mimeparser]") +{ + // The O(1) tail append derives the preceding field from the header it was + // handed, and caches nothing in the parser. This test guards that property. + // + // An earlier version cached a MIMEField* in the MIMEParser. Because a parser + // outlives the headers it parses -- it is a member of Http2Stream, while + // reset_send_headers destroys and recreates the header before re-parsing the + // trailer block with the same parser -- that pointer could name a field of a + // destroyed header. A freed HdrHeap returns from the allocator at the same + // address, so the stale slot still read live, the new field was spliced onto + // it, and mime_hdr_field_attach never ran: no presence bit, no DUP_HEAD. The + // field printed on the wire but mime_hdr_field_find could not see it, and + // deleting it later walked a broken dup chain into a null dereference. + // + // Reuse one parser across a destroyed header and assert the invariant a cache + // would break: a parsed field must be a proper chain head, reachable through + // its presence bit. Iterate, because whether the freed heap comes back at the + // same address is up to the allocator. + MIMEParser parser; + mime_parser_init(&parser); + + for (int round = 0; round < 8; ++round) { + // Header A: parse TWO fields so the last-attached one sits at slot 1, then + // destroy A. m_last_attached now dangles at slot 1 of the freed block. + // Two fields matter: with only one, the recycled slot 0 aliases the field B + // is about to create, which is not LIVE yet, so the hazard is masked. + { + MIMEHdr a; + a.create(nullptr); + std::string_view text = "X-Pad: 0\r\nAge: 1\r\n\r\n"sv; + const char *start = text.data(); + REQUIRE(a.parse(&parser, &start, text.data() + text.size(), true, false, false) == ParseResult::DONE); + a.destroy(); + } + + // Header B: same parser, deliberately NOT cleared, same field name. + MIMEHdr b; + b.create(nullptr); + { + std::string_view text = "Age: 2\r\n\r\n"sv; + const char *start = text.data(); + REQUIRE(b.parse(&parser, &start, text.data() + text.size(), true, false, false) == ParseResult::DONE); + } + + CAPTURE(round); + // The presence bit is what mime_hdr_field_find short-circuits on, so losing + // it makes the field unreachable by interned name even though it prints. + CHECK(b.presence(MIME_PRESENCE_AGE) != 0); + const MIMEField *found = b.field_find(static_cast(MIME_FIELD_AGE)); + REQUIRE(found != nullptr); + CHECK(found->value_get_int64() == 2); + // A single field must be its own chain head with no dup successor. + CHECK(found->m_next_dup == nullptr); + + b.destroy(); + } + + mime_parser_clear(&parser); +} From 278d7eba24cfc1e9a50d625b3114355e3ce8142f Mon Sep 17 00:00:00 2001 From: Mo Chen Date: Mon, 21 Sep 2026 10:20:00 -0500 Subject: [PATCH 7/9] Reject invalid benchmark iteration counts Prevent zero iteration counts from producing meaningless profile results. Reject malformed, missing, negative, and overflowing counts before entering the profiling loop. --- tools/benchmark/benchmark_HdrParse.cc | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tools/benchmark/benchmark_HdrParse.cc b/tools/benchmark/benchmark_HdrParse.cc index 3644ea396be..8856c6f513e 100644 --- a/tools/benchmark/benchmark_HdrParse.cc +++ b/tools/benchmark/benchmark_HdrParse.cc @@ -49,10 +49,10 @@ #include "proxy/hdrs/HdrToken.h" #include "proxy/hdrs/HdrHeap.h" +#include #include #include #include -#include #include #include #include @@ -868,8 +868,18 @@ main(int argc, char *argv[]) std::string_view a = argv[i]; if (a == "--profile" && i + 1 < argc) { profile_target = argv[++i]; - } else if (a == "--iters" && i + 1 < argc) { - iters = std::strtoull(argv[++i], nullptr, 10); + } else if (a == "--iters") { + if (i + 1 == argc) { + std::fprintf(stderr, "--iters requires a positive integer\n"); + return 2; + } + std::string_view value = argv[++i]; + auto const [end, ec] = std::from_chars(value.data(), value.data() + value.size(), iters); + + if (ec != std::errc{} || end != value.data() + value.size() || iters == 0) { + std::fprintf(stderr, "invalid --iters '%s': expected a positive integer in the uint64_t range\n", argv[i]); + return 2; + } } else if (a == "--corpus-file" && i + 1 < argc) { corpus_files.emplace_back(argv[++i]); } else if (a == "--corpus-dir" && i + 1 < argc) { From 265a1399666cf75dbbaf900ecfe0d7a068597174 Mon Sep 17 00:00:00 2001 From: Mo Chen Date: Mon, 21 Sep 2026 17:10:53 -0500 Subject: [PATCH 8/9] Fix type casting for field name size in MIME.cc Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/proxy/hdrs/MIME.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/proxy/hdrs/MIME.cc b/src/proxy/hdrs/MIME.cc index ef8bca9ceef..70051e11535 100644 --- a/src/proxy/hdrs/MIME.cc +++ b/src/proxy/hdrs/MIME.cc @@ -2622,7 +2622,7 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, MIMEHdrImpl *mh, const char if (name_scan_stale) { // BWS trimming shortened the name after the fused scan; redo the WKS // lookup and byte validation over the trimmed name. - field_name_wks_idx = hdrtoken_tokenize(field_name.data(), field_name.size()); + field_name_wks_idx = hdrtoken_tokenize(field_name.data(), static_cast(field_name.size())); if (field_name_wks_idx < 0) { for (auto i : field_name) { if (!ParseRules::is_http_field_name(i)) { From a1f9812971aefe52181d74067f35219f346766ea Mon Sep 17 00:00:00 2001 From: Mo Chen Date: Mon, 28 Sep 2026 14:25:10 -0500 Subject: [PATCH 9/9] Address header parsing review follow-ups Catch shared token lookup failures and stale cooked Cache-Control state in the parser tests, with an explicit cctype dependency. Keep benchmark results meaningful by checking MIME parses, matching whitespace policy, and rejecting ignored profile arguments. --- src/proxy/hdrs/MIME.cc | 4 +- src/proxy/hdrs/URL.cc | 1 + src/proxy/hdrs/unit_tests/test_mime.cc | 11 ++++-- tools/benchmark/benchmark_HdrParse.cc | 55 ++++++++++++++++---------- 4 files changed, 46 insertions(+), 25 deletions(-) diff --git a/src/proxy/hdrs/MIME.cc b/src/proxy/hdrs/MIME.cc index 70051e11535..615dd2bf54a 100644 --- a/src/proxy/hdrs/MIME.cc +++ b/src/proxy/hdrs/MIME.cc @@ -2670,8 +2670,8 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, MIMEHdrImpl *mh, const char // The previous field must have the same name and be the chain's tail. // Duplicate chains follow slot order, so the new field belongs after it. // - // The pointer check below is required: mime_field_create_for_name() can - // reuse an older slot. Only use this shortcut when the new field occupies + // mime_field_create_for_name() can reuse an older slot. The pointer check + // below limits this shortcut to cases where the new field occupies // the last allocated slot in the tail block and has a predecessor there. // Otherwise, fall back to normal attachment. // diff --git a/src/proxy/hdrs/URL.cc b/src/proxy/hdrs/URL.cc index 96abbe8cc57..06392c50851 100644 --- a/src/proxy/hdrs/URL.cc +++ b/src/proxy/hdrs/URL.cc @@ -1206,6 +1206,7 @@ url_is_strictly_compliant(const char *start, const char *end) bool url_is_mostly_compliant(const char *start, const char *end) { + // Accumulate invalid bytes without an early exit so the compiler can vectorize the scan. unsigned char bad = 0; for (const char *i = start; i < end; ++i) { unsigned char const c = static_cast(*i); diff --git a/src/proxy/hdrs/unit_tests/test_mime.cc b/src/proxy/hdrs/unit_tests/test_mime.cc index f752600db68..adc35202f71 100644 --- a/src/proxy/hdrs/unit_tests/test_mime.cc +++ b/src/proxy/hdrs/unit_tests/test_mime.cc @@ -21,6 +21,7 @@ limitations under the License. */ +#include #include #include #include @@ -160,6 +161,7 @@ TEST_CASE("MimeParserTailAppendEquivalence", "[proxy][mimeparser]") {{"X-A", "1"}, {"X-B", "2"}, {"X-A", "3"}}, // interleaved {{"X-A", "1"}, {"X-A", "2"}, {"X-B", "3"}, {"X-B", "4"}}, // two adjacent runs {{"Set-Cookie", "a"}, {"Set-Cookie", "b"}, {"Set-Cookie", "c"}, {"Set-Cookie", "d"}}, // well-known dups + {{"Cache-Control", "no-cache"}, {"Cache-Control", "max-age=5"}}, // cooked state on an adjacent duplicate {{"X-A", "1"}, {"X-B", "2"}, {"X-C", "3"}}, // no dups })); @@ -232,6 +234,8 @@ TEST_CASE("MimeParserTailAppendEquivalence", "[proxy][mimeparser]") CHECK(hdrA.fields_count() == hdrB.fields_count()); CHECK(hdrA.m_mime->m_presence_bits == hdrB.m_mime->m_presence_bits); + CHECK(hdrA.get_cooked_cc_mask() == hdrB.get_cooked_cc_mask()); + CHECK(hdrA.get_cooked_cc_max_age() == hdrB.get_cooked_cc_max_age()); hdrA.destroy(); hdrB.destroy(); @@ -294,10 +298,10 @@ TEST_CASE("HdrTokenFusedNameScanParity", "[proxy][hdrtoken]") std::string upper{wks}, lower{wks}; for (auto &ch : upper) { - ch = static_cast(toupper(static_cast(ch))); + ch = static_cast(std::toupper(static_cast(ch))); } for (auto &ch : lower) { - ch = static_cast(tolower(static_cast(ch))); + ch = static_cast(std::tolower(static_cast(ch))); } for (auto const &name : {wks, upper, lower}) { @@ -309,7 +313,8 @@ TEST_CASE("HdrTokenFusedNameScanParity", "[proxy][hdrtoken]") CAPTURE(name); CHECK(colon == name_len); - CHECK(hdrtoken_tokenize_prehashed(name.data(), name_len, hash) == hdrtoken_tokenize(name.data(), name_len)); + CHECK(hdrtoken_tokenize_prehashed(name.data(), name_len, hash) == idx); + CHECK(hdrtoken_tokenize(name.data(), name_len) == idx); } } diff --git a/tools/benchmark/benchmark_HdrParse.cc b/tools/benchmark/benchmark_HdrParse.cc index 8856c6f513e..cf93bde894f 100644 --- a/tools/benchmark/benchmark_HdrParse.cc +++ b/tools/benchmark/benchmark_HdrParse.cc @@ -20,9 +20,10 @@ be supplied with --corpus-file FILE or --corpus-dir DIR (blocks split on a blank line, classified request vs response by the first line); these are added to both modes, and each loaded request's request-target also feeds the url - target. In stats mode each target also gets a "corpus" benchmark that times one - pass over the loaded cases alone, so its result reflects only the supplied - traffic. + target. In stats mode the request, response, mime, and url targets also get a + "corpus" benchmark that times one pass over the loaded cases alone, so its + result reflects only the supplied traffic. The wks benchmarks combine field + names from the built-in and loaded cases. @section license License @@ -410,6 +411,11 @@ constexpr int PROD_STRICT = 2; // proxy.config.http.strict_uri_parsing defa using ParseOutcome = std::pair; +struct ParseInput { + std::string_view data; + bool remove_ws_from_field_name{false}; +}; + ParseOutcome drive_request(std::string_view raw, bool copy = PROD_COPY, int strict = PROD_STRICT) { @@ -442,7 +448,7 @@ drive_response(std::string_view raw, bool copy = PROD_COPY) } ParseOutcome -drive_mime(std::string_view fields, bool copy = PROD_COPY) +drive_mime(std::string_view fields, bool remove_ws_from_field_name = false, bool copy = PROD_COPY) { MIMEParser parser; mime_parser_init(&parser); @@ -451,7 +457,7 @@ drive_mime(std::string_view fields, bool copy = PROD_COPY) hdr.create(heap); const char *start = fields.data(); ParseResult ret = mime_parser_parse(&parser, hdr.m_heap, hdr.m_mime, &start, fields.data() + fields.size(), copy, - /*eof*/ true, /*remove_ws_from_field_name*/ false); + /*eof*/ true, remove_ws_from_field_name); uint64_t sink = static_cast(start - fields.data()); hdr.destroy(); return {ret, sink}; @@ -541,11 +547,11 @@ int run_profile(Target target, uint64_t iters) { // Assemble the input set and a per-iteration byte count for throughput. - std::vector inputs; - uint64_t bytes_per_pass = 0; + std::vector inputs; + uint64_t bytes_per_pass = 0; - auto add_input = [&](std::string_view v) { - inputs.push_back(v); + auto add_input = [&](std::string_view v, bool remove_ws_from_field_name = false) { + inputs.push_back({v, remove_ws_from_field_name}); bytes_per_pass += v.size(); }; @@ -566,7 +572,7 @@ run_profile(Target target, uint64_t iters) break; case Target::Mime: for (const auto &c : g_corpus.cases) { - add_input(strip_start_line(c.data)); + add_input(strip_start_line(c.data), c.is_response); } break; case Target::Url: @@ -610,20 +616,20 @@ run_profile(Target target, uint64_t iters) count = iters; // one full pass over all names per iter } else { for (uint64_t i = 0; i < iters; ++i) { - std::string_view in = inputs[i % inputs.size()]; - ParseOutcome r; + ParseInput const &in = inputs[i % inputs.size()]; + ParseOutcome r; switch (target) { case Target::Request: - r = drive_request(in); + r = drive_request(in.data); break; case Target::Response: - r = drive_response(in); + r = drive_response(in.data); break; case Target::Mime: - r = drive_mime(in); + r = drive_mime(in.data, in.remove_ws_from_field_name); break; case Target::Url: - r = drive_url(in); + r = drive_url(in.data); break; default: break; @@ -777,10 +783,15 @@ TEST_CASE("hdr parse: mime only", "[bench][mime]") const auto &realistic = find_case("req_realistic"); const auto &wksmiss = find_case("adv_wks_miss"); - std::vector loaded; + std::vector loaded; for (const auto &c : g_corpus.cases) { + CAPTURE(c.label); + std::string_view const fields = strip_start_line(c.data); + ParseOutcome const result = drive_mime(fields, c.is_response); + REQUIRE(result.first == ParseResult::DONE); + REQUIRE(result.second == fields.size()); if (is_file_case(c)) { - loaded.push_back(strip_start_line(c.data)); + loaded.push_back({fields, c.is_response}); } } @@ -796,8 +807,8 @@ TEST_CASE("hdr parse: mime only", "[bench][mime]") BENCHMARK("mime: corpus (" + std::to_string(loaded.size()) + " blocks)") { uint64_t sink = 0; - for (auto fields : loaded) { - sink += drive_mime(fields).second; + for (ParseInput const &fields : loaded) { + sink += drive_mime(fields.data, fields.remove_ws_from_field_name).second; } return sink; }; @@ -900,6 +911,10 @@ main(int argc, char *argv[]) } if (!profile_target.empty()) { + if (catch_args.size() > 1) { + std::fprintf(stderr, "unexpected argument in profile mode: %s\n", catch_args[1]); + return 2; + } Target t = parse_target(profile_target); if (t == Target::Unknown) { std::fprintf(stderr, "unknown target '%s' (want: request|response|mime|url|wks|wks-lower)\n", profile_target.c_str());