bryancall commented on code in PR #12879:
URL: https://github.com/apache/trafficserver/pull/12879#discussion_r3018609247
##########
src/proxy/http/HttpSM.cc:
##########
@@ -1685,9 +1685,20 @@ HttpSM::handle_api_return()
switch (t_state.next_action) {
case HttpTransact::StateMachineAction_t::TRANSFORM_READ: {
- HttpTunnelProducer *p = setup_transfer_from_transform();
- perform_transform_cache_write_action();
- tunnel.tunnel_run(p);
+ if (t_state.internal_msg_buffer) {
+ // A plugin replaced the response body via TSHttpTxnErrorBodySet().
+ // Use internal transfer instead of the transform tunnel.
+ SMDbg(dbg_ctl_http, "plugin set internal body, bypassing transform for
internal transfer");
+ if (server_txn != nullptr) {
+ do_drain_server_response_body();
+ release_server_session();
+ }
+ setup_internal_transfer(&HttpSM::tunnel_handler);
Review Comment:
Fixed in c9c19bcd6c — added `!t_state.api_server_request_body_set` guard to
the `TRANSFORM_READ` branch in `handle_api_return()`.
##########
src/proxy/http/HttpSM.cc:
##########
@@ -1720,6 +1731,15 @@ HttpSM::handle_api_return()
}
setup_blind_tunnel(true, initial_data);
+ } else if (t_state.internal_msg_buffer) {
+ // A plugin replaced the origin response body via
TSHttpTxnErrorBodySet().
+ // Drain the origin body if possible, then use internal transfer.
+ SMDbg(dbg_ctl_http, "plugin set internal body, using internal transfer
instead of server tunnel");
+ if (server_txn != nullptr) {
+ do_drain_server_response_body();
+ release_server_session();
+ }
+ setup_internal_transfer(&HttpSM::tunnel_handler);
Review Comment:
Fixed in c9c19bcd6c — added `!t_state.api_server_request_body_set` guard to
the `SERVER_READ` branch as well.
##########
src/proxy/http/HttpSM.cc:
##########
@@ -1685,9 +1685,20 @@ HttpSM::handle_api_return()
switch (t_state.next_action) {
case HttpTransact::StateMachineAction_t::TRANSFORM_READ: {
- HttpTunnelProducer *p = setup_transfer_from_transform();
- perform_transform_cache_write_action();
- tunnel.tunnel_run(p);
+ if (t_state.internal_msg_buffer) {
+ // A plugin replaced the response body via TSHttpTxnErrorBodySet().
+ // Use internal transfer instead of the transform tunnel.
+ SMDbg(dbg_ctl_http, "plugin set internal body, bypassing transform for
internal transfer");
+ if (server_txn != nullptr) {
+ do_drain_server_response_body();
+ release_server_session();
Review Comment:
Good catch. `release_server_session()` only pools connections for
304/auth-HEAD responses, so the drain doesn't enable actual connection reuse
for typical 200/403 responses. Updated the docs in c9c19bcd6c to say "clean
connection shutdown" instead of "connection reuse." The drain still has value —
it prevents RSTs by cleanly consuming the origin bytes before closing. Widening
the pooling condition in `release_server_session()` would be a broader change
best done separately.
##########
doc/admin-guide/plugins/header_rewrite.en.rst:
##########
@@ -1142,35 +1142,102 @@ set-body
set-body <text>
-Sets the body to ``<text>``. Can also be used to delete a body with ``""``.
This is only useful when overriding the origin status, i.e.
-intercepting/pre-empting a request so that you can override the body from the
body-factory with your own.
+Sets the response body to ``<text>``. Can also be used to delete a body with
``""``.
+
+This operator can be used to replace the response body in three scenarios:
+
+1. **Synthetic responses (no origin connection)**: When used at
``REMAP_PSEUDO_HOOK``
+ (before the origin connection is established), ``set-body`` will skip the
origin
+ connection entirely and serve a synthetic response directly. Use
``set-status`` to
+ control the response status code (defaults to 200 OK if not specified).
This is
+ useful for blocking requests, serving synthetic pages, or returning canned
responses
+ without touching the origin.
+
+2. **ATS-generated responses**: When overriding the origin status at remap
time (e.g.
+ with ``set-status``), you can use ``set-body`` at
``SEND_RESPONSE_HDR_HOOK`` to
+ override the body from the body-factory with your own.
+
+3. **Origin server responses**: When the origin returns a response with a body,
+ ``set-body`` can replace the origin body with the specified text. This is
useful
+ for sanitizing error responses (e.g. replacing a 403 body that contains
sensitive
+ information). Use ``READ_RESPONSE_HDR_HOOK`` to inspect the origin response
+ headers and replace the body, or ``SEND_RESPONSE_HDR_HOOK`` to replace the
body
+ just before sending to the client.
+
+When replacing an origin response body (scenario 3), ATS will attempt to drain
the
+origin body from the server connection buffer so the connection can be reused.
If the
+origin body is too large to be fully buffered or uses chunked encoding, the
server
+connection will be closed instead.
+
+.. note::
+
+ When replacing an origin response body, the origin's response headers are
+ preserved. If the origin headers contain sensitive information (e.g.
+ ``Server``, ``X-Amz-Request-Id``, or other internal identifiers), use
+ ``rm-header`` to strip them::
+
+ cond %{READ_RESPONSE_HDR_HOOK} [AND]
+ cond %{STATUS} =403
+ set-body "Access Denied"
+ rm-header Server
+ rm-header X-Amz-Request-Id
+
+ If a response transform plugin is also active on the same transaction,
+ ``set-body`` takes precedence and the transform is bypassed.
+
+Example: block a request at remap time without contacting the origin::
+
+ cond %{REMAP_PSEUDO_HOOK} [AND]
+ cond %{CLIENT-URL:PATH} /blocked
+ set-status 403
+ set-body "Access Denied"
+
+Example: sanitize a 403 response from the origin::
+
+ cond %{READ_RESPONSE_HDR_HOOK} [AND]
+ cond %{STATUS} =403
+ set-body "Access Denied"
set-body-from
~~~~~~~~~~~~~
::
set-body-from <URL>
-Will call ``<URL>`` (see URL in `URL Parts`_) to retrieve a custom error
response
-and set the body with the result. Triggering this rule on an OK transaction
will
-send a 500 status code to the client with the desired response. If this is
triggered
-on any error status code, that original status code will be sent to the client.
+Will call ``<URL>`` (see URL in `URL Parts`_) to retrieve a response body from
a
+secondary URL and use it to replace the origin's response body. The origin's
response
+status code and headers are preserved, and the fetched content replaces only
the body.
+
+This operator can be used at ``READ_RESPONSE_HDR_HOOK`` or
``SEND_RESPONSE_HDR_HOOK``.
+When the origin response body is fully buffered and has a known
``Content-Length``,
+ATS will drain the origin body to allow connection reuse. Otherwise, the origin
+connection is closed.
+
+If the fetch fails or times out, the original origin response (including its
body)
+is sent unmodified. This means ``set-body-from`` does **not** guarantee the
origin
+body will be hidden from the client -- only successful fetches result in body
+replacement. Use ``set-body`` instead if the replacement text is static and
must
Review Comment:
Fixed in c9c19bcd6c — clarified the docs to explain that certain fetch
connection failures can cause ATS to generate an internal error page that
`set-body-from` treats as a successful fetch, replacing the origin body with
ATS error page content.
##########
doc/admin-guide/plugins/header_rewrite.en.rst:
##########
@@ -1142,35 +1142,102 @@ set-body
set-body <text>
-Sets the body to ``<text>``. Can also be used to delete a body with ``""``.
This is only useful when overriding the origin status, i.e.
-intercepting/pre-empting a request so that you can override the body from the
body-factory with your own.
+Sets the response body to ``<text>``. Can also be used to delete a body with
``""``.
+
+This operator can be used to replace the response body in three scenarios:
+
+1. **Synthetic responses (no origin connection)**: When used at
``REMAP_PSEUDO_HOOK``
+ (before the origin connection is established), ``set-body`` will skip the
origin
+ connection entirely and serve a synthetic response directly. Use
``set-status`` to
+ control the response status code (defaults to 200 OK if not specified).
This is
+ useful for blocking requests, serving synthetic pages, or returning canned
responses
+ without touching the origin.
+
+2. **ATS-generated responses**: When overriding the origin status at remap
time (e.g.
+ with ``set-status``), you can use ``set-body`` at
``SEND_RESPONSE_HDR_HOOK`` to
+ override the body from the body-factory with your own.
+
+3. **Origin server responses**: When the origin returns a response with a body,
+ ``set-body`` can replace the origin body with the specified text. This is
useful
+ for sanitizing error responses (e.g. replacing a 403 body that contains
sensitive
+ information). Use ``READ_RESPONSE_HDR_HOOK`` to inspect the origin response
+ headers and replace the body, or ``SEND_RESPONSE_HDR_HOOK`` to replace the
body
+ just before sending to the client.
+
+When replacing an origin response body (scenario 3), ATS will attempt to drain
the
+origin body from the server connection buffer so the connection can be reused.
If the
+origin body is too large to be fully buffered or uses chunked encoding, the
server
+connection will be closed instead.
+
+.. note::
+
+ When replacing an origin response body, the origin's response headers are
+ preserved. If the origin headers contain sensitive information (e.g.
+ ``Server``, ``X-Amz-Request-Id``, or other internal identifiers), use
+ ``rm-header`` to strip them::
Review Comment:
Good catch. `set-body` and `set-body-from` both pass `nullptr` as the
mimetype to `TSHttpTxnErrorBodySet()`, so `setup_internal_transfer()` defaults
`Content-Type` to `text/html`. Fixed the docs in c9c19bcd6c to accurately state
that `Content-Type` is overwritten, and to recommend using `set-header
Content-Type` to restore the desired type.
##########
tests/gold_tests/pluginTest/header_rewrite/rules/rule_set_body_from_send_resp.conf:
##########
@@ -0,0 +1,21 @@
+#
+# 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.
+
+# Replace the origin response body with content fetched from a secondary URL
+# at SEND_RESPONSE_HDR_HOOK.
+cond %{SEND_RESPONSE_HDR_HOOK}
+set-body-from http://www.example.com/custom_body
Review Comment:
Fixed in c9c19bcd6c — indented the operator lines in all three
`rule_set_body_from*.conf` files for consistency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
