This is an automated email from the ASF dual-hosted git repository.
cmcfarlen pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new e55545540b Fix set-status crash inside if/endif at remap time (#13366)
e55545540b is described below
commit e55545540bb741efcfe1027da4df3e17c02683ab
Author: Mo Chen <[email protected]>
AuthorDate: Mon Jul 13 09:44:21 2026 -0500
Fix set-status crash inside if/endif at remap time (#13366)
Operators inside an if/endif block never had set_hook() called on them,
so _hook retained its default value of TS_HTTP_READ_RESPONSE_HDR_HOOK.
When set-status executed at REMAP_PSEUDO_HOOK time, exec() took the
response-hook branch and called TSHttpHdrStatusSet on a request buffer,
corrupting the header union and crashing.
Fix the operator to check TSHttpHdrTypeGet() at runtime instead of
trusting get_hook(). Removed _hook and get_hook() from operators.
set_hook() becomes is_hook_valid() in Statement and RuleSet to
only validate hook compatibility without mutating _hook.
Upgrade the HTTPHdr::status_set/reason_set asserts to release_assert so
that writing response fields into a request buffer is caught immediately
rather than silently corrupting memory.
(cherry picked from commit e4728cc49625cc3767f77b1ecd8ad0ebdf17d016)
---
include/proxy/hdrs/HTTP.h | 4 ++--
plugins/header_rewrite/operators.cc | 18 ++++++------------
plugins/header_rewrite/ruleset.cc | 4 ++--
plugins/header_rewrite/statement.cc | 10 ++--------
plugins/header_rewrite/statement.h | 10 ++--------
src/proxy/hdrs/HTTP.cc | 4 ++--
6 files changed, 16 insertions(+), 34 deletions(-)
diff --git a/include/proxy/hdrs/HTTP.h b/include/proxy/hdrs/HTTP.h
index 22d6c45a7b..22195e6366 100644
--- a/include/proxy/hdrs/HTTP.h
+++ b/include/proxy/hdrs/HTTP.h
@@ -1091,7 +1091,7 @@ inline void
HTTPHdr::status_set(HTTPStatus status)
{
ink_assert(valid());
- ink_assert(m_http->m_polarity == HTTP_TYPE_RESPONSE);
+ ink_release_assert(m_http->m_polarity == HTTP_TYPE_RESPONSE);
http_hdr_status_set(m_http, status);
}
@@ -1115,7 +1115,7 @@ inline void
HTTPHdr::reason_set(const char *value, int length)
{
ink_assert(valid());
- ink_assert(m_http->m_polarity == HTTP_TYPE_RESPONSE);
+ ink_release_assert(m_http->m_polarity == HTTP_TYPE_RESPONSE);
http_hdr_reason_set(m_heap, m_http, value, length, true);
}
diff --git a/plugins/header_rewrite/operators.cc
b/plugins/header_rewrite/operators.cc
index 0b6b30375b..f572ae7f5c 100644
--- a/plugins/header_rewrite/operators.cc
+++ b/plugins/header_rewrite/operators.cc
@@ -202,19 +202,13 @@ OperatorSetStatus::initialize_hooks()
bool
OperatorSetStatus::exec(const Resources &res) const
{
- switch (get_hook()) {
- case TS_HTTP_READ_RESPONSE_HDR_HOOK:
- case TS_HTTP_SEND_RESPONSE_HDR_HOOK:
- if (res.bufp && res.hdr_loc) {
- TSHttpHdrStatusSet(res.bufp, res.hdr_loc,
static_cast<TSHttpStatus>(_status.get_int_value()));
- if (_reason && _reason_len > 0) {
- TSHttpHdrReasonSet(res.bufp, res.hdr_loc, _reason, _reason_len);
- }
+ if (res.bufp && res.hdr_loc && TSHttpHdrTypeGet(res.bufp, res.hdr_loc) ==
TS_HTTP_TYPE_RESPONSE) {
+ TSHttpHdrStatusSet(res.bufp, res.hdr_loc,
static_cast<TSHttpStatus>(_status.get_int_value()));
+ if (_reason && _reason_len > 0) {
+ TSHttpHdrReasonSet(res.bufp, res.hdr_loc, _reason, _reason_len);
}
- break;
- default:
+ } else {
TSHttpTxnStatusSet(res.state.txnp,
static_cast<TSHttpStatus>(_status.get_int_value()));
- break;
}
Dbg(pi_dbg_ctl, "OperatorSetStatus::exec() invoked with status=%d",
_status.get_int_value());
@@ -602,7 +596,7 @@ OperatorSetRedirect::exec(const Resources &res) const
const_cast<Resources &>(res).changed_url = true;
res._rri->redirect = 1;
} else {
- Dbg(pi_dbg_ctl, "OperatorSetRedirect::exec() hook=%d", int(get_hook()));
+ Dbg(pi_dbg_ctl, "OperatorSetRedirect::exec() redirect to %s",
value.c_str());
// Set the new status code and reason.
TSHttpStatus status = static_cast<TSHttpStatus>(_status.get_int_value());
TSHttpHdrStatusSet(res.bufp, res.hdr_loc, status);
diff --git a/plugins/header_rewrite/ruleset.cc
b/plugins/header_rewrite/ruleset.cc
index 501315e063..c301522382 100644
--- a/plugins/header_rewrite/ruleset.cc
+++ b/plugins/header_rewrite/ruleset.cc
@@ -53,7 +53,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)) {
+ if (!c->is_hook_valid(_hook)) {
delete c;
TSError("[%s] in %s:%d: can't use this condition in hook=%s: %%{%s} with
arg: %s", PLUGIN_NAME, filename, lineno,
TSHttpHookNameLookup(_hook), p.get_op().c_str(),
p.get_arg().c_str());
@@ -82,7 +82,7 @@ RuleSet::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);
op->initialize(p);
- if (!op->set_hook(_hook)) {
+ 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());
diff --git a/plugins/header_rewrite/statement.cc
b/plugins/header_rewrite/statement.cc
index ca840d7a9b..f4335e243e 100644
--- a/plugins/header_rewrite/statement.cc
+++ b/plugins/header_rewrite/statement.cc
@@ -48,15 +48,9 @@ Statement::get_resource_ids() const
}
bool
-Statement::set_hook(TSHttpHookID hook)
+Statement::is_hook_valid(TSHttpHookID hook) const
{
- bool ret = std::find(_allowed_hooks.begin(), _allowed_hooks.end(), hook) !=
_allowed_hooks.end();
-
- if (ret) {
- _hook = hook;
- }
-
- return ret;
+ return std::find(_allowed_hooks.begin(), _allowed_hooks.end(), hook) !=
_allowed_hooks.end();
}
// This should be overridden for any Statement which only supports some hooks
diff --git a/plugins/header_rewrite/statement.h
b/plugins/header_rewrite/statement.h
index 510957837a..724017beb5 100644
--- a/plugins/header_rewrite/statement.h
+++ b/plugins/header_rewrite/statement.h
@@ -143,13 +143,8 @@ public:
Statement(const Statement &) = delete;
void operator=(const Statement &) = delete;
- // Which hook are we adding this statement to?
- bool set_hook(TSHttpHookID hook);
- TSHttpHookID
- get_hook() const
- {
- return _hook;
- }
+ // Validate that this statement is allowed on the given hook. Used during
parsing only.
+ bool is_hook_valid(TSHttpHookID hook) const;
// Which hooks are this "statement" applicable for? Used during parsing only.
void
@@ -244,7 +239,6 @@ protected:
private:
ResourceIDs _rsrc = RSRC_NONE;
- TSHttpHookID _hook = TS_HTTP_READ_RESPONSE_HDR_HOOK;
std::vector<TSHttpHookID> _allowed_hooks;
std::string _config_filename;
int _config_lineno = 0;
diff --git a/src/proxy/hdrs/HTTP.cc b/src/proxy/hdrs/HTTP.cc
index a4d350ee9e..e3fc63d763 100644
--- a/src/proxy/hdrs/HTTP.cc
+++ b/src/proxy/hdrs/HTTP.cc
@@ -754,7 +754,7 @@ http_hdr_url_set(HdrHeap *heap, HTTPHdrImpl *hh, URLImpl
*url)
void
http_hdr_status_set(HTTPHdrImpl *hh, HTTPStatus status)
{
- ink_assert(hh->m_polarity == HTTP_TYPE_RESPONSE);
+ ink_release_assert(hh->m_polarity == HTTP_TYPE_RESPONSE);
hh->u.resp.m_status = status;
}
@@ -775,7 +775,7 @@ http_hdr_reason_get(HTTPHdrImpl *hh, int *length)
void
http_hdr_reason_set(HdrHeap *heap, HTTPHdrImpl *hh, const char *value, int
length, bool must_copy)
{
- ink_assert(hh->m_polarity == HTTP_TYPE_RESPONSE);
+ ink_release_assert(hh->m_polarity == HTTP_TYPE_RESPONSE);
mime_str_u16_set(heap, value, length, &(hh->u.resp.m_ptr_reason),
&(hh->u.resp.m_len_reason), must_copy);
}
