header_rewrite: reject bad run-plugin at config load

[zwoop]

A run-plugin whose target plugin failed to load left a null instance that tripped a release assert and aborted the server on the first request. Propagate the load failure as an exception so the rule is rejected at config load time (a reload simply keeps the running config), and guard exec() so a stray bad rule can never abort the process.

[Copilot]

Pull request overview

This PR hardens the header_rewrite plugin’s run-plugin operator so that failures to load the target remap plugin are rejected during configuration load/reload rather than causing a TSReleaseAssert abort on first request. It also adds an AuTest gold test to prevent regressions and validate both cold-start and reload behavior.

Changes:

• Convert OperatorRunPlugin::initialize() load failures into exceptions, so bad run-plugin rules are rejected during config parsing.
• Catch operator initialization exceptions in RuleSet::add_operator() and fail the ruleset cleanly with an error log.
• Add a gold test covering both startup rejection and reload rejection while keeping the running config active.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bad_run_plugin.test.py	Adds coverage ensuring a bad run-plugin is rejected at config load/reload and never crashes request processing.
plugins/header_rewrite/ruleset.cc	Catches Operator::initialize() exceptions so operator init failures reject the config cleanly.
plugins/header_rewrite/operators.cc	Makes run-plugin initialization throw on load failures and guards exec() against a null plugin instance.

[zwoop]

[approve ci]

[cmcfarlen]

Did Claude do this? We typically don't throw exceptions.

[bneradt]

I found two exception-safety/resource-lifetime issues that should be addressed:

Make the temporary argv exception-safe

OperatorRunPlugin::initialize() allocates argv with new[], calls getRemapPlugin(), and only then reaches delete[]. The factory and its filesystem/allocation helpers can throw standard exceptions; those are now caught by RuleSet::add_operator() or OperatorIf::add_operator(), but unwinding skips the manual delete. A rejected reload can therefore leak this allocation. Please use RAII here, such as std::vector<char *> or std::unique_ptr<char *[]>.

trafficserver/plugins/header_rewrite/operators.cc

Lines 1297 to 1324 in 079936f

	// Create argc and argv
	int argc = tokens.size() + 2;
	char **argv = new char *[argc];
	argv[0] = p.from_url();
	argv[1] = p.to_url();
	for (size_t i = 0; i < tokens.size(); ++i) {
	argv[i + 2] = const_cast<char *>(tokens[i].c_str());
	}
	std::string error;
	// We have to escalate access while loading these plugins, just as done when loading remap.config
	{
	uint32_t elevate_access = 0;
	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,
	isPluginDynamicReloadEnabled());
	} // done elevating access
	delete[] argv;
	if (!_plugin) {
	throw std::runtime_error("run-plugin unable to load plugin '" + std::string{plugin_name} + "': " + error);

Clean up active nested-if frames on failure

When a bad run-plugin appears inside an open if, OperatorIf::add_operator() catches the initialization exception and returns false. RulesConfig::parse_config() then takes its runtime-error return path, but if_stack still contains raw OperatorIf * values created for active frames. Those frames have not yet been transferred into the RuleSet, so returning without draining the stack leaks them and any successfully initialized operators or plugin instances they already own. Repeated rejected reloads can accumulate these leaks. Please make the stack RAII-owned or clean it on all error exits; adding a nested run-plugin case to this test would cover the second initialization path.

trafficserver/plugins/header_rewrite/header_rewrite.cc

Lines 369 to 417 in 079936f

	auto *op_if = new OperatorIf();
	if_stack.push(op_if);
	group = op_if->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();
	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);
	} else {
	parent_sec->ops.oper.reset(op_if);
	}
	group = if_stack.top()->get_group();
	} else {
	if (!rule->add_operator(op_if)) {
	delete op_if;
	throw std::runtime_error("Failed to add nested OperatorIf to RuleSet");
	}
	group = rule->get_group();
	}
	Dbg(dbg_ctl, "Completed nested OperatorIf, depth now: %zu", if_stack.size());
	} else {
	if (!if_stack.empty()) {
	if (!if_stack.top()->add_operator(p, filename.c_str(), lineno)) {
	throw std::runtime_error("add_operator() failed in nested OperatorIf");
	}
	} else {
	if (!rule->add_operator(p, filename.c_str(), lineno)) {
	throw std::runtime_error("add_operator() failed");
	}
	}
	}
	}
	} catch (std::runtime_error &e) {
	TSError("[%s] header_rewrite configuration exception: %s in file: %s, lineno: %d", PLUGIN_NAME, e.what(), fname.c_str(),
	lineno);
	return false;
	}