fix: require RSRC_CLIENT_REQUEST_HEADERS in ConditionMethod

[yknoya]

Problem

In the header_rewrite plugin, there are some cases where the METHOD condition doesn't work correctly.
The following rewriting rule is one such case.

cond %{READ_REQUEST_HDR_HOOK}
cond %{METHOD} =GET
set-config proxy.config.http.insert_response_via_str 1 [L]

cond %{SEND_REQUEST_HDR_HOOK}
cond %{METHOD} =DELETE
set-config proxy.config.http.insert_response_via_str 1 [L]

How to reproduce

Please refer to the test added in this PR.

Cause

When header rewrite parses each rule, it keeps tabs of the resources required for evaluation. Then during evaluation, it gathers the required resources before applying each rule.

The required resource for the METHOD condition is the client request, used to extract the request method. However, the condition doesn't include the client request in its list of required resources. As a result, the condition will always fail to match because it'll compare the expected method against an empty string.

It's worth mentioning that the condition will sometimes work, though. If some other condition or operator in the same rule requires the client request, the METHOD condition can then obtain the request method. Otherwise, it'll be broken.

Here are some pointers to the relevant bits of code:

This is where the METHOD condition extracts the request method from the client request. It needs res.client_bufp and res.client_hdr_loc to be set beforehand.

trafficserver/plugins/header_rewrite/conditions.cc

Lines 106 to 121 in fc35742

	void
	ConditionMethod::append_value(std::string &s, const Resources &res)
	{
	TSMBuffer bufp;
	TSMLoc hdr_loc;
	int len;
	bufp = res.client_bufp;
	hdr_loc = res.client_hdr_loc;
	if (bufp && hdr_loc) {
	const char *value = TSHttpHdrMethodGet(bufp, hdr_loc, &len);
	TSDebug(PLUGIN_NAME, "Appending METHOD(%s) to evaluation value -> %.*s", _qualifier.c_str(), len, value);
	s.append(value, len);
	}
	}

This is where resources are gathered during evaluation of each rule. If the RSRC_CLIENT_REQUEST_HEADERS flag is set, then it proceeds to set res.client_bufp and res.client_hdr_loc.

trafficserver/plugins/header_rewrite/header_rewrite.cc

Line 286 in fc35742

res.gather(conf->resid(hook), hook);

trafficserver/plugins/header_rewrite/resources.cc

Lines 34 to 40 in fc35742

	if (ids & RSRC_CLIENT_REQUEST_HEADERS) {
	TSDebug(PLUGIN_NAME, "\tAdding TXN client request header buffers");
	if (TSHttpTxnClientReqGet(txnp, &client_bufp, &client_hdr_loc) != TS_SUCCESS) {
	TSDebug(PLUGIN_NAME, "could not gather bufp/hdr_loc for request");
	return;
	}
	}

This is where the METHOD condition is initialized during the parsing phase. As we can see, it doesn't request access to the client request by setting the RSRC_CLIENT_REQUEST_HEADERS flag.

trafficserver/plugins/header_rewrite/conditions.cc

Lines 85 to 93 in fc35742

	void
	ConditionMethod::initialize(Parser &p)
	{
	Condition::initialize(p);
	MatcherType *match = new MatcherType(_cond_op);
	match->set(p.get_arg());
	_matcher = match;
	}

In contrast, the HEADER condition sets the RSRC_CLIENT_REQUEST_HEADERS flag, enabling access to the client request for any rule that uses the condition. So when used together with the METHOD condition, the METHOD condition would be able to obtain the request method.

trafficserver/plugins/header_rewrite/conditions.cc

Lines 201 to 214 in fc35742

	void
	ConditionHeader::initialize(Parser &p)
	{
	Condition::initialize(p);
	MatcherType *match = new MatcherType(_cond_op);
	match->set(p.get_arg());
	_matcher = match;
	require_resources(RSRC_CLIENT_REQUEST_HEADERS);
	require_resources(RSRC_CLIENT_RESPONSE_HEADERS);
	require_resources(RSRC_SERVER_REQUEST_HEADERS);
	require_resources(RSRC_SERVER_RESPONSE_HEADERS);
	}

[bryancall]

[approve ci autest]

[cmcfarlen]

Makes sense to me. Thank you for the fix!

[zwoop]

Cherry-picked to v9.2.x