apache · bneradt · Aug 5, 2026 · Jul 24, 2026
diff --git a/plugins/header_rewrite/header_rewrite.cc b/plugins/header_rewrite/header_rewrite.cc
@@ -202,12 +202,12 @@ validate_rule_completion(RuleSet *rule, const std::string &fname, int lineno)
 bool
 RulesConfig::parse_config(const std::string &fname, TSHttpHookID default_hook, char *from_url, char *to_url)
 {
-  std::unique_ptr<RuleSet>     rule(nullptr);
-  std::string                  filename;
-  int                          lineno = 0;
-  ConditionGroup              *group  = nullptr;
-  std::stack<ConditionGroup *> group_stack;
-  std::stack<OperatorIf *>     if_stack;
+  std::unique_ptr<RuleSet>                rule(nullptr);
+  std::string                             filename;
+  int                                     lineno = 0;
+  ConditionGroup                         *group  = nullptr;
+  std::stack<ConditionGroup *>            group_stack;
+  std::stack<std::unique_ptr<OperatorIf>> if_stack;
 
   constexpr int MAX_IF_NESTING_DEPTH = 10;
 
@@ -366,32 +366,29 @@ RulesConfig::parse_config(const std::string &fname, TSHttpHookID default_hook, c
             throw std::runtime_error("maximum if nesting depth exceeded");
           }
 
-          auto *op_if = new OperatorIf();
-
-          if_stack.push(op_if);
-          group = op_if->get_group(); // Set group to the new OperatorIf's group
+          if_stack.push(std::make_unique<OperatorIf>());
+          group = if_stack.top()->get_group(); // Set group to the new OperatorIf's group
           Dbg(dbg_ctl, "Started nested OperatorIf, depth: %zu", if_stack.size());
 
         } else if (p.is_endif()) {
           if (if_stack.empty()) {
             throw std::runtime_error("endif without matching if");
           }
 
-          OperatorIf *op_if = if_stack.top();
+          auto op_if = std::move(if_stack.top());
 
           if_stack.pop();
           if (!if_stack.empty()) {
             auto *parent_sec = if_stack.top()->cur_section();
 
             if (parent_sec->ops.oper) {
-              parent_sec->ops.oper->append(op_if);
+              parent_sec->ops.oper->append(op_if.release());
             } else {
-              parent_sec->ops.oper.reset(op_if);
+              parent_sec->ops.oper = std::move(op_if);
             }
             group = if_stack.top()->get_group();
           } else {
-            if (!rule->add_operator(op_if)) {
-              delete op_if;
+            if (!rule->add_operator(std::move(op_if))) {
               throw std::runtime_error("Failed to add nested OperatorIf to RuleSet");
             }
             group = rule->get_group();
@@ -434,10 +431,6 @@ RulesConfig::parse_config(const std::string &fname, TSHttpHookID default_hook, c
   // Check for unmatched if statements
   if (!if_stack.empty()) {
     TSError("[%s] %zu unmatched 'if' statement(s) without 'endif' in file: %s", PLUGIN_NAME, if_stack.size(), fname.c_str());
-    while (!if_stack.empty()) {
-      delete if_stack.top();
-      if_stack.pop();
-    }
     return false;
   }
 

diff --git a/plugins/header_rewrite/operators.cc b/plugins/header_rewrite/operators.cc
@@ -23,6 +23,7 @@
 #include <cstring>
 #include <algorithm>
 #include <iomanip>
+#include <stdexcept>
 
 #include "records/RecCore.h"
 #include "ts/ts.h"
@@ -1272,8 +1273,7 @@ OperatorRunPlugin::initialize(Parser &p)
   auto plugin_args = p.get_value();
 
   if (plugin_name.empty()) {
-    TSError("[%s] missing plugin name", PLUGIN_NAME);
-    return;
+    throw std::runtime_error("run-plugin missing plugin name");
   }
 
   std::vector<std::string> tokens;
@@ -1284,15 +1284,10 @@ OperatorRunPlugin::initialize(Parser &p)
     tokens.push_back(token);
   }
 
-  // Create argc and argv
-  int    argc = tokens.size() + 2;
-  char **argv = new char *[argc];
-
-  argv[0] = p.from_url();
-  argv[1] = p.to_url();
+  std::vector<char *> argv{p.from_url(), p.to_url()};
 
-  for (size_t i = 0; i < tokens.size(); ++i) {
-    argv[i + 2] = const_cast<char *>(tokens[i].c_str());
+  for (auto const &argument : tokens) {
+    argv.push_back(const_cast<char *>(argument.c_str()));
   }
 
   std::string error;
@@ -1304,14 +1299,12 @@ OperatorRunPlugin::initialize(Parser &p)
     elevate_access = RecGetRecordInt("proxy.config.plugin.load_elevated").value_or(0);
     ElevateAccess access(elevate_access ? ElevateAccess::FILE_PRIVILEGE : 0);
 
-    _plugin = plugin_factory.getRemapPlugin(swoc::file::path(plugin_name), argc, const_cast<char **>(argv), error,
+    _plugin = plugin_factory.getRemapPlugin(swoc::file::path(plugin_name), static_cast<int>(argv.size()), argv.data(), error,
                                             isPluginDynamicReloadEnabled());
   } // done elevating access
 
-  delete[] argv;
-
   if (!_plugin) {
-    TSError("[%s] Unable to load plugin '%s': %s", PLUGIN_NAME, plugin_name.c_str(), error.c_str());
+    throw std::runtime_error("run-plugin unable to load plugin '" + std::string{plugin_name} + "': " + error);
   }
 }
 
@@ -1326,7 +1319,11 @@ OperatorRunPlugin::initialize_hooks()
 bool
 OperatorRunPlugin::exec(const Resources &res) const
 {
-  TSReleaseAssert(_plugin != nullptr);
+  // Rejected at config load (see initialize); guard anyway so a stray bad rule can't abort the server.
+  if (!_plugin) {
+    Dbg(pi_dbg_ctl, "OperatorRunPlugin::exec skipped, plugin was not loaded");
+    return true;
+  }
 
   if (res._rri && res.state.txnp) {
     _plugin->doRemap(res.state.txnp, res._rri);
@@ -1654,7 +1651,7 @@ OperatorIf::new_section(Parser::CondClause clause)
 bool
 OperatorIf::add_operator(Parser &p, const char *filename, int lineno)
 {
-  Operator *op = operator_factory(p.get_op());
+  std::unique_ptr<Operator> op{operator_factory(p.get_op())};
 
   if (!op) {
     TSError("[%s] Unknown operator: %s, file: %s, line: %d", PLUGIN_NAME, p.get_op().c_str(), filename, lineno);
@@ -1667,18 +1664,17 @@ OperatorIf::add_operator(Parser &p, const char *filename, int lineno)
   try {
     op->initialize(p);
   } catch (std::exception const &ex) {
-    delete op;
     TSError("[%s] Failed to initialize operator: %s, file: %s, line: %d, error: %s", PLUGIN_NAME, p.get_op().c_str(), filename,
             lineno, ex.what());
     return false;
   }
 
   // Add to current section
   if (_cur_section->ops.oper) {
-    _cur_section->ops.oper->append(op);
+    _cur_section->ops.oper->append(op.release());
   } else {
-    _cur_section->ops.oper.reset(op);
-    _cur_section->ops.oper_mods = op->get_oper_modifiers();
+    _cur_section->ops.oper      = std::move(op);
+    _cur_section->ops.oper_mods = _cur_section->ops.oper->get_oper_modifiers();
   }
 
   return true;

diff --git a/plugins/header_rewrite/ruleset.cc b/plugins/header_rewrite/ruleset.cc
@@ -19,6 +19,7 @@
 // ruleset.cc: implementation of the ruleset class
 //
 //
+#include <exception>
 #include <string>
 
 #include "ruleset.h"
@@ -89,14 +90,20 @@ RuleSet::make_condition(Parser &p, const char *filename, int lineno)
 bool
 RuleSet::add_operator(Parser &p, const char *filename, int lineno)
 {
-  Operator *op = operator_factory(p.get_op());
+  std::unique_ptr<Operator> op{operator_factory(p.get_op())};
 
-  if (nullptr != op) {
+  if (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);
+
+    try {
+      op->initialize(p);
+    } catch (std::exception const &ex) {
+      TSError("[%s] in %s:%d: failed to initialize operator %s: %s", PLUGIN_NAME, filename, lineno, p.get_op().c_str(), ex.what());
+      return false;
+    }
+
     if (!op->is_hook_valid(_hook)) {
-      delete op;
       Dbg(pi_dbg_ctl, "in %s:%d: can't use this operator in hook=%s:  %s(%s)", filename, lineno, TSHttpHookNameLookup(_hook),
           p.get_op().c_str(), p.get_arg().c_str());
       TSError("[%s] in %s:%d: can't use this operator in hook=%s:  %s(%s)", PLUGIN_NAME, filename, lineno,
@@ -107,9 +114,9 @@ RuleSet::add_operator(Parser &p, const char *filename, int lineno)
     auto *cur_sec = _op_if.cur_section();
 
     if (!cur_sec->ops.oper) {
-      cur_sec->ops.oper.reset(op);
+      cur_sec->ops.oper = std::move(op);
     } else {
-      cur_sec->ops.oper->append(op);
+      cur_sec->ops.oper->append(op.release());
     }
 
     cur_sec->ops.oper_mods = static_cast<OperModifiers>(cur_sec->ops.oper_mods | cur_sec->ops.oper->get_oper_modifiers());
@@ -136,14 +143,14 @@ RuleSet::get_all_resource_ids() const
 }
 
 bool
-RuleSet::add_operator(Operator *op)
+RuleSet::add_operator(std::unique_ptr<Operator> op)
 {
   auto *cur_sec = _op_if.cur_section();
 
   if (!cur_sec->ops.oper) {
-    cur_sec->ops.oper.reset(op);
+    cur_sec->ops.oper = std::move(op);
   } else {
-    cur_sec->ops.oper->append(op);
+    cur_sec->ops.oper->append(op.release());
   }
 
   // Update some ruleset state based on this new operator

diff --git a/plugins/header_rewrite/ruleset.h b/plugins/header_rewrite/ruleset.h
@@ -49,7 +49,7 @@ class RuleSet
   Condition  *make_condition(Parser &p, const char *filename, int lineno);
   ResourceIDs get_all_resource_ids() const;
   bool        add_operator(Parser &p, const char *filename, int lineno);
-  bool        add_operator(Operator *op);
+  bool        add_operator(std::unique_ptr<Operator> op);
 
   ConditionGroup *
   get_group()

diff --git a/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bad_run_plugin.test.py b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bad_run_plugin.test.py
@@ -0,0 +1,156 @@
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+'''
+Verify header_rewrite rejects a run-plugin operator whose target plugin fails to
+load. The failure must be caught at config load time, not aborted at request time.
+'''
+
+Test.Summary = '''
+header_rewrite must reject a run-plugin whose target plugin fails to load, at
+config load time, rather than aborting the server on the first request.
+'''
+
+# Reproduce the reported crash: run-plugin against a plugin whose instance-init fails
+# (conf_remap + a missing file) hands header_rewrite a null instance, which old code aborted on.
+Test.SkipUnless(
+    Condition.PluginExists('header_rewrite.so'),
+    Condition.PluginExists('conf_remap.so'),
+)
+
+
+class TestBadRunPlugin:
+    '''Verify failed run-plugin initialization is rejected safely.'''
+
+    ERROR_MARKER: str = 'run-plugin unable to load'
+    BAD_RULE_LINES: list[str] = [
+        'cond %{REMAP_PSEUDO_HOOK}',
+        '  run-plugin conf_remap.so no_such_conf_remap_file.yaml',
+    ]
+    NESTED_BAD_RULE_LINES: list[str] = [
+        'cond %{REMAP_PSEUDO_HOOK}',
+        '  if',
+        '    cond %{TRUE}',
+        '      run-plugin conf_remap.so no_such_conf_remap_file.yaml',
+        '  endif',
+    ]
+
+    def __init__(self) -> None:
+        '''Configure startup and reload rejection scenarios.'''
+        self._configure_startup_rejection()
+        self._server = self._configure_origin_server()
+        self._ts = self._configure_traffic_server()
+        self._configure_baseline_request()
+        self._configure_bad_remap_install()
+        self._configure_failed_reload()
+        self._configure_post_reload_request()
+        self._ts.Disk.diags_log.Content = Testers.IncludesExpression(
+            self.ERROR_MARKER, 'the rejected reload should log the run-plugin failure')
+
+    def _configure_startup_rejection(self) -> None:
+        '''Verify a bad top-level run-plugin fails startup cleanly.'''
+        ts = Test.MakeATSProcess("ts-startup", disable_log_checks=True)
+        ts.Disk.records_config.update({
+            'proxy.config.diags.debug.enabled': 1,
+            'proxy.config.diags.debug.tags': 'header_rewrite',
+        })
+        ts.Disk.MakeConfigFile('bad_run_plugin.conf').AddLines(self.BAD_RULE_LINES)
+        ts.Disk.remap_config.AddLine(
+            'map http://startup.example.com/ http://127.0.0.1/ '
+            '@plugin=header_rewrite.so @pparam=bad_run_plugin.conf')
+
+        # Invalid remap.config triggers a controlled exit rather than SIGABRT.
+        ts.ReturnCode = 33
+        ts.Ready = 0
+        ts.Disk.diags_log.Content = Testers.IncludesExpression(
+            self.ERROR_MARKER, 'header_rewrite must report the failed run-plugin load')
+        ts.Disk.traffic_out.Content = Testers.ExcludesExpression(
+            'Traffic Server is fully initialized', 'ATS must not initialize with a bad run-plugin config')
+
+        tr = Test.AddTestRun("Bad run-plugin config fails startup instead of crashing")
+        tr.Processes.Default.Command = 'echo verifying startup rejection'
+        tr.Processes.Default.ReturnCode = 0
+        tr.Processes.Default.StartBefore(ts)
+
+    def _configure_origin_server(self) -> 'Process':
+        '''Configure the origin used to verify reload behavior.'''
+        server = Test.MakeOriginServer("server")
+        request_header = {
+            "headers": "GET / HTTP/1.1\r\nHost: reload.example.com\r\n\r\n",
+            "timestamp": "1469733493.993",
+            "body": ""
+        }
+        response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+        server.addResponse("sessionfile.log", request_header, response_header)
+        return server
+
+    def _configure_traffic_server(self) -> 'Process':
+        '''Configure ATS with a valid initial remap table.'''
+        ts = Test.MakeATSProcess("ts-reload", disable_log_checks=True)
+        ts.Disk.records_config.update({
+            'proxy.config.diags.debug.enabled': 1,
+            'proxy.config.diags.debug.tags': 'header_rewrite',
+        })
+        ts.Disk.MakeConfigFile('nested_bad_run_plugin.conf').AddLines(self.NESTED_BAD_RULE_LINES)
+        ts.Disk.remap_config.AddLine(f'map http://reload.example.com http://127.0.0.1:{self._server.Variables.Port}')
+        return ts
+
+    def _configure_curl_run(self, name: str, expectation: str) -> 'TestRun':
+        '''Configure a request that verifies ATS still serves traffic.'''
+        tr = Test.AddTestRun(name)
+        tr.MakeCurlCommand(
+            f'--proxy 127.0.0.1:{self._ts.Variables.port} "http://reload.example.com" '
+            '-H "Proxy-Connection: keep-alive" --verbose',
+            ts=self._ts)
+        tr.Processes.Default.ReturnCode = 0
+        tr.Processes.Default.Streams.stderr = Testers.IncludesExpression('200 OK', expectation)
+        tr.StillRunningAfter = self._ts
+        tr.StillRunningAfter = self._server
+        return tr
+
+    def _configure_baseline_request(self) -> None:
+        '''Verify the valid initial configuration serves requests.'''
+        tr = self._configure_curl_run("Baseline request is served before reload", 'baseline request should be served')
+        tr.Processes.Default.StartBefore(self._server)
+        tr.Processes.Default.StartBefore(self._ts)
+
+    def _configure_bad_remap_install(self) -> None:
+        '''Replace remap.config with one containing a bad nested run-plugin.'''
+        tr = Test.AddTestRun("Install a remap.config with a bad run-plugin")
+        remap_path = self._ts.Disk.remap_config.AbsPath
+        tr.Disk.File(remap_path, id="remap_bad", typename="ats:config")
+        tr.Disk.remap_bad.AddLine(
+            f'map http://reload.example.com http://127.0.0.1:{self._server.Variables.Port} '
+            '@plugin=header_rewrite.so @pparam=nested_bad_run_plugin.conf')
+        tr.Processes.Default.Command = 'echo installed bad remap.config'
+        tr.Processes.Default.ReturnCode = 0
+        tr.Processes.Default.Env = self._ts.Env
+        tr.StillRunningAfter = self._ts
+        tr.StillRunningAfter = self._server
+
+    def _configure_failed_reload(self) -> None:
+        '''Verify the bad remap table is rejected without stopping ATS.'''
+        tr = Test.AddConfigReload(
+            self._ts, expect="fail", delay_start=2, description="Reload with bad run-plugin must be rejected, not fatal")
+        tr.StillRunningAfter = self._ts
+        tr.StillRunningAfter = self._server
+
+    def _configure_post_reload_request(self) -> None:
+        '''Verify the rejected reload leaves the old configuration active.'''
+        self._configure_curl_run(
+            "Server still serves the old config after the rejected reload", 'old config should still serve after a rejected reload')
+
+
+TestBadRunPlugin()