[header_rewrite] Fix set-body

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)

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;
 }
 

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);

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()