This is an automated email from the ASF dual-hosted git repository.
mochen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 56ec03c35f [header_rewrite] Fix set-body (#12560)
56ec03c35f is described below
commit 56ec03c35f144e1c2b6dd8742d9c46852e23b658
Author: Mo Chen <[email protected]>
AuthorDate: Tue Oct 21 17:24:19 2025 -0500
[header_rewrite] Fix set-body (#12560)
1. set-body with an empty string argument causes heap corruption in
The cause is incorrect handling of zero-length strings in
MIOBuffer::append_xmalloced.
2. set-body with variable expansion fails to expand the variable.
The cause is a typo in the OperatorSetBody::exec() method.
---
include/iocore/eventsystem/IOBuffer.h | 12 ++-
plugins/header_rewrite/operators.cc | 7 +-
src/iocore/eventsystem/P_IOBuffer.h | 4 +
.../header_rewrite/header_rewrite_set_body.test.py | 110 +++++++++++++++++++++
4 files changed, 129 insertions(+), 4 deletions(-)
diff --git a/include/iocore/eventsystem/IOBuffer.h
b/include/iocore/eventsystem/IOBuffer.h
index 33c151ff05..de01b59a6a 100644
--- a/include/iocore/eventsystem/IOBuffer.h
+++ b/include/iocore/eventsystem/IOBuffer.h
@@ -102,8 +102,16 @@ enum AllocType {
#define BUFFER_SIZE_INDEX_IS_FAST_ALLOCATED(_size_index)
(((uint64_t)_size_index) < DEFAULT_BUFFER_SIZES)
#define BUFFER_SIZE_INDEX_IS_CONSTANT(_size_index) (_size_index >=
DEFAULT_BUFFER_SIZES)
-#define BUFFER_SIZE_FOR_XMALLOC(_size) (-(_size))
-#define BUFFER_SIZE_INDEX_FOR_XMALLOC_SIZE(_size) (-(_size))
+#define BUFFER_SIZE_FOR_XMALLOC(_size) (-(_size))
+[[nodiscard]] constexpr int64_t
+BUFFER_SIZE_INDEX_FOR_XMALLOC_SIZE(int64_t size)
+{
+ // Positive size indices are interpreted as a BUFFER_SIZE_INDEX_*.
+ // Negative size indices are interpreted as a malloc size.
+ // A zero size index is BUFFER_SIZE_INDEX_128, which causes this buffer to
be freed incorrectly.
+ ink_release_assert(size > 0 && "Zero-length xmalloc buffer causes heap
corruption!");
+ return -size;
+}
#define BUFFER_SIZE_FOR_CONSTANT(_size) (_size -
DEFAULT_BUFFER_SIZES)
#define BUFFER_SIZE_INDEX_FOR_CONSTANT_SIZE(_size) (_size +
DEFAULT_BUFFER_SIZES)
diff --git a/plugins/header_rewrite/operators.cc
b/plugins/header_rewrite/operators.cc
index e1a9683d3c..ac7b750440 100644
--- a/plugins/header_rewrite/operators.cc
+++ b/plugins/header_rewrite/operators.cc
@@ -816,8 +816,11 @@ OperatorSetBody::exec(const Resources &res) const
std::string value;
_value.append_value(value, res);
- char *msg = TSstrdup(_value.get_value().c_str());
- TSHttpTxnErrorBodySet(res.state.txnp, msg, _value.size(), nullptr);
+ char *msg = nullptr;
+ if (!value.empty()) {
+ msg = TSstrdup(value.c_str());
+ }
+ TSHttpTxnErrorBodySet(res.state.txnp, msg, value.size(), nullptr);
return true;
}
diff --git a/src/iocore/eventsystem/P_IOBuffer.h
b/src/iocore/eventsystem/P_IOBuffer.h
index a72ff11556..e159cbafc5 100644
--- a/src/iocore/eventsystem/P_IOBuffer.h
+++ b/src/iocore/eventsystem/P_IOBuffer.h
@@ -942,6 +942,10 @@ MIOBuffer::set(void *b, int64_t len)
TS_INLINE void
MIOBuffer::append_xmalloced(void *b, int64_t len)
{
+ if (len == 0) {
+ return;
+ }
+
IOBufferBlock *x = new_IOBufferBlock_internal(_location);
x->set_internal(b, len, BUFFER_SIZE_INDEX_FOR_XMALLOC_SIZE(len));
append_block_internal(x);
diff --git
a/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_set_body.test.py
b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_set_body.test.py
new file mode 100644
index 0000000000..05c3b8b907
--- /dev/null
+++ b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_set_body.test.py
@@ -0,0 +1,110 @@
+'''
+'''
+# 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.
+
+Test.Summary = '''
+Test for empty set-body operator value.
+'''
+
+Test.ContinueOnFail = True
+
+
+class HeaderRewriteSetBodyTest:
+
+ def __init__(self):
+ self.setUpOriginServer()
+ self.setUpTS()
+
+ def setUpOriginServer(self):
+ self.server = Test.MakeOriginServer("server")
+
+ # Response for original transaction
+ request_header = {"headers": "GET /test HTTP/1.1\r\nHost:
www.example.com\r\n\r\n", "body": ""}
+
+ response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection:
close\r\n\r\n", "body": "ATS should not serve this body"}
+ self.server.addResponse("sessionlog.log", request_header,
response_header)
+
+ def setUpTS(self):
+ self.ts = Test.MakeATSProcess("ts")
+ empty_body_rule_file = Test.Disk.File("empty_body_rule.conf", "txt",
"")
+ empty_body_rule_file.WriteOn(
+ '''
+ cond %{REMAP_PSEUDO_HOOK}
+ set-status 200
+
+ cond %{SEND_RESPONSE_HDR_HOOK}
+ set-body ""
+ ''')
+ self.ts.Disk.remap_config.AddLine(
+ f'map http://www.example.com/emptybody
http://127.0.0.1:{self.server.Variables.Port}/test @plugin=header_rewrite.so
@pparam={empty_body_rule_file.AbsRunTimePath}'
+ )
+
+ set_body_rule_file = Test.Disk.File("set_body_rule.conf", "txt", "")
+ set_body_rule_file.WriteOn(
+ '''
+ cond %{REMAP_PSEUDO_HOOK}
+ set-status 200
+
+ cond %{SEND_RESPONSE_HDR_HOOK}
+ set-body "%{STATUS}"
+ ''')
+ self.ts.Disk.remap_config.AddLine(
+ f'map http://www.example.com/setbody
http://127.0.0.1:{self.server.Variables.Port}/test @plugin=header_rewrite.so
@pparam={set_body_rule_file.AbsRunTimePath}'
+ )
+
+ # Enable debug logging to help diagnose issues
+ self.ts.Disk.records_config.update(
+ {
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http|header_rewrite',
+ })
+
+ def test_empty_body(self):
+ '''
+ Test that empty set-body doesn't crash the server and properly deletes
internal error body
+ '''
+ tr = Test.AddTestRun()
+ tr.MakeCurlCommand(f'-s -v --proxy 127.0.0.1:{self.ts.Variables.port}
"http://www.example.com/emptybody";', ts=self.ts)
+ tr.Processes.Default.ReturnCode = 0
+ tr.Processes.Default.StartBefore(self.server)
+ tr.Processes.Default.StartBefore(self.ts)
+ tr.Processes.Default.Streams.stderr.Content =
Testers.ContainsExpression("200 OK", "expected 200 response")
+ tr.Processes.Default.Streams.stderr.Content +=
Testers.ContainsExpression("Content-Length: 0", "expected content-length 0")
+ tr.Processes.Default.Streams.stdout.Content =
Testers.ExcludesExpression("should not", "body should be removed")
+
+ tr.StillRunningAfter = self.ts # Verify server didn't crash
+
+ def test_set_body(self):
+ '''
+ Test that set-body with a variable works correctly
+ '''
+ tr = Test.AddTestRun()
+ tr.MakeCurlCommand(f'-s -v --proxy 127.0.0.1:{self.ts.Variables.port}
"http://www.example.com/setbody";', ts=self.ts)
+ tr.Processes.Default.ReturnCode = 0
+ tr.Processes.Default.Streams.stderr.Content =
Testers.ContainsExpression("200 OK", "expected 200 response")
+ tr.Processes.Default.Streams.stderr.Content +=
Testers.ContainsExpression("Content-Length: 3", "expected content-length 3")
+ tr.Processes.Default.Streams.stdout.Content =
Testers.ContainsExpression("200", "body should be set to 200")
+ tr.Processes.Default.Streams.stdout.Content +=
Testers.ExcludesExpression("should not", "body should be removed")
+
+ tr.StillRunningAfter = self.ts # Verify server didn't crash
+
+ def run(self):
+ self.test_empty_body()
+ self.test_set_body()
+
+
+HeaderRewriteSetBodyTest().run()
