bneradt commented on code in PR #13838:
URL: https://github.com/apache/trafficserver/pull/13838#discussion_r4470302676
##########
plugins/experimental/jax_fingerprint/plugin.cc:
##########
@@ -447,6 +524,12 @@ TSPluginInit(int argc, char const **argv)
} else {
TSHttpHookAdd(TS_HTTP_TXN_CLOSE_HOOK, cont);
}
+ if (config->session_user_arg_index != -1) {
+ if (config->method.type == Method::Type::CONNECTION_BASED) {
+ TSHttpHookAdd(TS_HTTP_SSN_START_HOOK, cont);
Review Comment:
Confirmed and fixed in 2daf16c211. With JA3/JA4 `--log-field` on an HTTP/3
port, ATS segfaulted on the first connection in
`ProxySession::state_api_callout`, called from `HQSession::new_connection`.
`HQSession` now takes the connection mutex, as HTTP/1 and HTTP/2 sessions do
(`Http3Session.cc`). The new `test_jax_fingerprint_log_field_http3` reproduces
the crash without that line.
##########
plugins/experimental/jax_fingerprint/userarg.cc:
##########
@@ -26,95 +26,235 @@
#include "fingerprint_registry.h"
#include <cstring>
+#include <vector>
namespace
{
+// All plugin instances share this slot. It is deliberately not named after
--export or described with
+// REGISTRY_DESCRIPTION, so other plugins do not mistake it for the
fingerprint registry.
+constexpr char SESSION_ARG_NAME[] = "jax_fingerprint.log_field";
+constexpr char SESSION_ARG_DESCRIPTION[] = "jax_fingerprint private
--log-field fingerprint copies";
+
+/** The fingerprints the --log-field callbacks of all plugin instances saved
on a session. */
+class SessionFingerprints
+{
+public:
+ void
+ set(std::string_view method_name, std::string_view fingerprint,
std::optional<uint64_t> txn_id)
+ {
+ Entry *entry = find(method_name);
+ if (entry == nullptr) {
+ entry = &_entries.emplace_back();
+ entry->method_name = method_name;
+ }
+ entry->fingerprint = fingerprint;
+ entry->txn_id = txn_id;
+ }
+
+ std::string const *
+ get(std::string_view method_name, std::optional<uint64_t> txn_id) const
+ {
+ for (auto const &entry : _entries) {
+ if (entry.method_name == method_name) {
+ return entry.txn_id == txn_id ? &entry.fingerprint : nullptr;
+ }
+ }
+ return nullptr;
+ }
+
+ /** Remove the copy for @a method_name.
+ *
+ * @return true if no copies remain.
+ */
+ bool
+ remove(std::string_view method_name)
+ {
+ std::erase_if(_entries, [method_name](Entry const &entry) { return
entry.method_name == method_name; });
+ return _entries.empty();
+ }
+
+private:
+ struct Entry {
+ // References a PluginConfig method name, which outlives every session.
+ std::string_view method_name;
+ std::string fingerprint;
+ std::optional<uint64_t> txn_id;
+ };
+
+ Entry *
+ find(std::string_view method_name)
+ {
+ for (auto &entry : _entries) {
+ if (entry.method_name == method_name) {
+ return &entry;
+ }
+ }
+ return nullptr;
+ }
+
+ std::vector<Entry> _entries;
+};
+
char const *
user_arg_type_name(TSUserArgType type)
{
switch (type) {
case TS_USER_ARGS_VCONN:
return "vconn";
+ case TS_USER_ARGS_SSN:
+ return "ssn";
case TS_USER_ARGS_TXN:
return "txn";
default:
return "unknown";
}
}
-} // anonymous namespace
-
int
-reserve_user_arg(PluginConfig &config)
+reserve_slot(TSUserArgType type, PluginConfig const &config, int &index)
{
- TSUserArgType type = config.method.type == Method::Type::CONNECTION_BASED ?
TS_USER_ARGS_VCONN : TS_USER_ARGS_TXN;
- const char *name = config.export_name.empty() ? PLUGIN_NAME :
config.export_name.c_str();
+ const char *name = config.export_name.empty() ? PLUGIN_NAME :
config.export_name.c_str();
const char *description = nullptr;
- if (TSUserArgIndexNameLookup(type, name, &config.user_arg_index,
&description) == TS_SUCCESS) {
+ if (TSUserArgIndexNameLookup(type, name, &index, &description) ==
TS_SUCCESS) {
if (description == nullptr || std::strcmp(description,
jax_fingerprint::REGISTRY_DESCRIPTION) != 0) {
TSError("[%s] User arg '%s' is already reserved with an incompatible
data contract", PLUGIN_NAME, name);
return TS_ERROR;
}
- } else if (TSUserArgIndexReserve(type, name,
jax_fingerprint::REGISTRY_DESCRIPTION, &config.user_arg_index) == TS_SUCCESS) {
- Dbg(dbg_ctl, "Reserved shared user_arg slot: type=%s, name=%s, index=%d",
user_arg_type_name(type), name,
- config.user_arg_index);
+ } else if (TSUserArgIndexReserve(type, name,
jax_fingerprint::REGISTRY_DESCRIPTION, &index) == TS_SUCCESS) {
+ Dbg(dbg_ctl, "Reserved shared user_arg slot: type=%s, name=%s, index=%d",
user_arg_type_name(type), name, index);
} else {
Dbg(dbg_ctl, "Failed to reserve shared user_arg slot: type=%s, name=%s",
user_arg_type_name(type), name);
return TS_ERROR;
}
Dbg(dbg_ctl, "Using shared user_arg: type=%s, name=%s, method=%.*s,
index=%d", user_arg_type_name(type), name,
- static_cast<int>(config.method.name.size()), config.method.name.data(),
config.user_arg_index);
+ static_cast<int>(config.method.name.size()), config.method.name.data(),
index);
return TS_SUCCESS;
}
+ContextMap *
+get_map(void *container, int index)
+{
+ auto *registry = static_cast<jax_fingerprint::RegistryV1
*>(TSUserArgGet(container, index));
+ return registry == nullptr ? nullptr : static_cast<ContextMap *>(registry);
+}
+
void
-set_user_arg(void *container, PluginConfig &config, JAxContext *ctx)
+set_context(void *container, int index, std::string_view method_name,
JAxContext *ctx)
{
- auto *registry = static_cast<jax_fingerprint::RegistryV1
*>(TSUserArgGet(container, config.user_arg_index));
- ContextMap *map = registry == nullptr ? nullptr :
static_cast<ContextMap *>(registry);
+ ContextMap *map = get_map(container, index);
if (map == nullptr) {
map = new ContextMap();
- TSUserArgSet(container, config.user_arg_index,
static_cast<jax_fingerprint::RegistryV1 *>(map));
+ TSUserArgSet(container, index, static_cast<jax_fingerprint::RegistryV1
*>(map));
}
- map->set(config.method.name, ctx);
+ map->set(method_name, ctx);
}
JAxContext *
-get_user_arg(void *container, PluginConfig &config)
+get_context(void *container, int index, std::string_view method_name)
{
- auto *registry = static_cast<jax_fingerprint::RegistryV1
*>(TSUserArgGet(container, config.user_arg_index));
- ContextMap *map = registry == nullptr ? nullptr :
static_cast<ContextMap *>(registry);
- if (map == nullptr) {
- return nullptr;
+ ContextMap *map = get_map(container, index);
+ return map == nullptr ? nullptr : map->get(method_name);
+}
+
+void
+cleanup_context(void *container, int index, std::string_view method_name)
+{
+ ContextMap *map = get_map(container, index);
+ if (map != nullptr) {
+ // Remove this plugin's context from the map.
+ map->remove(method_name);
+
+ // If the map is now empty, delete it and clear the user arg.
+ if (map->empty()) {
+ delete map;
+ TSUserArgSet(container, index, nullptr);
+ }
}
- return map->get(config.method.name);
+}
+
+} // anonymous namespace
+
+int
+reserve_user_arg(PluginConfig &config)
+{
+ TSUserArgType type = config.method.type == Method::Type::CONNECTION_BASED ?
TS_USER_ARGS_VCONN : TS_USER_ARGS_TXN;
+ return reserve_slot(type, config, config.user_arg_index);
+}
+
+void
+set_user_arg(void *container, PluginConfig &config, JAxContext *ctx)
+{
+ set_context(container, config.user_arg_index, config.method.name, ctx);
+}
+
+JAxContext *
+get_user_arg(void *container, PluginConfig &config)
+{
+ return get_context(container, config.user_arg_index, config.method.name);
}
void
refresh_user_arg(void *container, PluginConfig &config)
{
- auto *registry = static_cast<jax_fingerprint::RegistryV1
*>(TSUserArgGet(container, config.user_arg_index));
- if (registry != nullptr) {
- static_cast<ContextMap *>(registry)->refresh(config.method.name);
+ if (ContextMap *map = get_map(container, config.user_arg_index); map !=
nullptr) {
+ map->refresh(config.method.name);
}
}
void
cleanup_user_arg(void *container, PluginConfig &config)
{
- auto *registry = static_cast<jax_fingerprint::RegistryV1
*>(TSUserArgGet(container, config.user_arg_index));
- ContextMap *map = registry == nullptr ? nullptr :
static_cast<ContextMap *>(registry);
- if (map != nullptr) {
- // Remove this plugin's context from the map.
- map->remove(config.method.name);
+ cleanup_context(container, config.user_arg_index, config.method.name);
+}
- // If the map is now empty, delete it and clear the user arg.
- if (map->empty()) {
- delete map;
- TSUserArgSet(container, config.user_arg_index, nullptr);
+int
+reserve_session_user_arg(PluginConfig &config)
+{
+ int &index = config.session_user_arg_index;
+ const char *description = nullptr;
+
+ if (TSUserArgIndexNameLookup(TS_USER_ARGS_SSN, SESSION_ARG_NAME, &index,
&description) == TS_SUCCESS) {
+ if (description == nullptr || std::strcmp(description,
SESSION_ARG_DESCRIPTION) != 0) {
+ TSError("[%s] User arg '%s' is already reserved by another plugin",
PLUGIN_NAME, SESSION_ARG_NAME);
+ return TS_ERROR;
}
+ } else if (TSUserArgIndexReserve(TS_USER_ARGS_SSN, SESSION_ARG_NAME,
SESSION_ARG_DESCRIPTION, &index) != TS_SUCCESS) {
+ Dbg(dbg_ctl, "Failed to reserve the session user_arg slot: name=%s",
SESSION_ARG_NAME);
+ return TS_ERROR;
+ }
+
+ Dbg(dbg_ctl, "Using session user_arg: name=%s, method=%.*s, index=%d",
SESSION_ARG_NAME,
+ static_cast<int>(config.method.name.size()), config.method.name.data(),
index);
+ return TS_SUCCESS;
+}
+
+void
+save_session_fingerprint(TSHttpSsn ssnp, PluginConfig &config,
std::string_view fingerprint, std::optional<uint64_t> txn_id)
+{
+ auto *copies = static_cast<SessionFingerprints *>(TSUserArgGet(ssnp,
config.session_user_arg_index));
+ if (copies == nullptr) {
+ copies = new SessionFingerprints();
+ TSUserArgSet(ssnp, config.session_user_arg_index, copies);
Review Comment:
Confirmed: `HQSession` never fires SSN_CLOSE, and making it do so means
reworking how `Http3App` deletes the session, which is out of scope here. In
2daf16c211 `save_session_fingerprint` no longer creates copies on a QUIC
session, so nothing leaks. JA4H logs `-` over HTTP/3, and the docs say so.
JA3/JA4 are not generated for QUIC anyway. The HTTP/3 test checks that no
copies are created, and both log-field tests check that every copy created is
freed.
##########
plugins/experimental/jax_fingerprint/userarg.cc:
##########
@@ -26,95 +26,235 @@
#include "fingerprint_registry.h"
#include <cstring>
+#include <vector>
namespace
{
+// All plugin instances share this slot. It is deliberately not named after
--export or described with
+// REGISTRY_DESCRIPTION, so other plugins do not mistake it for the
fingerprint registry.
+constexpr char SESSION_ARG_NAME[] = "jax_fingerprint.log_field";
+constexpr char SESSION_ARG_DESCRIPTION[] = "jax_fingerprint private
--log-field fingerprint copies";
+
+/** The fingerprints the --log-field callbacks of all plugin instances saved
on a session. */
+class SessionFingerprints
+{
+public:
+ void
+ set(std::string_view method_name, std::string_view fingerprint,
std::optional<uint64_t> txn_id)
+ {
+ Entry *entry = find(method_name);
+ if (entry == nullptr) {
+ entry = &_entries.emplace_back();
+ entry->method_name = method_name;
+ }
+ entry->fingerprint = fingerprint;
+ entry->txn_id = txn_id;
+ }
+
+ std::string const *
+ get(std::string_view method_name, std::optional<uint64_t> txn_id) const
+ {
+ for (auto const &entry : _entries) {
+ if (entry.method_name == method_name) {
+ return entry.txn_id == txn_id ? &entry.fingerprint : nullptr;
+ }
+ }
+ return nullptr;
+ }
+
+ /** Remove the copy for @a method_name.
+ *
+ * @return true if no copies remain.
+ */
+ bool
+ remove(std::string_view method_name)
+ {
+ std::erase_if(_entries, [method_name](Entry const &entry) { return
entry.method_name == method_name; });
+ return _entries.empty();
+ }
+
+private:
+ struct Entry {
+ // References a PluginConfig method name, which outlives every session.
+ std::string_view method_name;
+ std::string fingerprint;
+ std::optional<uint64_t> txn_id;
+ };
+
+ Entry *
+ find(std::string_view method_name)
+ {
+ for (auto &entry : _entries) {
+ if (entry.method_name == method_name) {
+ return &entry;
+ }
+ }
+ return nullptr;
+ }
+
+ std::vector<Entry> _entries;
+};
+
char const *
user_arg_type_name(TSUserArgType type)
{
switch (type) {
case TS_USER_ARGS_VCONN:
return "vconn";
+ case TS_USER_ARGS_SSN:
+ return "ssn";
case TS_USER_ARGS_TXN:
return "txn";
default:
return "unknown";
}
}
-} // anonymous namespace
-
int
-reserve_user_arg(PluginConfig &config)
+reserve_slot(TSUserArgType type, PluginConfig const &config, int &index)
{
- TSUserArgType type = config.method.type == Method::Type::CONNECTION_BASED ?
TS_USER_ARGS_VCONN : TS_USER_ARGS_TXN;
- const char *name = config.export_name.empty() ? PLUGIN_NAME :
config.export_name.c_str();
+ const char *name = config.export_name.empty() ? PLUGIN_NAME :
config.export_name.c_str();
const char *description = nullptr;
- if (TSUserArgIndexNameLookup(type, name, &config.user_arg_index,
&description) == TS_SUCCESS) {
+ if (TSUserArgIndexNameLookup(type, name, &index, &description) ==
TS_SUCCESS) {
if (description == nullptr || std::strcmp(description,
jax_fingerprint::REGISTRY_DESCRIPTION) != 0) {
TSError("[%s] User arg '%s' is already reserved with an incompatible
data contract", PLUGIN_NAME, name);
return TS_ERROR;
}
- } else if (TSUserArgIndexReserve(type, name,
jax_fingerprint::REGISTRY_DESCRIPTION, &config.user_arg_index) == TS_SUCCESS) {
- Dbg(dbg_ctl, "Reserved shared user_arg slot: type=%s, name=%s, index=%d",
user_arg_type_name(type), name,
- config.user_arg_index);
+ } else if (TSUserArgIndexReserve(type, name,
jax_fingerprint::REGISTRY_DESCRIPTION, &index) == TS_SUCCESS) {
+ Dbg(dbg_ctl, "Reserved shared user_arg slot: type=%s, name=%s, index=%d",
user_arg_type_name(type), name, index);
} else {
Dbg(dbg_ctl, "Failed to reserve shared user_arg slot: type=%s, name=%s",
user_arg_type_name(type), name);
return TS_ERROR;
}
Dbg(dbg_ctl, "Using shared user_arg: type=%s, name=%s, method=%.*s,
index=%d", user_arg_type_name(type), name,
- static_cast<int>(config.method.name.size()), config.method.name.data(),
config.user_arg_index);
+ static_cast<int>(config.method.name.size()), config.method.name.data(),
index);
return TS_SUCCESS;
}
+ContextMap *
+get_map(void *container, int index)
+{
+ auto *registry = static_cast<jax_fingerprint::RegistryV1
*>(TSUserArgGet(container, index));
+ return registry == nullptr ? nullptr : static_cast<ContextMap *>(registry);
+}
+
void
-set_user_arg(void *container, PluginConfig &config, JAxContext *ctx)
+set_context(void *container, int index, std::string_view method_name,
JAxContext *ctx)
{
- auto *registry = static_cast<jax_fingerprint::RegistryV1
*>(TSUserArgGet(container, config.user_arg_index));
- ContextMap *map = registry == nullptr ? nullptr :
static_cast<ContextMap *>(registry);
+ ContextMap *map = get_map(container, index);
if (map == nullptr) {
map = new ContextMap();
- TSUserArgSet(container, config.user_arg_index,
static_cast<jax_fingerprint::RegistryV1 *>(map));
+ TSUserArgSet(container, index, static_cast<jax_fingerprint::RegistryV1
*>(map));
}
- map->set(config.method.name, ctx);
+ map->set(method_name, ctx);
}
JAxContext *
-get_user_arg(void *container, PluginConfig &config)
+get_context(void *container, int index, std::string_view method_name)
{
- auto *registry = static_cast<jax_fingerprint::RegistryV1
*>(TSUserArgGet(container, config.user_arg_index));
- ContextMap *map = registry == nullptr ? nullptr :
static_cast<ContextMap *>(registry);
- if (map == nullptr) {
- return nullptr;
+ ContextMap *map = get_map(container, index);
+ return map == nullptr ? nullptr : map->get(method_name);
+}
+
+void
+cleanup_context(void *container, int index, std::string_view method_name)
+{
+ ContextMap *map = get_map(container, index);
+ if (map != nullptr) {
+ // Remove this plugin's context from the map.
+ map->remove(method_name);
+
+ // If the map is now empty, delete it and clear the user arg.
+ if (map->empty()) {
+ delete map;
+ TSUserArgSet(container, index, nullptr);
+ }
}
- return map->get(config.method.name);
+}
+
+} // anonymous namespace
+
+int
+reserve_user_arg(PluginConfig &config)
+{
+ TSUserArgType type = config.method.type == Method::Type::CONNECTION_BASED ?
TS_USER_ARGS_VCONN : TS_USER_ARGS_TXN;
+ return reserve_slot(type, config, config.user_arg_index);
+}
+
+void
+set_user_arg(void *container, PluginConfig &config, JAxContext *ctx)
+{
+ set_context(container, config.user_arg_index, config.method.name, ctx);
+}
+
+JAxContext *
+get_user_arg(void *container, PluginConfig &config)
+{
+ return get_context(container, config.user_arg_index, config.method.name);
}
void
refresh_user_arg(void *container, PluginConfig &config)
{
- auto *registry = static_cast<jax_fingerprint::RegistryV1
*>(TSUserArgGet(container, config.user_arg_index));
- if (registry != nullptr) {
- static_cast<ContextMap *>(registry)->refresh(config.method.name);
+ if (ContextMap *map = get_map(container, config.user_arg_index); map !=
nullptr) {
+ map->refresh(config.method.name);
}
}
void
cleanup_user_arg(void *container, PluginConfig &config)
{
- auto *registry = static_cast<jax_fingerprint::RegistryV1
*>(TSUserArgGet(container, config.user_arg_index));
- ContextMap *map = registry == nullptr ? nullptr :
static_cast<ContextMap *>(registry);
- if (map != nullptr) {
- // Remove this plugin's context from the map.
- map->remove(config.method.name);
+ cleanup_context(container, config.user_arg_index, config.method.name);
+}
- // If the map is now empty, delete it and clear the user arg.
- if (map->empty()) {
- delete map;
- TSUserArgSet(container, config.user_arg_index, nullptr);
+int
+reserve_session_user_arg(PluginConfig &config)
+{
+ int &index = config.session_user_arg_index;
+ const char *description = nullptr;
+
+ if (TSUserArgIndexNameLookup(TS_USER_ARGS_SSN, SESSION_ARG_NAME, &index,
&description) == TS_SUCCESS) {
+ if (description == nullptr || std::strcmp(description,
SESSION_ARG_DESCRIPTION) != 0) {
+ TSError("[%s] User arg '%s' is already reserved by another plugin",
PLUGIN_NAME, SESSION_ARG_NAME);
+ return TS_ERROR;
}
+ } else if (TSUserArgIndexReserve(TS_USER_ARGS_SSN, SESSION_ARG_NAME,
SESSION_ARG_DESCRIPTION, &index) != TS_SUCCESS) {
+ Dbg(dbg_ctl, "Failed to reserve the session user_arg slot: name=%s",
SESSION_ARG_NAME);
+ return TS_ERROR;
+ }
+
+ Dbg(dbg_ctl, "Using session user_arg: name=%s, method=%.*s, index=%d",
SESSION_ARG_NAME,
+ static_cast<int>(config.method.name.size()), config.method.name.data(),
index);
+ return TS_SUCCESS;
+}
+
+void
+save_session_fingerprint(TSHttpSsn ssnp, PluginConfig &config,
std::string_view fingerprint, std::optional<uint64_t> txn_id)
+{
+ auto *copies = static_cast<SessionFingerprints *>(TSUserArgGet(ssnp,
config.session_user_arg_index));
+ if (copies == nullptr) {
+ copies = new SessionFingerprints();
+ TSUserArgSet(ssnp, config.session_user_arg_index, copies);
+ }
+ copies->set(config.method.name, fingerprint, txn_id);
Review Comment:
Confirmed and fixed in 2daf16c211. Entries are now keyed by the export
registry slot (`config.user_arg_index`) and the method, the same pair that
identifies a context. The new `log-field-per-export` variant runs two JA4
instances with different `--export`/`--servernames`. It fails without the fix:
the non-matching field logged the other instance's fingerprint.
##########
plugins/experimental/jax_fingerprint/plugin.cc:
##########
@@ -310,11 +310,82 @@ handle_read_request_hdr(void *edata, PluginConfig &config)
return TS_SUCCESS;
}
+int
+handle_ssn_start(void *edata, PluginConfig &config)
+{
+ TSHttpSsn ssnp = static_cast<TSHttpSsn>(edata);
+
+ if (TSVConn vconn = TSHttpSsnClientVConnGet(ssnp); vconn != nullptr) {
+ if (JAxContext *ctx = get_user_arg(vconn, config); ctx != nullptr) {
+ save_session_fingerprint(ssnp, config, ctx->get_fingerprint());
+ }
+ }
+
+ TSHttpSsnReenable(ssnp, TS_EVENT_HTTP_CONTINUE);
+ return TS_SUCCESS;
+}
+
+int
+handle_ssn_close(void *edata, PluginConfig &config)
+{
+ TSHttpSsn ssnp = static_cast<TSHttpSsn>(edata);
+
+ // HQSession never fires SSN_CLOSE, so on HTTP/3 the copy saved by the last
transaction is not freed.
+ cleanup_session_user_arg(ssnp, config);
+
+ TSHttpSsnReenable(ssnp, TS_EVENT_HTTP_CONTINUE);
+ return TS_SUCCESS;
+}
+
+/** Find the fingerprint the --log-field callback logs for a transaction.
+ *
+ * @param[in] txnp The transaction being logged.
+ * @param[in] config The plugin configuration.
+ * @return The fingerprint, or nullptr if none was generated for the
transaction.
+ */
+std::string const *
+get_log_fingerprint(TSHttpTxn txnp, PluginConfig &config)
+{
+ TSHttpSsn ssnp = TSHttpTxnSsnGet(txnp);
+
+ if (config.method.type != Method::Type::CONNECTION_BASED) {
+ if (JAxContext *ctx = get_user_arg(txnp, config); ctx != nullptr) {
+ return &ctx->get_fingerprint();
+ }
+ if (ssnp == nullptr) {
+ return nullptr;
+ }
+ // The session copy may belong to another transaction: an earlier
keep-alive transaction when this one never
+ // got a fingerprint, or, on HTTP/2, a later stream that closed first.
+ return get_session_fingerprint(ssnp, config, TSHttpTxnIdGet(txnp));
+ }
+
+ if (ssnp == nullptr) {
+ return nullptr;
+ }
+ // The client vconn is gone if ATS closed the client connection before
logging the transaction, as it does
+ // for Connection: close requests and client timeouts.
+ if (TSVConn vconn = TSHttpSsnClientVConnGet(ssnp); vconn != nullptr) {
+ if (JAxContext *ctx = get_user_arg(vconn, config); ctx != nullptr) {
+ return &ctx->get_fingerprint();
+ }
+ }
+ return get_session_fingerprint(ssnp, config);
+}
+
int
handle_http_txn_close(void *edata, PluginConfig &config)
{
TSHttpTxn txnp = static_cast<TSHttpTxn>(edata);
+ // ATS logs the transaction after its TXN_CLOSE hooks, so keep a copy for
the --log-field callback.
+ if (config.session_user_arg_index != -1) {
+ JAxContext *ctx = get_user_arg(txnp, config);
Review Comment:
Confirmed and fixed in 2daf16c211. Every request-based instance now saves
the copy at TXN_CLOSE when any `--log-field` instance has reserved the session
slot. The slot is found by name the first time a hook needs it. SSN_CLOSE frees
the copies of all instances. The new `log-field-second-instance` variant has an
instance without `--log-field` first. It fails without the fix (`JA4H: -`).
##########
src/api/InkAPI.cc:
##########
@@ -9688,7 +9688,8 @@ TSLogStringUnmarshal(char **buf, char *dest, int len)
// of the alignment.
if (int l = strlen(*buf); l < len) {
memcpy(dest, *buf, l);
- return {l, l};
+ // TSLogStringMarshal also wrote the terminating NUL, so the field
occupies one more byte than was written to dest.
+ return {l + 1, l};
} else {
Review Comment:
Still leaving this open as out of scope: the `strlen` predates this PR,
which only fixes the consumed length. The buffer always holds the NUL that
`TSLogStringMarshal` writes. The new `test_log_string_unmarshal` covers the
lengths where the old count was wrong.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
