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 46413e8303 header_rewrite: Improve URL Error messages (#13261)
46413e8303 is described below
commit 46413e8303cd8467c4f8267fb529e00e0472b58e
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
---
plugins/header_rewrite/conditions.cc | 58 ++++++++++++++++++++++++++++++++----
plugins/header_rewrite/conditions.h | 2 ++
plugins/header_rewrite/operators.cc | 2 ++
plugins/header_rewrite/resources.cc | 1 +
plugins/header_rewrite/resources.h | 1 +
plugins/header_rewrite/ruleset.cc | 2 ++
plugins/header_rewrite/statement.h | 29 +++++++++++++++++-
plugins/header_rewrite/value.cc | 6 ++++
8 files changed, 95 insertions(+), 6 deletions(-)
diff --git a/plugins/header_rewrite/conditions.cc
b/plugins/header_rewrite/conditions.cc
index 3d32c3515d..1953864d79 100644
--- a/plugins/header_rewrite/conditions.cc
+++ b/plugins/header_rewrite/conditions.cc
@@ -39,6 +39,42 @@
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::SERVER:
+ return "SERVER-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
@@ -318,6 +354,18 @@ ConditionUrl::set_qualifier(const std::string &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)
{
@@ -328,7 +376,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 (_type == SERVER) {
@@ -336,7 +384,7 @@ ConditionUrl::append_value(std::string &s, const Resources
&res)
bufp = res.server_bufp;
if (bufp && res.server_hdr_loc) {
if (TSHttpHdrUrlGet(bufp, res.server_hdr_loc, &url) != TS_SUCCESS) {
- TSError("[%s] Error getting the server request URL", PLUGIN_NAME);
+ log_error(res, "Error getting the server request URL");
return;
}
} else {
@@ -356,7 +404,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 {
@@ -364,11 +412,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 631854e8f4..65020646b8 100644
--- a/plugins/header_rewrite/conditions.h
+++ b/plugins/header_rewrite/conditions.h
@@ -300,6 +300,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;
std::string _query_param; // Optional: specific query parameter name for
QUERY sub-key
diff --git a/plugins/header_rewrite/operators.cc
b/plugins/header_rewrite/operators.cc
index 2913c9349f..e82b834468 100644
--- a/plugins/header_rewrite/operators.cc
+++ b/plugins/header_rewrite/operators.cc
@@ -1720,6 +1720,7 @@ OperatorIf::add_operator(Parser &p, const char *filename,
int lineno)
}
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);
try {
op->initialize(p);
@@ -1752,6 +1753,7 @@ OperatorIf::make_condition(Parser &p, const char
*filename, int lineno)
}
Dbg(pi_dbg_ctl, " Creating condition: %%{%s} with arg: %s",
p.get_op().c_str(), p.get_arg().c_str());
+ cond->set_config_location(filename, lineno);
try {
cond->initialize(p);
diff --git a/plugins/header_rewrite/resources.cc
b/plugins/header_rewrite/resources.cc
index 78450147b9..ed8d4ffe6a 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 80d1268106..d745dffdee 100644
--- a/plugins/header_rewrite/resources.h
+++ b/plugins/header_rewrite/resources.h
@@ -128,6 +128,7 @@ public:
#endif
TSHttpStatus resp_status = TS_HTTP_STATUS_NONE;
void *geo_handle = nullptr;
+ 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 28fda37a51..39964fc68e 100644
--- a/plugins/header_rewrite/ruleset.cc
+++ b/plugins/header_rewrite/ruleset.cc
@@ -65,6 +65,7 @@ RuleSet::make_condition(Parser &p, const char *filename, int
lineno)
}
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;
@@ -92,6 +93,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 1f16c24246..4ed73ec681 100644
--- a/plugins/header_rewrite/statement.h
+++ b/plugins/header_rewrite/statement.h
@@ -172,6 +172,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 &)
{
@@ -277,7 +302,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 70fd17893a..38b00dd825 100644
--- a/plugins/header_rewrite/value.cc
+++ b/plugins/header_rewrite/value.cc
@@ -39,6 +39,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);
@@ -54,6 +57,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 {
