Skip to content

Fix truncated HTTP version in log field unmarshalling #13236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
masaori335 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix truncated HTTP version in log field unmarshalling #13236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44976f4
Fix truncated HTTP version in log field unmarshalling
masaori335 Jun 3, 2026
820f5f6
Remove dead LogAccess marshal/unmarshal declarations
masaori335 Jun 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions include/proxy/logging/LogAccess.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,10 +311,6 @@ class LogAccess
//
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);
Expand Down Expand Up @@ -343,7 +339,6 @@ class LogAccess
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);
Expand All @@ -353,7 +348,6 @@ class LogAccess
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);

Expand Down
37 changes: 1 addition & 36 deletions src/proxy/logging/LogAccess.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,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) {
Expand All @@ -1085,42 +1086,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

Expand Down
19 changes: 19 additions & 0 deletions src/proxy/logging/unit-tests/test_LogAccess.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -213,3 +214,21 @@ 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");
}