header_rewrite: Improve URL Error messages

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,15 +376,15 @@ 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) {
     Dbg(pi_dbg_ctl, "   Using the server request url");
     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,19 +404,19 @@ 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 {
     if (_type == URL) {
       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;
     }
   }

plugins/header_rewrite/conditions.h

@@ -300,6 +300,8 @@ class ConditionUrl : public Condition
   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

plugins/header_rewrite/operators.cc

@@ -1713,6 +1713,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);
@@ -1745,6 +1746,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);

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);
 

plugins/header_rewrite/resources.h

@@ -128,6 +128,7 @@ class Resources
 #endif
   TSHttpStatus resp_status = TS_HTTP_STATUS_NONE;
   void        *geo_handle  = nullptr;
+  TSHttpHookID hook        = TS_HTTP_LAST_HOOK;
 
   struct LifetimeExtension {
     std::string                                        subject_storage;

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;

plugins/header_rewrite/statement.h

@@ -164,6 +164,31 @@ class Statement
 
   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 &)
   {
@@ -269,7 +294,9 @@ class Statement
   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 {

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 {