This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.2.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 54c0d61323d04cc2b39004f22a35e9ab2ec1d039 Author: Masaori Koshiba <[email protected]> AuthorDate: Mon Jun 15 08:41:11 2026 +0900 Fix truncated HTTP version in log field unmarshalling (#13236) * Fix truncated HTTP version in log field unmarshalling unmarshal_http_version did not advance the buffer pointer past the minor-version digits before computing the output length, so logged HTTP versions dropped the minor number ("HTTP/1." instead of "HTTP/1.1"). This is a regression from #11437, which changed val_len to a pointer subtraction without advancing past the second integer. * Remove dead LogAccess marshal/unmarshal declarations These functions have no callers, and several are declared without ever being defined; none are paired with a log field: - unmarshal_http_text: no marshal counterpart; referenced only by its own declaration and (former) definition. - marshal_milestone_fmt_squid/_netscape/_date/_time: declared but never defined. Only _fmt_sec and _fmt_ms are implemented and used; the date/time/netscape/squid formatting happens at unmarshal time. - unmarshal_client_protocol_stack: declared but never defined or wired into any log field. * Add test cases for 2.0 and 3.0 (cherry picked from commit 51ba3a4def82c06803842ae8d47dd366222739a6) --- include/proxy/logging/LogAccess.h | 6 ----- src/proxy/logging/LogAccess.cc | 37 +------------------------- src/proxy/logging/unit-tests/test_LogAccess.cc | 22 +++++++++++++++ 3 files changed, 23 insertions(+), 42 deletions(-) diff --git a/include/proxy/logging/LogAccess.h b/include/proxy/logging/LogAccess.h index dccb620ba3..e59b49a8f2 100644 --- a/include/proxy/logging/LogAccess.h +++ b/include/proxy/logging/LogAccess.h @@ -312,10 +312,6 @@ public: // int marshal_milestone(TSMilestonesType ms, char *buf); int marshal_milestone_fmt_sec(TSMilestonesType ms, char *buf); - int marshal_milestone_fmt_squid(TSMilestonesType ms, char *buf); - int marshal_milestone_fmt_netscape(TSMilestonesType ms, char *buf); - int marshal_milestone_fmt_date(TSMilestonesType ms, char *buf); - int marshal_milestone_fmt_time(TSMilestonesType ms, char *buf); int marshal_milestone_fmt_ms(TSMilestonesType ms, char *buf); int marshal_milestone_diff(TSMilestonesType ms1, TSMilestonesType ms2, char *buf); int marshal_milestones_csv(char *buf); @@ -340,7 +336,6 @@ public: static int unmarshal_int_to_time_str(char **buf, char *dest, int len); static int unmarshal_int_to_netscape_str(char **buf, char *dest, int len); static int unmarshal_http_version(char **buf, char *dest, int len); - static int unmarshal_http_text(char **buf, char *dest, int len, LogSlice *slice, LogEscapeType escape_type); static int unmarshal_http_status(char **buf, char *dest, int len); static int unmarshal_ip(char **buf, IpEndpoint *dest); static int unmarshal_ip_to_str(char **buf, char *dest, int len); @@ -350,7 +345,6 @@ public: static int unmarshal_cache_code(char **buf, char *dest, int len, const Ptr<LogFieldAliasMap> &map); static int unmarshal_cache_hit_miss(char **buf, char *dest, int len, const Ptr<LogFieldAliasMap> &map); static int unmarshal_cache_write_code(char **buf, char *dest, int len, const Ptr<LogFieldAliasMap> &map); - static int unmarshal_client_protocol_stack(char **buf, char *dest, int len, Ptr<LogFieldAliasMap> map); static int unmarshal_with_map(int64_t code, char *dest, int len, const Ptr<LogFieldAliasMap> &map, const char *msg = nullptr); diff --git a/src/proxy/logging/LogAccess.cc b/src/proxy/logging/LogAccess.cc index ec8eb803f3..662a8b5daa 100644 --- a/src/proxy/logging/LogAccess.cc +++ b/src/proxy/logging/LogAccess.cc @@ -1068,6 +1068,7 @@ LogAccess::unmarshal_http_version(char **buf, char *dest, int len) DBG_UNMARSHAL_DEST_OVERRUN return -1; } + p += res2; int val_len = p - val_buf; if (val_len < len) { @@ -1078,42 +1079,6 @@ LogAccess::unmarshal_http_version(char **buf, char *dest, int len) return -1; } -/*------------------------------------------------------------------------- - LogAccess::unmarshal_http_text - - The http text is a reproduced HTTP/1.x request line. It's HTTP method (cqhm) + URL (pqu) + HTTP version. - This doesn't support HTTP/2 and HTTP/3 since those don't have a request line. - -------------------------------------------------------------------------*/ - -int -LogAccess::unmarshal_http_text(char **buf, char *dest, int len, LogSlice *slice, LogEscapeType escape_type) -{ - ink_assert(buf != nullptr); - ink_assert(*buf != nullptr); - ink_assert(dest != nullptr); - - char *p = dest; - - // int res1 = unmarshal_http_method (buf, p, len); - int res1 = unmarshal_str(buf, p, len, nullptr, escape_type); - if (res1 < 0) { - return -1; - } - p += res1; - *p++ = ' '; - int res2 = unmarshal_str(buf, p, len - res1 - 1, slice, escape_type); - if (res2 < 0) { - return -1; - } - p += res2; - *p++ = ' '; - int res3 = unmarshal_http_version(buf, p, len - res1 - res2 - 2); - if (res3 < 0) { - return -1; - } - return res1 + res2 + res3 + 2; -} - /*------------------------------------------------------------------------- LogAccess::unmarshal_http_status diff --git a/src/proxy/logging/unit-tests/test_LogAccess.cc b/src/proxy/logging/unit-tests/test_LogAccess.cc index 94f6f8f91d..a3158aba4d 100644 --- a/src/proxy/logging/unit-tests/test_LogAccess.cc +++ b/src/proxy/logging/unit-tests/test_LogAccess.cc @@ -26,6 +26,7 @@ #include "proxy/NonHttpSmLogData.h" #include "proxy/logging/LogAccess.h" #include "proxy/logging/TransactionLogData.h" +#include "tscore/ink_align.h" #include "tscore/ink_inet.h" #include <string_view> @@ -213,3 +214,24 @@ TEST_CASE("LogAccess non-HttpSM client host port is null-safe", "[LogAccess]") CHECK(access.marshal_client_host_port(nullptr) == INK_MIN_ALIGN); CHECK(marshal_int_value([&](char *buf) { return access.marshal_client_host_port(buf); }) == 4321); } + +TEST_CASE("LogAccess unmarshal_http_version keeps the minor version", "[LogAccess]") +{ + auto render = [](int64_t major, int64_t minor) -> std::string { + char marshalled[2 * INK_MIN_ALIGN]; + LogAccess::marshal_int(marshalled, major); + LogAccess::marshal_int(marshalled + INK_MIN_ALIGN, minor); + + char dest[64] = {}; + char *src = marshalled; + int len = LogAccess::unmarshal_http_version(&src, dest, sizeof(dest)); + REQUIRE(len > 0); + return std::string(dest, len); + }; + + CHECK(render(1, 1) == "HTTP/1.1"); + CHECK(render(1, 0) == "HTTP/1.0"); + // known bug of `.0` suffix for HTTP/2 and HTTP/3 + CHECK(render(2, 0) == "HTTP/2.0"); + CHECK(render(3, 0) == "HTTP/3.0"); +}
