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 afcf8d02e4 Rename: PreTransactionLogData -> NonHttpSmLogData (#13154)
afcf8d02e4 is described below
commit afcf8d02e4dfb074ba5b74082edd783d84166ecb
Author: Brian Neradt <[email protected]>
AuthorDate: Mon Jun 1 15:00:24 2026 -0500
Rename: PreTransactionLogData -> NonHttpSmLogData (#13154)
The alternate-HttpSM access log entries have more opportunities for use
than just pre-HttpSM state machine uses. There may be uses for it, for
instance, to log TLS handshake issues, for instance. Thus the old
pre-transaction terminology was too narrow.
This renames the exceptional carrier to NonHttpSmLogData, documents
that normal transaction logs should still use HttpSM-backed data, and
updates the protocol logging path and tests to use the new terminology.
---
...{PreTransactionLogData.h => NonHttpSmLogData.h} | 35 +++++++++------
include/proxy/ProxyTransaction.h | 9 ++--
include/proxy/logging/Log.h | 2 +-
include/proxy/logging/LogAccess.h | 4 +-
include/proxy/logging/TransactionLogData.h | 18 ++++----
src/proxy/ProxyTransaction.cc | 6 +--
src/proxy/http2/Http2ConnectionState.cc | 4 +-
src/proxy/http3/Http3HeaderVIOAdaptor.cc | 4 +-
src/proxy/logging/TransactionLogData.cc | 50 +++++++++++-----------
src/proxy/logging/unit-tests/test_LogAccess.cc | 22 +++++-----
10 files changed, 82 insertions(+), 72 deletions(-)
diff --git a/include/proxy/PreTransactionLogData.h
b/include/proxy/NonHttpSmLogData.h
similarity index 61%
rename from include/proxy/PreTransactionLogData.h
rename to include/proxy/NonHttpSmLogData.h
index 0ec03c646b..552e09622e 100644
--- a/include/proxy/PreTransactionLogData.h
+++ b/include/proxy/NonHttpSmLogData.h
@@ -1,7 +1,7 @@
/** @file
- PreTransactionLogData populates LogData for requests that fail before
- HttpSM creation.
+ NonHttpSmLogData populates LogData for access-log entries that cannot be
+ backed by an HttpSM.
@section license License
@@ -30,23 +30,32 @@
#include <string>
-/** Populate LogData for requests that never created an HttpSM.
+/** Owns access-log data for entries that cannot be backed by an @c HttpSM.
*
- * Malformed HTTP/2 or HTTP/3 request headers can be rejected while the
- * connection is still decoding and validating the stream, before the request
- * progresses far enough to create an HttpSM. This class carries the
- * copied request and session metadata needed to emit a best-effort
- * transaction log entry for those failures.
+ * Normal transaction access logging is expected to use data extracted from a
+ * live @c HttpSM. This type is for exceptional client-facing failures that
need
+ * transaction-log visibility but occur before an @c HttpSM exists, such as
+ * malformed HTTP/2 or HTTP/3 request headers rejected during protocol
+ * validation. It may also be used for connection-level failures, such as TLS
+ * handshake errors, when operators need those events in the access log.
*
- * This class owns its milestones, addresses, and strings because the
- * originating stream is about to be destroyed.
+ * Because the protocol stream or connection state may be destroyed immediately
+ * after the failure is handled, this object owns the copied headers,
+ * addresses, milestones, protocol strings, and outcome fields needed by
+ * @c LogAccess. Fields that require an @c HttpSM, origin transaction, cache
+ * lookup, or server response are intentionally left unset and marshal through
+ * the normal default values.
+ *
+ * This path should remain narrow. If an @c HttpSM exists, prefer the standard
+ * @c HttpSM-backed logging path so normal transactions do not pay for extra
+ * copying or exceptional state.
*/
-class PreTransactionLogData
+class NonHttpSmLogData
{
public:
- PreTransactionLogData() = default;
+ NonHttpSmLogData() = default;
- ~PreTransactionLogData()
+ ~NonHttpSmLogData()
{
if (owned_client_request.valid()) {
owned_client_request.destroy();
diff --git a/include/proxy/ProxyTransaction.h b/include/proxy/ProxyTransaction.h
index 7316be293a..32b24c6b5b 100644
--- a/include/proxy/ProxyTransaction.h
+++ b/include/proxy/ProxyTransaction.h
@@ -146,18 +146,19 @@ public:
void mark_as_tunnel_endpoint() override;
- /** Emit a best-effort access log entry for a request that failed before
- * HttpSM creation.
+ /** Emit a best-effort access log entry for a request without an HttpSM.
*
* Call this when a malformed request is rejected at the protocol layer
* (e.g. during HTTP/2 or HTTP/3 header decoding) and no HttpSM was
- * created. The method populates a PreTransactionLogData from the
+ * created. The method populates a NonHttpSmLogData from the
* session and the partially decoded request, then invokes Log::access.
+ * If an HttpSM exists, callers should use the normal transaction logging
+ * path instead.
*
* @param[in] request The decoded (possibly partial) request header.
* @param[in] protocol_str Protocol string for the log entry (e.g. "http/2").
*/
- void log_pre_transaction_access(HTTPHdr const *request, const char
*protocol_str);
+ void log_non_http_sm_access(HTTPHdr const *request, const char
*protocol_str);
/// Variables
//
diff --git a/include/proxy/logging/Log.h b/include/proxy/logging/Log.h
index 20019bf6ce..fffc8adf2f 100644
--- a/include/proxy/logging/Log.h
+++ b/include/proxy/logging/Log.h
@@ -43,7 +43,7 @@
@section example Example usage of the API
@code
- // Populate a TransactionLogData, then:
+ // Populate a TransactionLogData source, then:
LogAccess entry(data);
int ret = Log::access(&entry);
@endcode
diff --git a/include/proxy/logging/LogAccess.h
b/include/proxy/logging/LogAccess.h
index 51d80471f8..c78fcd988d 100644
--- a/include/proxy/logging/LogAccess.h
+++ b/include/proxy/logging/LogAccess.h
@@ -123,8 +123,8 @@ public:
* The caller retains ownership of @a data, which must outlive the
* synchronous Log::access() call that marshals this entry.
*
- * @param[in] data Populated TransactionLogData for a completed or
- * pre-transaction entry.
+ * @param[in] data Populated TransactionLogData for an HttpSM-backed or
+ * non-HttpSM entry.
*/
explicit LogAccess(TransactionLogData &data);
diff --git a/include/proxy/logging/TransactionLogData.h
b/include/proxy/logging/TransactionLogData.h
index 0db209e5a2..128c0d67a3 100644
--- a/include/proxy/logging/TransactionLogData.h
+++ b/include/proxy/logging/TransactionLogData.h
@@ -31,20 +31,20 @@
#include <string_view>
class HttpSM;
-class PreTransactionLogData;
+class NonHttpSmLogData;
-/** Provide access-log data from either a completed HttpSM or pre-transaction
storage.
+/** Provide access-log data from either a completed HttpSM or non-HttpSM
storage.
*
* The common completed-transaction path reads directly from @c HttpSM. The
- * rare pre-transaction path reads from @c PreTransactionLogData, which owns
- * copied request/session state for malformed requests rejected before HttpSM
- * creation.
+ * rare non-HttpSM path reads from @c NonHttpSmLogData, which owns copied
+ * request/session state for exceptional access-log entries that cannot be
+ * backed by an @c HttpSM.
*/
class TransactionLogData
{
public:
explicit TransactionLogData(HttpSM *sm);
- explicit TransactionLogData(PreTransactionLogData const &pre_data);
+ explicit TransactionLogData(NonHttpSmLogData const &non_http_sm_data);
void *http_sm_for_plugins() const;
@@ -186,7 +186,7 @@ public:
// ===== Server response Transfer-Encoding =====
std::string_view get_server_response_transfer_encoding() const;
- // ===== Fallback fields for pre-transaction logging =====
+ // ===== Fallback fields for non-HttpSM logging =====
std::string_view get_method() const;
std::string_view get_scheme() const;
std::string_view get_client_protocol_str() const;
@@ -195,8 +195,8 @@ public:
TransactionLogData &operator=(const TransactionLogData &) = delete;
private:
- HttpSM *m_http_sm = nullptr;
- PreTransactionLogData const *m_pre_data = nullptr;
+ HttpSM *m_http_sm = nullptr;
+ NonHttpSmLogData const *m_non_http_sm_data = nullptr;
// Cached values for fields that require computation or string formatting.
mutable char m_client_rx_error_code[10] = {'-', '\0'};
diff --git a/src/proxy/ProxyTransaction.cc b/src/proxy/ProxyTransaction.cc
index c2a5a4584e..a301ce20de 100644
--- a/src/proxy/ProxyTransaction.cc
+++ b/src/proxy/ProxyTransaction.cc
@@ -23,7 +23,7 @@
#include "proxy/http/HttpSM.h"
#include "proxy/Plugin.h"
-#include "proxy/PreTransactionLogData.h"
+#include "proxy/NonHttpSmLogData.h"
#include "proxy/logging/LogAccess.h"
#include "proxy/logging/TransactionLogData.h"
#include "proxy/logging/Log.h"
@@ -343,7 +343,7 @@ get_pseudo_header_value(HTTPHdr const &hdr,
std::string_view name)
} // end anonymous namespace
void
-ProxyTransaction::log_pre_transaction_access(HTTPHdr const *request, const
char *protocol_str)
+ProxyTransaction::log_non_http_sm_access(HTTPHdr const *request, const char
*protocol_str)
{
if (get_sm() != nullptr) {
return;
@@ -358,7 +358,7 @@ ProxyTransaction::log_pre_transaction_access(HTTPHdr const
*request, const char
return;
}
- PreTransactionLogData data;
+ NonHttpSmLogData data;
data.owned_client_request.create(HTTPType::REQUEST, request->version_get());
data.owned_client_request.copy(request);
diff --git a/src/proxy/http2/Http2ConnectionState.cc
b/src/proxy/http2/Http2ConnectionState.cc
index 34effdf60b..9c80aefe29 100644
--- a/src/proxy/http2/Http2ConnectionState.cc
+++ b/src/proxy/http2/Http2ConnectionState.cc
@@ -497,7 +497,7 @@ Http2ConnectionState::rcv_headers_frame(const Http2Frame
&frame)
"recv headers enhance your calm");
} else {
if (!stream->trailing_header_is_possible() &&
!stream->is_outbound_connection()) {
- stream->log_pre_transaction_access(stream->get_receive_header(),
"http/2");
+ stream->log_non_http_sm_access(stream->get_receive_header(),
"http/2");
}
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM,
Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
"recv headers malformed request");
@@ -1112,7 +1112,7 @@ Http2ConnectionState::rcv_continuation_frame(const
Http2Frame &frame)
"continuation enhance your calm");
} else {
if (!stream->trailing_header_is_possible() &&
!stream->is_outbound_connection()) {
- stream->log_pre_transaction_access(stream->get_receive_header(),
"http/2");
+ stream->log_non_http_sm_access(stream->get_receive_header(),
"http/2");
}
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION,
Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
"continuation malformed request");
diff --git a/src/proxy/http3/Http3HeaderVIOAdaptor.cc
b/src/proxy/http3/Http3HeaderVIOAdaptor.cc
index 396733d625..9d3e256b7f 100644
--- a/src/proxy/http3/Http3HeaderVIOAdaptor.cc
+++ b/src/proxy/http3/Http3HeaderVIOAdaptor.cc
@@ -110,7 +110,7 @@ Http3HeaderVIOAdaptor::_on_qpack_decode_complete()
NON_TRAILER)) {
Dbg(dbg_ctl_http3, "Header is invalid");
if (this->_txn != nullptr) {
- this->_txn->log_pre_transaction_access(&this->_header, "http/3");
+ this->_txn->log_non_http_sm_access(&this->_header, "http/3");
}
return -1;
}
@@ -118,7 +118,7 @@ Http3HeaderVIOAdaptor::_on_qpack_decode_complete()
if (res != 0) {
Dbg(dbg_ctl_http3, "ParseResult::ERROR");
if (this->_txn != nullptr) {
- this->_txn->log_pre_transaction_access(&this->_header, "http/3");
+ this->_txn->log_non_http_sm_access(&this->_header, "http/3");
}
return -1;
}
diff --git a/src/proxy/logging/TransactionLogData.cc
b/src/proxy/logging/TransactionLogData.cc
index b1b50e3730..841e7a8fee 100644
--- a/src/proxy/logging/TransactionLogData.cc
+++ b/src/proxy/logging/TransactionLogData.cc
@@ -22,7 +22,7 @@
*/
#include "proxy/logging/TransactionLogData.h"
-#include "proxy/PreTransactionLogData.h"
+#include "proxy/NonHttpSmLogData.h"
#include "proxy/http/HttpSM.h"
#include "proxy/logging/LogAccess.h"
#include "proxy/hdrs/MIME.h"
@@ -92,7 +92,7 @@ TransactionLogData::TransactionLogData(HttpSM *sm) :
m_http_sm(sm)
ink_assert(sm != nullptr);
}
-TransactionLogData::TransactionLogData(PreTransactionLogData const &pre_data)
: m_pre_data(&pre_data) {}
+TransactionLogData::TransactionLogData(NonHttpSmLogData const
&non_http_sm_data) : m_non_http_sm_data(&non_http_sm_data) {}
void *
TransactionLogData::http_sm_for_plugins() const
@@ -111,7 +111,7 @@ TransactionLogData::get_milestones() const
if (likely(m_http_sm != nullptr)) {
return &m_http_sm->milestones;
}
- return &m_pre_data->owned_milestones;
+ return &m_non_http_sm_data->owned_milestones;
}
// ===== Headers =====
@@ -127,8 +127,8 @@ TransactionLogData::get_client_request() const
return nullptr;
}
- if (m_pre_data->owned_client_request.valid()) {
- return const_cast<HTTPHdr *>(&m_pre_data->owned_client_request);
+ if (m_non_http_sm_data->owned_client_request.valid()) {
+ return const_cast<HTTPHdr *>(&m_non_http_sm_data->owned_client_request);
}
return nullptr;
}
@@ -207,7 +207,7 @@ TransactionLogData::get_client_req_url_str() const
cache_url_strings();
return m_client_req_url_str;
}
- return m_pre_data->owned_url.empty() ? nullptr :
m_pre_data->owned_url.c_str();
+ return m_non_http_sm_data->owned_url.empty() ? nullptr :
m_non_http_sm_data->owned_url.c_str();
}
int
@@ -217,7 +217,7 @@ TransactionLogData::get_client_req_url_len() const
cache_url_strings();
return m_client_req_url_len;
}
- return static_cast<int>(m_pre_data->owned_url.size());
+ return static_cast<int>(m_non_http_sm_data->owned_url.size());
}
const char *
@@ -227,7 +227,7 @@ TransactionLogData::get_client_req_url_path_str() const
cache_url_strings();
return m_client_req_url_path_str;
}
- return m_pre_data->owned_path.empty() ? nullptr :
m_pre_data->owned_path.c_str();
+ return m_non_http_sm_data->owned_path.empty() ? nullptr :
m_non_http_sm_data->owned_path.c_str();
}
int
@@ -237,7 +237,7 @@ TransactionLogData::get_client_req_url_path_len() const
cache_url_strings();
return m_client_req_url_path_len;
}
- return static_cast<int>(m_pre_data->owned_path.size());
+ return static_cast<int>(m_non_http_sm_data->owned_path.size());
}
// ===== Proxy response content-type / reason =====
@@ -384,7 +384,7 @@ TransactionLogData::get_client_addr() const
if (likely(m_http_sm != nullptr)) {
return &m_http_sm->t_state.effective_client_addr.sa;
}
- return &m_pre_data->owned_client_addr.sa;
+ return &m_non_http_sm_data->owned_client_addr.sa;
}
sockaddr const *
@@ -393,7 +393,7 @@ TransactionLogData::get_client_src_addr() const
if (likely(m_http_sm != nullptr)) {
return &m_http_sm->t_state.client_info.src_addr.sa;
}
- return &m_pre_data->owned_client_src_addr.sa;
+ return &m_non_http_sm_data->owned_client_src_addr.sa;
}
sockaddr const *
@@ -402,7 +402,7 @@ TransactionLogData::get_client_dst_addr() const
if (likely(m_http_sm != nullptr)) {
return &m_http_sm->t_state.client_info.dst_addr.sa;
}
- return &m_pre_data->owned_client_dst_addr.sa;
+ return &m_non_http_sm_data->owned_client_dst_addr.sa;
}
sockaddr const *
@@ -428,7 +428,7 @@ TransactionLogData::get_client_port() const
}
return 0;
}
- return m_pre_data->m_client_port;
+ return m_non_http_sm_data->m_client_port;
}
// ===== Server addressing =====
@@ -483,7 +483,7 @@ TransactionLogData::get_log_code() const
if (likely(m_http_sm != nullptr)) {
return m_http_sm->t_state.squid_codes.log_code;
}
- return m_pre_data->m_log_code;
+ return m_non_http_sm_data->m_log_code;
}
SquidSubcode
@@ -501,7 +501,7 @@ TransactionLogData::get_hit_miss_code() const
if (likely(m_http_sm != nullptr)) {
return m_http_sm->t_state.squid_codes.hit_miss_code;
}
- return m_pre_data->m_hit_miss_code;
+ return m_non_http_sm_data->m_hit_miss_code;
}
SquidHierarchyCode
@@ -510,7 +510,7 @@ TransactionLogData::get_hier_code() const
if (likely(m_http_sm != nullptr)) {
return m_http_sm->t_state.squid_codes.hier_code;
}
- return m_pre_data->m_hier_code;
+ return m_non_http_sm_data->m_hier_code;
}
// ===== Byte counters =====
@@ -595,7 +595,7 @@ TransactionLogData::get_connection_id() const
if (likely(m_http_sm != nullptr)) {
return m_http_sm->client_connection_id();
}
- return m_pre_data->m_connection_id;
+ return m_non_http_sm_data->m_connection_id;
}
int
@@ -604,7 +604,7 @@ TransactionLogData::get_transaction_id() const
if (likely(m_http_sm != nullptr)) {
return m_http_sm->client_transaction_id();
}
- return m_pre_data->m_transaction_id;
+ return m_non_http_sm_data->m_transaction_id;
}
int
@@ -653,7 +653,7 @@ TransactionLogData::get_client_protocol() const
if (likely(m_http_sm != nullptr)) {
return m_http_sm->get_user_agent().get_client_protocol();
}
- return m_pre_data->owned_client_protocol_str.empty() ? nullptr :
m_pre_data->owned_client_protocol_str.c_str();
+ return m_non_http_sm_data->owned_client_protocol_str.empty() ? nullptr :
m_non_http_sm_data->owned_client_protocol_str.c_str();
}
const char *
@@ -744,7 +744,7 @@ TransactionLogData::get_client_connection_is_ssl() const
if (likely(m_http_sm != nullptr)) {
return m_http_sm->get_user_agent().get_client_connection_is_ssl();
}
- return m_pre_data->m_client_connection_is_ssl;
+ return m_non_http_sm_data->m_client_connection_is_ssl;
}
bool
@@ -815,7 +815,7 @@ TransactionLogData::get_server_transact_count() const
if (likely(m_http_sm != nullptr)) {
return m_http_sm->server_transact_count;
}
- return m_pre_data->m_server_transact_count;
+ return m_non_http_sm_data->m_server_transact_count;
}
// ===== Finish status =====
@@ -1097,7 +1097,7 @@
TransactionLogData::get_server_response_transfer_encoding() const
return {};
}
-// ===== Fallback fields for pre-transaction logging =====
+// ===== Fallback fields for non-HttpSM logging =====
std::string_view
TransactionLogData::get_method() const
@@ -1105,7 +1105,7 @@ TransactionLogData::get_method() const
if (likely(m_http_sm != nullptr)) {
return {};
}
- return m_pre_data->owned_method;
+ return m_non_http_sm_data->owned_method;
}
std::string_view
@@ -1114,7 +1114,7 @@ TransactionLogData::get_scheme() const
if (likely(m_http_sm != nullptr)) {
return {};
}
- return m_pre_data->owned_scheme;
+ return m_non_http_sm_data->owned_scheme;
}
std::string_view
@@ -1123,5 +1123,5 @@ TransactionLogData::get_client_protocol_str() const
if (likely(m_http_sm != nullptr)) {
return {};
}
- return m_pre_data->owned_client_protocol_str;
+ return m_non_http_sm_data->owned_client_protocol_str;
}
diff --git a/src/proxy/logging/unit-tests/test_LogAccess.cc
b/src/proxy/logging/unit-tests/test_LogAccess.cc
index aee2bd6728..94f6f8f91d 100644
--- a/src/proxy/logging/unit-tests/test_LogAccess.cc
+++ b/src/proxy/logging/unit-tests/test_LogAccess.cc
@@ -23,7 +23,7 @@
#include <catch2/catch_test_macros.hpp>
-#include "proxy/PreTransactionLogData.h"
+#include "proxy/NonHttpSmLogData.h"
#include "proxy/logging/LogAccess.h"
#include "proxy/logging/TransactionLogData.h"
#include "tscore/ink_inet.h"
@@ -101,8 +101,8 @@ set_socket_address(IpEndpoint &ep, std::string_view text)
}
void
-populate_pre_transaction_data(PreTransactionLogData &data, std::string_view
method, std::string_view scheme,
- std::string_view authority, std::string_view
path)
+populate_non_http_sm_data(NonHttpSmLogData &data, std::string_view method,
std::string_view scheme, std::string_view authority,
+ std::string_view path)
{
initialize_headers_once();
@@ -164,10 +164,10 @@ marshal_int_value(Marshal marshal)
}
} // namespace
-TEST_CASE("LogAccess pre-transaction CONNECT fields", "[LogAccess]")
+TEST_CASE("LogAccess non-HttpSM CONNECT fields", "[LogAccess]")
{
- PreTransactionLogData data;
- populate_pre_transaction_data(data, "CONNECT", ""sv, "example.com:443",
""sv);
+ NonHttpSmLogData data;
+ populate_non_http_sm_data(data, "CONNECT", ""sv, "example.com:443", ""sv);
TransactionLogData log_data(data);
LogAccess access(log_data);
@@ -187,8 +187,8 @@ TEST_CASE("LogAccess pre-transaction CONNECT fields",
"[LogAccess]")
TEST_CASE("LogAccess malformed CONNECT without authority falls back to path",
"[LogAccess]")
{
- PreTransactionLogData data;
- populate_pre_transaction_data(data, "CONNECT", "https"sv, ""sv, "/"sv);
+ NonHttpSmLogData data;
+ populate_non_http_sm_data(data, "CONNECT", "https"sv, ""sv, "/"sv);
TransactionLogData log_data(data);
LogAccess access(log_data);
@@ -201,10 +201,10 @@ TEST_CASE("LogAccess malformed CONNECT without authority
falls back to path", "[
CHECK(marshal_int_value([&](char *buf) { return
access.marshal_transfer_time_ms(buf); }) == 5);
}
-TEST_CASE("LogAccess pre-transaction client host port is null-safe",
"[LogAccess]")
+TEST_CASE("LogAccess non-HttpSM client host port is null-safe", "[LogAccess]")
{
- PreTransactionLogData data;
- populate_pre_transaction_data(data, "GET", "https"sv, "example.com",
"/client-port"sv);
+ NonHttpSmLogData data;
+ populate_non_http_sm_data(data, "GET", "https"sv, "example.com",
"/client-port"sv);
TransactionLogData log_data(data);
LogAccess access(log_data);
