This is an automated email from the ASF dual-hosted git repository.
masaori335 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 51ba3a4def Fix truncated HTTP version in log field unmarshalling
(#13236)
51ba3a4def is described below
commit 51ba3a4def82c06803842ae8d47dd366222739a6
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
---
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 c4a58a888a..6a870db96c 100644
--- a/include/proxy/logging/LogAccess.h
+++ b/include/proxy/logging/LogAccess.h
@@ -316,10 +316,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);
@@ -348,7 +344,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);
@@ -358,7 +353,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 1de1217bfa..7d2f43593a 100644
--- a/src/proxy/logging/LogAccess.cc
+++ b/src/proxy/logging/LogAccess.cc
@@ -1097,6 +1097,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) {
@@ -1107,42 +1108,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");
+}
