Support JSON error response bodies

[bneradt]

Problem

ATS cannot reliably emit JSON error responses through header rewrite: literal bodies cannot select a MIME type, HRW4U interprets JSON braces as interpolation, fetched bodies lose their Content-Type, and conditional local files are unavailable. Body factory error pages are also hardcoded to HTML.

Changes

• Add optional Content-Type arguments to set-body and set-body-from.
• Preserve the fetched response Content-Type unless a rule overrides it.
• Add set-body-from-file, loaded once at rule-configuration time.
• Add HRW4U literal JSON braces and the MIME-aware body forms.
• Add configurable body-factory Content-Type metadata.
• Document and test literal, fetched, file-backed, and body-factory bodies.

The asynchronous URL form remains READ_RESPONSE-only because remap callbacks cannot suspend for an internal fetch. REMAP errors are handled by the literal or file-backed forms without origin or cache lookup.

This includes the body-factory work from #12947 and preserves Bryan Call's original commits.

Related: #10893
Fixes: #13393
Fixes: #11480

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Pull request overview

Copilot reviewed 44 out of 44 changed files in this pull request and generated 1 comment.

[bryancall]

I read the whole diff across all 44 files. The feature is a good idea and the execution is mostly careful:

• The charset fix is carried through consistently, all 14 gold files updated plus tests/gold_tests/ip_allow/gold/log.gold response sizes bumped 453 to 468, which is exactly the 15 bytes of ; charset=utf-8. That is the kind of detail that usually gets missed.
• OperatorSetBodyFrom's struct sockaddr_in addr {} value-initializes what was previously an uninitialized stack struct, so sin_zero was being passed to TSFetchUrl uninitialized before this.
• The hrw4u section restrictions are tightened correctly for set-body-from and set-body-from-file, matching the C++ initialize_hooks().
• The new coverage is behavioral rather than decorative: both the default and custom Content-Type paths, the fetched Content-Type propagating, the explicit override winning, and the file-backed body working.
• I verified the documentation claims rather than taking them on trust. A missing .body_factory_info does skip the whole set, since HttpBodySet::init returns -1 on open() failure, and the documented \n, \r, \t and escaped-quote support already exists in the tokenizer.

Requesting changes on one item, plus a rebase.

set-body-from-file swallows a load failure

plugins/header_rewrite/operators.cc, OperatorSetBodyFromFile::initialize and ::exec

initialize() does _body = swoc::file::load(path, ec); if (ec) { TSError(...); return; } and leaves _loaded = false without throwing. parse_config() still returns true, TSRemapNewInstance returns success, and the remap config reloads cleanly. From then on every matching request gets the default body-factory HTML page with Content-Type: text/html instead of the intended JSON, and exec()'s if (!_loaded) { return true; } emits nothing, so there is no runtime signal at all. The single startup TSError is easy to lose in a reload log.

I want to correct the framing I would otherwise have used here. Operator::initialize() being void is not the obstacle: the exception-based failure channel already exists and is already used for this exact case. OperatorIf::add_operator and RuleSet::add_operator wrap op->initialize(p) in try/catch and return false on std::exception, RulesConfig::parse_config propagates that, and TSRemapNewInstance turns it into TS_ERROR. More to the point, OperatorRunPlugin::initialize in this same file already throws std::runtime_error when the referenced plugin cannot be loaded, which is the identical class of failure.

So the fix is a one-line throw, not a lazy-load redesign, and it brings set-body-from-file in line with the convention the file already follows.

Also worth addressing

plugins/header_rewrite/operators.cc, OperatorSetBody::initialize The new _content_type.set_value(p.get_value(), this) gives meaning to trailing tokens that Parser::preprocess already collected into _val but that set-body previously discarded. An existing rule set-body Sorry, page not found tokenizes to _arg = "Sorry," and _val = "page not found". Before this PR only _arg was consumed, so the body was the already-wrong string Sorry,. After it, the client gets Content-Type: page not found. Nothing validates that the second argument resembles a MIME type and there is no warning that the operator saw more tokens than it expected. Emitting a bogus Content-Type to clients is worse than the previous truncated body, and it happens on reload with no diagnostic. Rejecting or warning when the second argument contains whitespace or lacks a / would catch both the legacy config and simple typos.

Smaller items

• tools/hrw4u/src/tables.py:67 set-body is registered with HTTP_SECTIONS even though the C++ operator allows only TS_REMAP_PSEUDO_HOOK and TS_HTTP_SEND_RESPONSE_HDR_HOOK, so READ_RESPONSE { set-body(...) } passes hrw4u validation and is rejected later at ATS config load. The same commit restricts the other two operators correctly, so the omission looks accidental. Pre-existing inbound.resp.body on line 46 has the same over-permissive sections, so this does not make things worse.
• plugins/header_rewrite/operators.cc:838-839 The two new require_resources() calls in OperatorSetBody::initialize are dead. RSRC_SERVER_RESPONSE_HEADERS is only read under TS_HTTP_READ_RESPONSE_HDR_HOOK, and exec() reads neither res.bufp nor res.resp_status. Harmless, but it was evidently copied from OperatorSetBodyFrom where the requirements are load-bearing, and a reader will assume a dependency that is not there.
• doc/admin-guide/plugins/header_rewrite.en.rst:~1206-1215 A set-body REMAP example is inserted into the middle of the set-body-from section, which orphans the sentence "Where http://www.example.com/second is the destination..." two blocks away from the example it refers to, and duplicates a block shown 30 lines earlier. The new set-body-from-file section is also placed between set-body and set-body-from.
• src/proxy/http/HttpBodyFactory.cc:~1010-1014 The new Content-Type directive inherits the .body_factory_info value parser's off-by-one: value_e = buffer + strlen(buffer) - 1 assumes a trailing character to discard, so a file with no trailing newline yields application/jso. Pre-existing, since Content-Language and Content-Charset have always had it, but this PR adds a third directive that hits it and the new autest writes a trailing newline so the case is not covered. Worth a one-line fix while the file is open.
• Test gaps on the degraded paths, which are the ones that fail quietly in production: set-body-from-file with an unreadable path, set-body "" "application/json" which clears the body but keeps the message buffer type so build_error_response fabricates the default HTML page and then labels it application/json, and a body file larger than proxy.config.body_factory.response_max_size, which becomes a zero-length body with no error.
• Not introduced here and not a reason to hold the PR, but flagging so it is known rather than discovered from a sanitizer crash later: SetBodyFromData is raw new/delete whose lifetime is tied to TS_EVENT_HTTP_TXN_CLOSE on the same continuation that receives the fetch callbacks. A client abort while the internal TSFetchUrl is outstanding frees the state and destroys the continuation before the FetchSM delivers its result. The continuation was already destroyed at TXN_CLOSE before this PR; the change only adds owned heap state to the existing race.

The branch is conflicting with master and needs a rebase.