Fix set-status crash inside if/endif at remap time

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.

Includes an autest that reproduces the crash scenario: set-status 403
inside an if/endif block under REMAP_PSEUDO_HOOK.

Author: moonchen

include/proxy/hdrs/HTTP.h

@@ -1088,7 +1088,7 @@ inline void
 HTTPHdr::status_set(HTTPStatus status)
 {
   ink_assert(valid());
-  ink_assert(m_http->m_polarity == HTTPType::RESPONSE);
+  ink_release_assert(m_http->m_polarity == HTTPType::RESPONSE);
 
   http_hdr_status_set(m_http, status);
 }
@@ -1112,7 +1112,7 @@ inline void
 HTTPHdr::reason_set(std::string_view value)
 {
   ink_assert(valid());
-  ink_assert(m_http->m_polarity == HTTPType::RESPONSE);
+  ink_release_assert(m_http->m_polarity == HTTPType::RESPONSE);
 
   http_hdr_reason_set(m_heap, m_http, value, true);
 }

plugins/header_rewrite/operators.cc

@@ -198,19 +198,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()), res.state.txnp, PLUGIN_NAME);
-      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()), res.state.txnp, PLUGIN_NAME);
+    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()), PLUGIN_NAME);
-    break;
   }
 
   Dbg(pi_dbg_ctl, "OperatorSetStatus::exec() invoked with status=%d", _status.get_int_value());
@@ -601,7 +595,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, res.state.txnp, PLUGIN_NAME);

plugins/header_rewrite/ruleset.cc

@@ -66,7 +66,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->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());
@@ -93,7 +93,7 @@ RuleSet::add_operator(Parser &p, const char *filename, int lineno)
   if (nullptr != 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->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());

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

plugins/header_rewrite/statement.h

@@ -144,13 +144,8 @@ class Statement
   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
@@ -267,7 +262,6 @@ class Statement
 
 private:
   ResourceIDs               _rsrc = RSRC_NONE;
-  TSHttpHookID              _hook = TS_HTTP_READ_RESPONSE_HDR_HOOK;
   std::vector<TSHttpHookID> _allowed_hooks;
   bool                      _initialized = false;
 };

src/proxy/hdrs/HTTP.cc

@@ -693,7 +693,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 == HTTPType::RESPONSE);
+  ink_release_assert(hh->m_polarity == HTTPType::RESPONSE);
   hh->u.resp.m_status = static_cast<int16_t>(status);
 }
 
@@ -713,7 +713,7 @@ http_hdr_reason_get(HTTPHdrImpl *hh)
 void
 http_hdr_reason_set(HdrHeap *heap, HTTPHdrImpl *hh, std::string_view value, bool must_copy)
 {
-  ink_assert(hh->m_polarity == HTTPType::RESPONSE);
+  ink_release_assert(hh->m_polarity == HTTPType::RESPONSE);
   mime_str_u16_set(heap, value, &(hh->u.resp.m_ptr_reason), &(hh->u.resp.m_len_reason), must_copy);
 }
 

tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bundle.replay.yaml

@@ -176,6 +176,13 @@ autest:
             args:
               - "rules/rule_session_vars.conf"
 
+      - from: "http://www.example.com/from_18/"
+        to: "http://backend.ex:{SERVER_HTTP_PORT}/to_18/"
+        plugins:
+          - name: "header_rewrite.so"
+            args:
+              - "rules/set_status_in_if.conf"
+
     log_validation:
       traffic_out:
         excludes:
@@ -1880,3 +1887,27 @@ sessions:
       headers:
         fields:
         - [ X-Session-Seen, { value: "yes", as: equal } ]
+
+# Test 67: set-status inside if/endif at REMAP_PSEUDO_HOOK time.
+# Before the fix, the set-status operator inside an if/endif block retained
+# its default _hook (TS_HTTP_READ_RESPONSE_HDR_HOOK), so exec() called
+# TSHttpHdrStatusSet on a request buffer, corrupting the union and crashing.
+- transactions:
+  - client-request:
+      method: "GET"
+      version: "1.1"
+      url: /from_18/test
+      headers:
+        fields:
+        - [ Host, www.example.com ]
+        - [ uuid, 67 ]
+
+    server-response:
+      status: 200
+      reason: OK
+      headers:
+        fields:
+        - [ Connection, close ]
+
+    proxy-response:
+      status: 403

tests/gold_tests/pluginTest/header_rewrite/rules/set_status_in_if.conf

@@ -0,0 +1,27 @@
+#
+# 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.
+#
+# Minimal reproduction for set-status inside if/endif at remap time.
+# Before the fix, the operator's _hook defaulted to
+# TS_HTTP_READ_RESPONSE_HDR_HOOK, so exec() called TSHttpHdrStatusSet
+# on a request buffer, corrupting the header union and crashing.
+#
+cond %{REMAP_PSEUDO_HOOK}
+    if
+        cond %{CLIENT-URL:PATH} /from_18/
+            set-status 403
+    endif