This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 4b4d9de06738e90f0c4f64e8bd1a8aca71050049 Author: Brian Neradt <[email protected]> AuthorDate: Tue Jun 23 12:56:43 2026 -0500 header_rewrite: Improve URL Error messages (#13261) Old log: ERROR: [header_rewrite] Rule not supported at this hook New log: ERROR: [header_rewrite] Rule not supported at hook=TS_HTTP_READ_RESPONSE_HDR_HOOK: %{TO-URL:URL} in /tmp/sb/header_rewrite_unsupported_url_hook/unsupported_url_hook.conf:18 (cherry picked from commit 46413e8303cd8467c4f8267fb529e00e0472b58e) --- plugins/header_rewrite/conditions.cc | 54 +++++++++++++++++++++++++++++++++--- plugins/header_rewrite/conditions.h | 2 ++ plugins/header_rewrite/resources.cc | 1 + plugins/header_rewrite/resources.h | 1 + plugins/header_rewrite/ruleset.cc | 4 ++- plugins/header_rewrite/statement.h | 29 ++++++++++++++++++- plugins/header_rewrite/value.cc | 6 ++++ 7 files changed, 91 insertions(+), 6 deletions(-) diff --git a/plugins/header_rewrite/conditions.cc b/plugins/header_rewrite/conditions.cc index fb44abdba6..fbd8590b44 100644 --- a/plugins/header_rewrite/conditions.cc +++ b/plugins/header_rewrite/conditions.cc @@ -36,6 +36,40 @@ static const sockaddr *getClientAddr(TSHttpTxn txnp, int txn_private_slot); +static const char * +get_hook_name(TSHttpHookID hook) +{ + if (hook == TS_REMAP_PSEUDO_HOOK) { + return "REMAP_PSEUDO_HOOK"; + } + + if (hook == TS_HTTP_LAST_HOOK) { + return "UNKNOWN_HOOK"; + } + + if (const char *name = TSHttpHookNameLookup(hook); name != nullptr) { + return name; + } + + return "UNKNOWN_HOOK"; +} + +static const char * +get_url_type_name(ConditionUrl::UrlType type) +{ + switch (type) { + case ConditionUrl::CLIENT: + return "CLIENT-URL"; + case ConditionUrl::FROM: + return "FROM-URL"; + case ConditionUrl::TO: + return "TO-URL"; + case ConditionUrl::URL: + default: + return "URL"; + } +} + #if TS_HAS_CRIPTS #include "cripts/Certs.hpp" #endif @@ -275,6 +309,18 @@ ConditionUrl::set_qualifier(const std::string &q) _url_qual = parse_url_qualifier(q); } +void +ConditionUrl::log_error(const Resources &res, const char *message) const +{ + if (has_config_location()) { + TSError("[%s] %s at hook=%s: %%{%s%s%s} in %s:%d", PLUGIN_NAME, message, get_hook_name(res.hook), get_url_type_name(_type), + _qualifier.empty() ? "" : ":", _qualifier.c_str(), get_config_filename().c_str(), get_config_lineno()); + } else { + TSError("[%s] %s at hook=%s: %%{%s%s%s}", PLUGIN_NAME, message, get_hook_name(res.hook), get_url_type_name(_type), + _qualifier.empty() ? "" : ":", _qualifier.c_str()); + } +} + void ConditionUrl::append_value(std::string &s, const Resources &res) { @@ -285,7 +331,7 @@ ConditionUrl::append_value(std::string &s, const Resources &res) // CLIENT always uses the pristine URL Dbg(pi_dbg_ctl, " Using the pristine url"); if (TSHttpTxnPristineUrlGet(res.state.txnp, &bufp, &url) != TS_SUCCESS) { - TSError("[%s] Error getting the pristine URL", PLUGIN_NAME); + log_error(res, "Error getting the pristine URL"); return; } } else if (res._rri != nullptr) { @@ -301,7 +347,7 @@ ConditionUrl::append_value(std::string &s, const Resources &res) Dbg(pi_dbg_ctl, " Using the to url"); url = res._rri->mapToUrl; } else { - TSError("[%s] Invalid option value", PLUGIN_NAME); + log_error(res, "Invalid URL option value"); return; } } else { @@ -309,11 +355,11 @@ ConditionUrl::append_value(std::string &s, const Resources &res) bufp = res.bufp; TSMLoc hdr_loc = res.hdr_loc; if (TSHttpHdrUrlGet(bufp, hdr_loc, &url) != TS_SUCCESS) { - TSError("[%s] Error getting the URL", PLUGIN_NAME); + log_error(res, "Error getting the URL"); return; } } else { - TSError("[%s] Rule not supported at this hook", PLUGIN_NAME); + log_error(res, "Rule not supported"); return; } } diff --git a/plugins/header_rewrite/conditions.h b/plugins/header_rewrite/conditions.h index 10d05fbaac..bc3d0b85ec 100644 --- a/plugins/header_rewrite/conditions.h +++ b/plugins/header_rewrite/conditions.h @@ -286,6 +286,8 @@ protected: bool eval(const Resources &res) override; private: + void log_error(const Resources &res, const char *message) const; + UrlQualifiers _url_qual = URL_QUAL_NONE; UrlType _type; }; diff --git a/plugins/header_rewrite/resources.cc b/plugins/header_rewrite/resources.cc index 025fe390e4..b270fa809f 100644 --- a/plugins/header_rewrite/resources.cc +++ b/plugins/header_rewrite/resources.cc @@ -32,6 +32,7 @@ void Resources::gather(const ResourceIDs ids, TSHttpHookID hook) { + this->hook = hook; Dbg(pi_dbg_ctl, "Building resources, hook=%s", TSHttpHookNameLookup(hook)); Dbg(pi_dbg_ctl, "Gathering resources for hook %s with IDs %d", TSHttpHookNameLookup(hook), ids); diff --git a/plugins/header_rewrite/resources.h b/plugins/header_rewrite/resources.h index ab59540f97..f8f7ddd201 100644 --- a/plugins/header_rewrite/resources.h +++ b/plugins/header_rewrite/resources.h @@ -112,6 +112,7 @@ public: TransactionState state; // Without cripts, txnp / ssnp goes here #endif TSHttpStatus resp_status = TS_HTTP_STATUS_NONE; + TSHttpHookID hook = TS_HTTP_LAST_HOOK; struct LifetimeExtension { std::string subject_storage; diff --git a/plugins/header_rewrite/ruleset.cc b/plugins/header_rewrite/ruleset.cc index a5d5709932..501315e063 100644 --- a/plugins/header_rewrite/ruleset.cc +++ b/plugins/header_rewrite/ruleset.cc @@ -50,7 +50,8 @@ RuleSet::make_condition(Parser &p, const char *filename, int lineno) return nullptr; // Complete failure in the factory } - Dbg(pi_dbg_ctl, " Adding condition: %%{%s} with arg: %s", p.get_op().c_str(), p.get_arg().c_str()); + Dbg(pi_dbg_ctl, " Creating condition: %%{%s} with arg: %s", p.get_op().c_str(), p.get_arg().c_str()); + c->set_config_location(filename, lineno); c->initialize(p); if (!c->set_hook(_hook)) { delete c; @@ -79,6 +80,7 @@ RuleSet::add_operator(Parser &p, const char *filename, int lineno) if (nullptr != op) { Dbg(pi_dbg_ctl, " Adding operator: %s(%s)=\"%s\"", p.get_op().c_str(), p.get_arg().c_str(), p.get_value().c_str()); + op->set_config_location(filename, lineno); op->initialize(p); if (!op->set_hook(_hook)) { delete op; diff --git a/plugins/header_rewrite/statement.h b/plugins/header_rewrite/statement.h index d711b9915c..510957837a 100644 --- a/plugins/header_rewrite/statement.h +++ b/plugins/header_rewrite/statement.h @@ -163,6 +163,31 @@ public: ResourceIDs get_resource_ids() const; + void + set_config_location(const char *filename, int lineno) + { + _config_filename = filename ? filename : ""; + _config_lineno = lineno; + } + + bool + has_config_location() const + { + return !_config_filename.empty(); + } + + const std::string & + get_config_filename() const + { + return _config_filename; + } + + int + get_config_lineno() const + { + return _config_lineno; + } + virtual void initialize(Parser &) { @@ -221,7 +246,9 @@ private: ResourceIDs _rsrc = RSRC_NONE; TSHttpHookID _hook = TS_HTTP_READ_RESPONSE_HDR_HOOK; std::vector<TSHttpHookID> _allowed_hooks; - bool _initialized = false; + std::string _config_filename; + int _config_lineno = 0; + bool _initialized = false; }; union PrivateSlotData { diff --git a/plugins/header_rewrite/value.cc b/plugins/header_rewrite/value.cc index 28966cc292..0c49ca5252 100644 --- a/plugins/header_rewrite/value.cc +++ b/plugins/header_rewrite/value.cc @@ -42,6 +42,9 @@ void Value::set_value(const std::string &val, Statement *owner) { _value = val; + if (owner && owner->has_config_location()) { + set_config_location(owner->get_config_filename().c_str(), owner->get_config_lineno()); + } if (_value.find("%{") != std::string::npos) { HRWSimpleTokenizer tokenizer(_value); @@ -57,6 +60,9 @@ Value::set_value(const std::string &val, Statement *owner) Parser parser; if (parser.parse_line(cond_token)) { + if (has_config_location()) { + tcond_val->set_config_location(get_config_filename().c_str(), get_config_lineno()); + } tcond_val->initialize(parser); require_resources(tcond_val->get_resource_ids()); } else {
