From bd81037b4d3c29e9fb368d184a1e42dfcc9f2096 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Thu, 25 Jun 2026 09:44:00 -0500 Subject: [PATCH 01/15] slice_prefetch.test.py: address cache.log flakiness (#13322) Background slice prefetch requests can log around the client transactions. A run can therefore contain the expected cache records while still failing an ordered cache.log gold comparison. This replaces the full-file gold comparison with per-entry log assertions that verify the expected cache and prefetch records independently of ordering, then removes the unused gold file. Fixes: #13311 (cherry picked from commit c179b6b9bbc0853d2a184296ec38a37b5032e4c4) --- .../pluginTest/slice/gold/slice_prefetch.gold | 26 ------------ .../pluginTest/slice/slice_prefetch.test.py | 42 ++++++++++++++++--- 2 files changed, 37 insertions(+), 31 deletions(-) delete mode 100644 tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold diff --git a/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold b/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold deleted file mode 100644 index 3445708de2c..00000000000 --- a/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold +++ /dev/null @@ -1,26 +0,0 @@ -bytes 0-6/18 miss -bytes ``/18 miss -bytes ``/18 miss -bytes 14-17/18 hit-fresh -- miss, none -bytes 0-6/18 hit-fresh -bytes 7-13/18 hit-fresh -bytes 14-17/18 hit-fresh -- hit-fresh, none -bytes 0-6/18 hit-stale -bytes ``/18 hit-stale -bytes ``/18 hit-stale -bytes 14-17/18 hit-fresh -- hit-stale, none -bytes 0-6/18 hit-fresh -bytes 7-13/18 hit-fresh -bytes 14-17/18 hit-fresh -bytes 0-17/18 hit-fresh, none -bytes 0-4/18 miss -bytes ``/18 miss -bytes ``/18 miss -bytes ``/18 miss -bytes 15-17/18 hit-fresh -bytes 5-16/18 miss, none -bytes 0-6/18 hit-fresh -*/18 hit-fresh, none diff --git a/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py b/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py index 5b544e9d091..28eecfe2ee2 100644 --- a/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py +++ b/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py @@ -16,6 +16,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import os +import re Test.Summary = ''' slice plugin prefetch feature test @@ -174,8 +175,39 @@ # 6 Test - All requests (client & slice internal) logs to see background fetches cache_file = os.path.join(ts.Variables.LOGDIR, 'cache.log') -# Wait for log file to appear, then wait one extra second to make sure TS is done writing it. -test_run = Test.AddTestRun("Checking debug logs for background fetches") -test_run.Processes.Default.Command = (os.path.join(Test.Variables.AtsTestToolsDir, 'condwait') + ' 60 1 -f ' + cache_file) -ts.Disk.File(cache_file).Content = "gold/slice_prefetch.gold" -test_run.Processes.Default.ReturnCode = 0 +# Wait for the final cache log line to be written. +test_run = Test.AddAwaitFileContainsTestRun( + "Checking debug logs for background fetches", + cache_file, + r'\*/18 hit-fresh, none$', +) +cache_log = ts.Disk.File(cache_file) +expected_cache_entries = [ + "bytes 0-6/18 miss", + "bytes 7-13/18 miss", + "bytes 14-17/18 miss", + "bytes 14-17/18 hit-fresh", + "- miss, none", + "bytes 0-6/18 hit-fresh", + "bytes 7-13/18 hit-fresh", + "bytes 14-17/18 hit-fresh", + "- hit-fresh, none", + "bytes 0-6/18 hit-stale", + "bytes 7-13/18 hit-stale", + "bytes 14-17/18 hit-stale", + "- hit-stale, none", + "bytes 0-17/18 hit-fresh, none", + "bytes 0-4/18 miss", + "bytes 5-9/18 miss", + "bytes 10-14/18 miss", + "bytes 15-17/18 miss", + "bytes 15-17/18 hit-fresh", + "bytes 5-16/18 miss, none", + "*/18 hit-fresh, none", +] +for index, entry in enumerate(expected_cache_entries): + tester = Testers.ContainsExpression(f'(?m)^{re.escape(entry)}$', f'Verify cache log contains: {entry}') + if index == 0: + cache_log.Content = tester + else: + cache_log.Content += tester From 790d2e4540d2128bb6e5136b2e06036a900794f0 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Fri, 26 Jun 2026 19:49:09 -0500 Subject: [PATCH 02/15] Expand client IP debug logging test coverage (#13290) Before this patch, the client ip debug logging test only covered one HTTP transaction, so regressions across protocols or persistent client sessions could pass unnoticed. This converts the test to replay-driven coverage for HTTP, HTTPS, and HTTP/2, with an HTTP/3 scenario enabled when QUICHE is available. Each replay sends multiple transactions on one client connection and checks that all four request and response header dumps include per-transaction markers. This test found no issues, thus this is a test-only patch. (cherry picked from commit a2c55e3dbe918b229981074c6c81a92b5534c88f) --- .../autest-site/ats_replay.test.ext | 25 +++- .../logging/log-debug-client-ip.test.py | 39 ++--- .../log-debug-client-ip-http.replay.yaml | 129 +++++++++++++++++ .../log-debug-client-ip-http2.replay.yaml | 134 +++++++++++++++++ .../log-debug-client-ip-http3.replay.yaml | 136 ++++++++++++++++++ .../log-debug-client-ip-https.replay.yaml | 134 +++++++++++++++++ 6 files changed, 559 insertions(+), 38 deletions(-) create mode 100644 tests/gold_tests/logging/replay/log-debug-client-ip-http.replay.yaml create mode 100644 tests/gold_tests/logging/replay/log-debug-client-ip-http2.replay.yaml create mode 100644 tests/gold_tests/logging/replay/log-debug-client-ip-http3.replay.yaml create mode 100644 tests/gold_tests/logging/replay/log-debug-client-ip-https.replay.yaml diff --git a/tests/gold_tests/autest-site/ats_replay.test.ext b/tests/gold_tests/autest-site/ats_replay.test.ext index 92332b3d76c..f258ea14497 100644 --- a/tests/gold_tests/autest-site/ats_replay.test.ext +++ b/tests/gold_tests/autest-site/ats_replay.test.ext @@ -19,9 +19,19 @@ Implement general-purpose ATS test extensions using proxy verifier replay files. from typing import Optional import os +import re import yaml +def _contains_expression(contains_entry: dict, default_description: str): + '''Create a ContainsExpression tester from a log validation entry.''' + expression = contains_entry['expression'] + description = contains_entry.get('description', default_description) + reflags = re.S | re.M if contains_entry.get('multiline', False) else 0 + + return Testers.ContainsExpression(expression, description, reflags=reflags) + + def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Optional['Process'] = None): '''Configure ATS per the configuration in the replay file. @@ -109,8 +119,7 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti traffic_out = log_validation.get('traffic_out', {}) for contains_entry in traffic_out.get('contains', []): expression = contains_entry['expression'] - description = contains_entry.get('description', f'Verify traffic_out contains: {expression}') - ts.Disk.traffic_out.Content += Testers.ContainsExpression(expression, description) + ts.Disk.traffic_out.Content += _contains_expression(contains_entry, f'Verify traffic_out contains: {expression}') for excludes_entry in traffic_out.get('excludes', []): expression = excludes_entry['expression'] description = excludes_entry.get('description', f'Verify traffic_out excludes: {expression}') @@ -124,8 +133,7 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti diags_log = log_validation.get('diags_log', {}) for contains_entry in diags_log.get('contains', []): expression = contains_entry['expression'] - description = contains_entry.get('description', f'Verify diags_log contains: {expression}') - ts.Disk.diags_log.Content += Testers.ContainsExpression(expression, description) + ts.Disk.diags_log.Content += _contains_expression(contains_entry, f'Verify diags_log contains: {expression}') for excludes_entry in diags_log.get('excludes', []): expression = excludes_entry['expression'] description = excludes_entry.get('description', f'Verify diags_log excludes: {expression}') @@ -196,6 +204,7 @@ def ATSReplayTest(obj, replay_file: str): ats_config = autest_config['ats'] process_config = ats_config.get('process_config', {}) enable_tls = process_config.get('enable_tls', ats_config.get('enable_tls', False)) + enable_quic = process_config.get('enable_quic', ats_config.get('enable_quic', False)) ts = configure_ats(tr, server=server, ats_config=ats_config, dns=dns) # Proxy Verifier Client configuration. @@ -203,10 +212,12 @@ def ATSReplayTest(obj, replay_file: str): raise ValueError(f"Replay file {replay_file} does not contain 'autest.client' section") client_config = autest_config['client'] name = client_config.get('name', 'client') - process_config = client_config.get('process_config', {}) - https_ports = [ts.Variables.ssl_port] if enable_tls else None + process_config = client_config.get('process_config', {}).copy() + http_ports = process_config.pop('http_ports', [ts.Variables.port]) + https_ports = process_config.pop('https_ports', [ts.Variables.ssl_port] if enable_tls else None) + http3_ports = process_config.pop('http3_ports', [ts.Variables.ssl_port] if enable_quic else None) client = tr.AddVerifierClientProcess( - name, replay_file, http_ports=[ts.Variables.port], https_ports=https_ports, **process_config) + name, replay_file, http_ports=http_ports, https_ports=https_ports, http3_ports=http3_ports, **process_config) # Set expected return code for client if specified. A list of codes is # wrapped in Any() so any of the listed values is accepted. diff --git a/tests/gold_tests/logging/log-debug-client-ip.test.py b/tests/gold_tests/logging/log-debug-client-ip.test.py index 4516fb893c5..a6894d8a65d 100644 --- a/tests/gold_tests/logging/log-debug-client-ip.test.py +++ b/tests/gold_tests/logging/log-debug-client-ip.test.py @@ -1,4 +1,5 @@ ''' +Verify debug logging filtered by client IP. ''' # Licensed to the Apache Software Foundation (ASF) under one # or more contributor license agreements. See the NOTICE file @@ -16,39 +17,15 @@ # See the License for the specific language governing permissions and # limitations under the License. -import os - Test.Summary = ''' -Test log filter. +Verify per-client-IP debug logging emits request and response header dumps. ''' -ts = Test.MakeATSProcess("ts", enable_cache=False) -replay_file = "log-filter.replays.yaml" -server = Test.MakeVerifierServerProcess("server", replay_file) -nameserver = Test.MakeDNServer("dns", default='127.0.0.1') - -ts.Disk.records_config.update( - { - 'proxy.config.diags.debug.enabled': 2, - 'proxy.config.diags.debug.tags': 'http', - 'proxy.config.diags.debug.client_ip': '127.0.0.1', - 'proxy.config.dns.nameservers': f"127.0.0.1:{nameserver.Variables.Port}", - }) -ts.Disk.remap_config.AddLine('map / http://localhost:{}/'.format(server.Variables.http_port)) +Test.ContinueOnFail = True -# Verify that the various aspects of the expected debug output for the -# transaction are logged. -ts.Disk.traffic_out.Content = Testers.ContainsExpression( - r"\+ Incoming Request \+", "Make sure the client request information is present.") -ts.Disk.traffic_out.Content += Testers.ContainsExpression( - r"\+ Proxy's Request after hooks \+", "Make sure the proxy request information is present.") -ts.Disk.traffic_out.Content += Testers.ContainsExpression( - r"\+ Incoming O.S. Response \+", "Make sure the server's response information is present.") -ts.Disk.traffic_out.Content += Testers.ContainsExpression( - r"\+ Proxy's Response 2 \+", "Make sure the proxy response information is present.") +Test.ATSReplayTest(replay_file='replay/log-debug-client-ip-http.replay.yaml') +Test.ATSReplayTest(replay_file='replay/log-debug-client-ip-https.replay.yaml') +Test.ATSReplayTest(replay_file='replay/log-debug-client-ip-http2.replay.yaml') -tr = Test.AddTestRun() -tr.Processes.Default.StartBefore(server) -tr.Processes.Default.StartBefore(ts) -tr.Processes.Default.StartBefore(nameserver) -tr.AddVerifierClientProcess("client-1", replay_file, http_ports=[ts.Variables.port], other_args="--keys test-1") +if Condition.HasATSFeature('TS_HAS_QUICHE') and Condition.HasCurlFeature('http3'): + Test.ATSReplayTest(replay_file='replay/log-debug-client-ip-http3.replay.yaml') diff --git a/tests/gold_tests/logging/replay/log-debug-client-ip-http.replay.yaml b/tests/gold_tests/logging/replay/log-debug-client-ip-http.replay.yaml new file mode 100644 index 00000000000..f92d3336d62 --- /dev/null +++ b/tests/gold_tests/logging/replay/log-debug-client-ip-http.replay.yaml @@ -0,0 +1,129 @@ +# 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. + +meta: + version: "1.0" + +autest: + description: 'Verify client-IP-filtered debug logging for HTTP/1.1' + + server: + name: 'server-log-debug-client-ip-http' + + client: + name: 'client-log-debug-client-ip-http' + + ats: + name: 'ts-log-debug-client-ip-http' + process_config: + enable_cache: false + + records_config: + proxy.config.diags.debug.enabled: 2 + proxy.config.diags.debug.tags: 'http' + proxy.config.diags.debug.client_ip: '127.0.0.1' + + remap_config: + - from: 'http://debug-client-ip-http.test/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + + log_validation: + traffic_out: + contains: + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-http-1' + description: 'HTTP transaction 1 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-http-1' + description: 'HTTP transaction 1 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-http-1' + description: 'HTTP transaction 1 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-http-1' + description: 'HTTP transaction 1 logs the proxy response headers.' + multiline: true + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-http-2' + description: 'HTTP transaction 2 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-http-2' + description: 'HTTP transaction 2 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-http-2' + description: 'HTTP transaction 2 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-http-2' + description: 'HTTP transaction 2 logs the proxy response headers.' + multiline: true + +sessions: +- transactions: + - client-request: + method: GET + url: /debug-http-1 + version: '1.1' + headers: + fields: + - [Host, debug-client-ip-http.test] + - [Content-Length, 0] + - [uuid, log-debug-http-1] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-http-1, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-http-1] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-http-1, as: equal }] + + - client-request: + method: GET + url: /debug-http-2 + version: '1.1' + headers: + fields: + - [Host, debug-client-ip-http.test] + - [Content-Length, 0] + - [uuid, log-debug-http-2] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-http-2, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-http-2] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-http-2, as: equal }] diff --git a/tests/gold_tests/logging/replay/log-debug-client-ip-http2.replay.yaml b/tests/gold_tests/logging/replay/log-debug-client-ip-http2.replay.yaml new file mode 100644 index 00000000000..5230a7ebeb9 --- /dev/null +++ b/tests/gold_tests/logging/replay/log-debug-client-ip-http2.replay.yaml @@ -0,0 +1,134 @@ +# 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. + +meta: + version: "1.0" + +autest: + description: 'Verify client-IP-filtered debug logging for HTTP/2' + + server: + name: 'server-log-debug-client-ip-http2' + + client: + name: 'client-log-debug-client-ip-http2' + + ats: + name: 'ts-log-debug-client-ip-http2' + process_config: + enable_cache: false + enable_tls: true + + records_config: + proxy.config.diags.debug.enabled: 2 + proxy.config.diags.debug.tags: 'http' + proxy.config.diags.debug.client_ip: '127.0.0.1' + + remap_config: + - from: 'https://debug-client-ip-http2.test/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + + log_validation: + traffic_out: + contains: + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-http2-1' + description: 'HTTP/2 transaction 1 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-http2-1' + description: 'HTTP/2 transaction 1 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-http2-1' + description: 'HTTP/2 transaction 1 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-http2-1' + description: 'HTTP/2 transaction 1 logs the proxy response headers.' + multiline: true + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-http2-2' + description: 'HTTP/2 transaction 2 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-http2-2' + description: 'HTTP/2 transaction 2 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-http2-2' + description: 'HTTP/2 transaction 2 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-http2-2' + description: 'HTTP/2 transaction 2 logs the proxy response headers.' + multiline: true + +sessions: +- protocol: + stack: http2 + tls: + sni: debug-client-ip-http2.test + transactions: + - client-request: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", debug-client-ip-http2.test] + - [":path", /debug-http2-1] + - [Content-Length, 0] + - [uuid, log-debug-http2-1] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-http2-1, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-http2-1] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-http2-1, as: equal }] + + - client-request: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", debug-client-ip-http2.test] + - [":path", /debug-http2-2] + - [Content-Length, 0] + - [uuid, log-debug-http2-2] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-http2-2, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-http2-2] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-http2-2, as: equal }] diff --git a/tests/gold_tests/logging/replay/log-debug-client-ip-http3.replay.yaml b/tests/gold_tests/logging/replay/log-debug-client-ip-http3.replay.yaml new file mode 100644 index 00000000000..d0599f91a07 --- /dev/null +++ b/tests/gold_tests/logging/replay/log-debug-client-ip-http3.replay.yaml @@ -0,0 +1,136 @@ +# 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. + +meta: + version: "1.0" + +autest: + description: 'Verify client-IP-filtered debug logging for HTTP/3' + + server: + name: 'server-log-debug-client-ip-http3' + + client: + name: 'client-log-debug-client-ip-http3' + + ats: + name: 'ts-log-debug-client-ip-http3' + process_config: + enable_cache: false + enable_tls: true + enable_quic: true + + records_config: + proxy.config.diags.debug.enabled: 2 + proxy.config.diags.debug.tags: 'http' + proxy.config.diags.debug.client_ip: '127.0.0.1' + proxy.config.quic.no_activity_timeout_in: 0 + + remap_config: + - from: 'https://debug-client-ip-http3.test/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + + log_validation: + traffic_out: + contains: + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-http3-1' + description: 'HTTP/3 transaction 1 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-http3-1' + description: 'HTTP/3 transaction 1 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-http3-1' + description: 'HTTP/3 transaction 1 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-http3-1' + description: 'HTTP/3 transaction 1 logs the proxy response headers.' + multiline: true + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-http3-2' + description: 'HTTP/3 transaction 2 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-http3-2' + description: 'HTTP/3 transaction 2 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-http3-2' + description: 'HTTP/3 transaction 2 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-http3-2' + description: 'HTTP/3 transaction 2 logs the proxy response headers.' + multiline: true + +sessions: +- protocol: + stack: http3 + tls: + sni: debug-client-ip-http3.test + transactions: + - client-request: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", debug-client-ip-http3.test] + - [":path", /debug-http3-1] + - [Content-Length, 0] + - [uuid, log-debug-http3-1] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-http3-1, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-http3-1] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-http3-1, as: equal }] + + - client-request: + headers: + fields: + - [":method", GET] + - [":scheme", https] + - [":authority", debug-client-ip-http3.test] + - [":path", /debug-http3-2] + - [Content-Length, 0] + - [uuid, log-debug-http3-2] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-http3-2, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-http3-2] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-http3-2, as: equal }] diff --git a/tests/gold_tests/logging/replay/log-debug-client-ip-https.replay.yaml b/tests/gold_tests/logging/replay/log-debug-client-ip-https.replay.yaml new file mode 100644 index 00000000000..79e9b5aa4b7 --- /dev/null +++ b/tests/gold_tests/logging/replay/log-debug-client-ip-https.replay.yaml @@ -0,0 +1,134 @@ +# 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. + +meta: + version: "1.0" + +autest: + description: 'Verify client-IP-filtered debug logging for HTTPS' + + server: + name: 'server-log-debug-client-ip-https' + + client: + name: 'client-log-debug-client-ip-https' + + ats: + name: 'ts-log-debug-client-ip-https' + process_config: + enable_cache: false + enable_tls: true + + records_config: + proxy.config.diags.debug.enabled: 2 + proxy.config.diags.debug.tags: 'http' + proxy.config.diags.debug.client_ip: '127.0.0.1' + + remap_config: + - from: 'https://debug-client-ip-https.test/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + + log_validation: + traffic_out: + contains: + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-https-1' + description: 'HTTPS transaction 1 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-https-1' + description: 'HTTPS transaction 1 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-https-1' + description: 'HTTPS transaction 1 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-https-1' + description: 'HTTPS transaction 1 logs the proxy response headers.' + multiline: true + - expression: '\+ Incoming Request \+(?:(?!\+ Proxy''s Request after hooks \+)[\s\S])*?uuid: log-debug-https-2' + description: 'HTTPS transaction 2 logs the incoming request headers.' + multiline: true + - expression: '\+ Proxy''s Request after hooks \+(?:(?!\+ Incoming O.S. Response \+)[\s\S])*?uuid: log-debug-https-2' + description: 'HTTPS transaction 2 logs the proxy request headers after hooks.' + multiline: true + - expression: '\+ Incoming O.S. Response \+(?:(?!\+ Proxy''s Response 2 \+)[\s\S])*?uuid: log-debug-https-2' + description: 'HTTPS transaction 2 logs the incoming origin response headers.' + multiline: true + - expression: '\+ Proxy''s Response 2 \+(?:(?!\+ Incoming Request \+)[\s\S])*?uuid: log-debug-https-2' + description: 'HTTPS transaction 2 logs the proxy response headers.' + multiline: true + +sessions: +- protocol: + stack: https + tls: + sni: debug-client-ip-https.test + transactions: + - client-request: + method: GET + url: /debug-https-1 + version: '1.1' + headers: + fields: + - [Host, debug-client-ip-https.test] + - [Content-Length, 0] + - [uuid, log-debug-https-1] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-https-1, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-https-1] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-https-1, as: equal }] + + - client-request: + method: GET + url: /debug-https-2 + version: '1.1' + headers: + fields: + - [Host, debug-client-ip-https.test] + - [Content-Length, 0] + - [uuid, log-debug-https-2] + + proxy-request: + headers: + fields: + - [uuid, { value: log-debug-https-2, as: equal }] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, 0] + - [uuid, log-debug-https-2] + + proxy-response: + status: 200 + headers: + fields: + - [uuid, { value: log-debug-https-2, as: equal }] From e1b6f1bca44c7fac7739d38e4ee49f3968d790eb Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Mon, 13 Jul 2026 10:14:18 -0600 Subject: [PATCH 03/15] autest: skip async handshake test when plugin is absent (#13372) The async_handshake test plugin is only built with OpenSSL (TS_USE_TLS_ASYNC). SkipUnless does not evaluate its conditions where it appears; it only registers them for the framework to check later, so the test file keeps executing and PrepareTestPlugin ran at load time and raised a ValueError when the plugin was missing, reported as a test exception instead of a skip. Guard the call on file existence so the test skips cleanly on non-OpenSSL builds. (cherry picked from commit caf9c87097d7231e74e536337c78eb4eba306ba5) --- tests/gold_tests/tls/tls_async_handshake.test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/gold_tests/tls/tls_async_handshake.test.py b/tests/gold_tests/tls/tls_async_handshake.test.py index 5ac4e289593..e0050010eee 100644 --- a/tests/gold_tests/tls/tls_async_handshake.test.py +++ b/tests/gold_tests/tls/tls_async_handshake.test.py @@ -33,7 +33,8 @@ ts = Test.MakeATSProcess("ts", enable_tls=True) server = Test.MakeOriginServer("server") -Test.PrepareTestPlugin(async_handshake, ts) +if os.path.isfile(async_handshake): + Test.PrepareTestPlugin(async_handshake, ts) server.addResponse( "sessionlog.json", { From f3b13ecfa9fa4e32f38d4c6240a9a11ebad0d363 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Mon, 13 Jul 2026 13:40:05 -0400 Subject: [PATCH 04/15] stale_response.test.py: address log wait flakiness (#13323) The stale_response log checks can run before every directive that they later assert has been written. Waiting for one marker with a sleep-based process leaves the final content checks exposed to ATS log flush timing when both stale directives are expected. This replaces the sleep-based watcher with explicit await runs for each directive being asserted. The test now waits for the matching stale-while-revalidate and stale-if-error entries before performing the final log content checks. Fixes: #13301 (cherry picked from commit e3dc7e76c3555be938ed0ec1a05b6952eff65cec) --- .../stale_response/stale_response.test.py | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/tests/gold_tests/pluginTest/stale_response/stale_response.test.py b/tests/gold_tests/pluginTest/stale_response/stale_response.test.py index 40d7b9841e9..1b77d368990 100644 --- a/tests/gold_tests/pluginTest/stale_response/stale_response.test.py +++ b/tests/gold_tests/pluginTest/stale_response/stale_response.test.py @@ -153,28 +153,22 @@ def verify_plugin_log(self) -> None: diagnostic, "Verify max-memory stale-if-error fallback is logged") return - tr = Test.AddTestRun("Verify stale_response plugin log") - name = f'log_waiter_{TestStaleResponse._ts_counter}' - log_waiter = tr.Processes.Process(name) - log_waiter.Command = 'sleep 30' + swr_log_pattern = "stale-while-revalidate:.*stale.jpeg" + sie_log_pattern = "stale-if-error:.*error.jpeg" + + def expect_log_entry(pattern: str, description: str) -> None: + tr = Test.AddAwaitFileContainsTestRun(f"Await {description}", self._ts.Disk.stale_responses_log.AbsPath, pattern) + tr.StillRunningBefore = self._ts + tr.StillRunningAfter = self._ts + self._ts.Disk.stale_responses_log.Content += Testers.ContainsExpression(pattern, f"Verify {description}") + if self._option_type == OptionType.FORCE_SWR: - log_waiter.Ready = When.FileContains(self._ts.Disk.stale_responses_log.Name, "stale-while-revalidate:") - self._ts.Disk.stale_responses_log.Content += Testers.ContainsExpression( - "stale-while-revalidate:.*stale.jpeg", "Verify stale-while-revalidate directive is logged") + expect_log_entry(swr_log_pattern, "stale-while-revalidate directive is logged") elif self._option_type == OptionType.FORCE_SIE: - log_waiter.Ready = When.FileContains(self._ts.Disk.stale_responses_log.Name, "stale-if-error:") - self._ts.Disk.stale_responses_log.Content += Testers.ContainsExpression( - "stale-if-error:.*error.jpeg", "Verify stale-if-error directive is logged") + expect_log_entry(sie_log_pattern, "stale-if-error directive is logged") else: - log_waiter.Ready = When.FileContains(self._ts.Disk.stale_responses_log.Name, "stale-if-error:") - self._ts.Disk.stale_responses_log.Content += Testers.ContainsExpression( - "stale-while-revalidate:.*stale.jpeg", "Verify stale-while-revalidate directive is logged") - self._ts.Disk.stale_responses_log.Content += Testers.ContainsExpression( - "stale-if-error:.*error.jpeg", "Verify stale-if-error directive is logged") - p = tr.Processes.Default - p.Command = 'echo "Waiting upon the stale response log."' - p.StartBefore(log_waiter) - p.StillRunningAfter = self._ts + expect_log_entry(swr_log_pattern, "stale-while-revalidate directive is logged") + expect_log_entry(sie_log_pattern, "stale-if-error directive is logged") TestStaleResponse(OptionType.NONE, is_global=True) From 67e38ce1be669bf192f22790a2f5c3ad4921fc38 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Fri, 17 Jul 2026 12:35:56 -0500 Subject: [PATCH 05/15] Handle webp_transform input robustly (#13390) Empty or mislabeled image responses need consistent handling by the webp_transform plugin. These inputs should pass through unchanged while valid images continue to be converted normally. This checks the expected image signature before conversion and preserves the original body and content type whenever conversion is bypassed. Replay coverage exercises empty, invalid, and valid image bodies. (cherry picked from commit f7f1d830387f017591b8ac3d194414e1743d6fd1) --- plugins/webp_transform/ImageTransform.cc | 99 ++++++--- .../webp_transform_invalid_input.replay.yaml | 196 ++++++++++++++++++ .../webp_transform_invalid_input.test.py | 25 +++ 3 files changed, 295 insertions(+), 25 deletions(-) create mode 100644 tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.replay.yaml create mode 100644 tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.test.py diff --git a/plugins/webp_transform/ImageTransform.cc b/plugins/webp_transform/ImageTransform.cc index b532bbd93f9..e4ebc7f4d7c 100644 --- a/plugins/webp_transform/ImageTransform.cc +++ b/plugins/webp_transform/ImageTransform.cc @@ -24,6 +24,7 @@ #include #include #include +#include #include "ts/ts.h" @@ -64,6 +65,26 @@ bool config_convert_to_jpeg = false; Stat stat_convert_to_webp; Stat stat_convert_to_jpeg; +bool +has_signature_for(std::string_view data, ImageEncoding encoding) +{ + constexpr std::string_view png_signature{"\x89PNG\r\n\x1a\n", 8}; + + switch (encoding) { + case ImageEncoding::webp: + return data.size() >= 12 && data.substr(0, 4) == "RIFF" && data.substr(8, 4) == "WEBP"; + case ImageEncoding::jpeg: + return data.size() >= 3 && static_cast(data[0]) == 0xff && static_cast(data[1]) == 0xd8 && + static_cast(data[2]) == 0xff; + case ImageEncoding::png: + return data.starts_with(png_signature); + case ImageEncoding::unknown: + return false; + } + + return false; +} + // Cap the buffered (encoded) response body. 16 MiB fits every // realistic image asset while keeping the worst case bounded. The default is // overridable with the max_buffer_size plugin argument (see TSPluginInit). @@ -159,8 +180,10 @@ content_type_for(ImageEncoding encoding) class ImageTransform : public TransformationPlugin { public: - ImageTransform(Transaction &transaction, ImageEncoding input_image_type, ImageEncoding transform_image_type) + ImageTransform(Transaction &transaction, std::string input_content_type, ImageEncoding input_image_type, + ImageEncoding transform_image_type) : TransformationPlugin(transaction, TransformationPlugin::RESPONSE_TRANSFORMATION), + _input_content_type(std::move(input_content_type)), _input_image_type(input_image_type), _transform_image_type(transform_image_type) { @@ -168,6 +191,24 @@ class ImageTransform : public TransformationPlugin TransformationPlugin::registerHook(HOOK_SEND_RESPONSE_HEADERS); } + void + handleReadResponseHeaders(Transaction &transaction) override + { + // Label the server response so both the cached transform and the client + // copy carry the target type. On a degraded transform (pass-through or + // decode error) the body is the original encoding but the label still says + // the target; handleSendResponseHeaders below corrects the client-facing + // copy using _input_content_type in that case. The cached label can still + // end up wrong on a degraded transform; fixing that without mislabeling + // the cache is tracked as a separate correctness issue. + if (const char *ctype = content_type_for(_transform_image_type); ctype != nullptr) { + transaction.getServerResponse().getHeaders()["Content-Type"] = ctype; + } + transaction.getServerResponse().getHeaders()["Vary"] = "Accept"; // separate cache entry per Accept + Dbg(webp_dbg_ctl, "url %s", transaction.getServerRequest().getUrl().getUrlString().c_str()); + transaction.resume(); + } + void handleSendResponseHeaders(Transaction &transaction) override { @@ -185,23 +226,16 @@ class ImageTransform : public TransformationPlugin // not happen; this is an empty error response, not an image. response.getHeaders().erase("Content-Type"); response.getHeaders().erase("Vary"); + transaction.resume(); + return; } - transaction.resume(); - } - void - handleReadResponseHeaders(Transaction &transaction) override - { - // Label the server response so both the cached transform and the client - // copy carry the target type. On a degraded transform (pass-through or - // decode error) the body is the original encoding but the label still says - // the target; correcting that without mislabeling the cache is tracked as a - // separate correctness issue, out of scope for this DoS fix. - if (const char *ctype = content_type_for(_transform_image_type); ctype != nullptr) { - transaction.getServerResponse().getHeaders()["Content-Type"] = ctype; + // Signature mismatch or decode failure reverted us to the original + // encoding (see pass_through()); relabel the client-facing response to + // match the body we actually sent. + if (_transform_image_type == _input_image_type) { + transaction.getClientResponse().getHeaders()["Content-Type"] = _input_content_type; } - transaction.getServerResponse().getHeaders()["Vary"] = "Accept"; // separate cache entry per Accept - Dbg(webp_dbg_ctl, "url %s", transaction.getServerRequest().getUrl().getUrlString().c_str()); transaction.resume(); } @@ -237,6 +271,15 @@ class ImageTransform : public TransformationPlugin setOutputComplete(); // no body produced; handleSendResponseHeaders turns this into a 502 return; } + + if (!has_signature_for(_img, _input_image_type)) { + TSError("[webp_transform] input body does not match its declared image encoding: %d, length: %zu", + static_cast(_input_image_type), _img.length()); + pass_through(_img); + setOutputComplete(); + return; + } + Blob input_blob(_img.data(), _img.length()); Image image; @@ -257,12 +300,10 @@ class ImageTransform : public TransformationPlugin produce(std::string_view(reinterpret_cast(output_blob.data()), output_blob.length())); } catch (const Magick::Warning &warning) { TSError("ImageMagick++ warning: %s", warning.what()); - produce(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); - _transform_image_type = _input_image_type; // Revert to original encoding on error + pass_through(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); } catch (const Magick::Error &error) { TSError("ImageMagick++ error: %s _image_type: %d input length: %zu", error.what(), (int)_transform_image_type, _img.length()); - produce(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); - _transform_image_type = _input_image_type; // Revert to original encoding on error + pass_through(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); } catch (const std::exception &e) { // ImageMagick++ can throw other exception types (e.g. // std::bad_alloc on huge or malformed inputs). Catch them so an @@ -271,13 +312,11 @@ class ImageTransform : public TransformationPlugin // large inputs) is distinguishable from a one-off decode hiccup. TSError("[webp_transform] std::exception during transform: %s _image_type: %d input length: %zu", e.what(), (int)_transform_image_type, _img.length()); - produce(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); - _transform_image_type = _input_image_type; + pass_through(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); } catch (...) { TSError("[webp_transform] unknown exception during transform _image_type: %d input length: %zu", (int)_transform_image_type, _img.length()); - produce(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); - _transform_image_type = _input_image_type; + pass_through(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); } setOutputComplete(); @@ -286,7 +325,17 @@ class ImageTransform : public TransformationPlugin ~ImageTransform() override = default; private: + void + pass_through(std::string_view data) + { + if (!data.empty()) { + produce(data); + } + _transform_image_type = _input_image_type; + } + std::string _img; + std::string _input_content_type; bool _refused = false; ImageEncoding _input_image_type; ImageEncoding _transform_image_type; @@ -385,13 +434,13 @@ class GlobalHookPlugin : public GlobalPlugin if (!content_length_usable) { TSHttpTxnServerRespNoStoreSet(static_cast(transaction.getAtsHandle()), 1); } - transaction.addPlugin(new ImageTransform(transaction, input_image_type, ImageEncoding::webp)); + transaction.addPlugin(new ImageTransform(transaction, ctype, input_image_type, ImageEncoding::webp)); } else if (webp_supported == false && transaction_convert_to_jpeg == true) { Dbg(webp_dbg_ctl, "Content type is webp. Converting to jpeg"); if (!content_length_usable) { TSHttpTxnServerRespNoStoreSet(static_cast(transaction.getAtsHandle()), 1); } - transaction.addPlugin(new ImageTransform(transaction, input_image_type, ImageEncoding::jpeg)); + transaction.addPlugin(new ImageTransform(transaction, ctype, input_image_type, ImageEncoding::jpeg)); } else { Dbg(webp_dbg_ctl, "Nothing to convert"); } diff --git a/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.replay.yaml b/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.replay.yaml new file mode 100644 index 00000000000..ccc8cc5de24 --- /dev/null +++ b/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.replay.yaml @@ -0,0 +1,196 @@ +# 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. + +meta: + version: "1.0" + +autest: + description: 'Verify webp_transform safely passes through invalid image bodies' + + server: + name: 'webp-transform-server' + + client: + name: 'webp-transform-client' + + ats: + name: 'webp-transform-ts' + process_config: + enable_cache: false + disable_log_checks: true + + plugin_config: + - 'webp_transform.so convert_to_jpeg,convert_to_webp' + + records_config: + proxy.config.diags.debug.enabled: 1 + proxy.config.diags.debug.tags: 'webp_transform' + + remap_config: + - from: 'http://www.example.com/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + + log_validation: + diags_log: + contains: + - expression: 'input body does not match its declared image encoding' + description: 'Verify invalid input is rejected before conversion' + excludes: + - expression: 'zero-length blob not permitted|no decode delegate for this image format' + description: 'Verify ImageMagick never receives the invalid input' + +sessions: + - transactions: + - client-request: + method: GET + version: '1.1' + url: /empty.webp + headers: + fields: + - [Host, www.example.com] + - [Accept, 'image/jpeg'] + - [uuid, empty-webp] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, image/webp] + - [Content-Length, 0] + content: + size: 0 + + proxy-response: + status: 200 + headers: + fields: + - [Content-Type, {value: image/webp, as: equal}] + content: + size: 0 + + - client-request: + method: GET + version: '1.1' + url: /invalid.webp + headers: + fields: + - [Host, www.example.com] + - [Accept, 'image/jpeg'] + - [uuid, invalid-webp] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, image/webp] + - [Content-Length, 1] + content: + data: x + + proxy-response: + status: 200 + headers: + fields: + - [Content-Type, {value: image/webp, as: equal}] + content: + verify: {value: x, as: equal} + + - client-request: + method: GET + version: '1.1' + url: /invalid.jpg + headers: + fields: + - [Host, www.example.com] + - [Accept, 'image/webp'] + - [uuid, invalid-jpeg] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, image/jpeg] + - [Content-Length, 8] + content: + data: not-jpeg + + proxy-response: + status: 200 + headers: + fields: + - [Content-Type, {value: image/jpeg, as: equal}] + content: + verify: {value: not-jpeg, as: equal} + + - client-request: + method: GET + version: '1.1' + url: /valid.webp + headers: + fields: + - [Host, www.example.com] + - [Accept, 'image/jpeg'] + - [uuid, valid-webp] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, image/webp] + - [Content-Length, 68] + content: + encoding: uri + data: "%52%49%46%46%3c%00%00%00%57%45%42%50%56%50%38%20%30%00%00%00%d0%01%00%9d\ + %01%2a%01%00%01%00%02%00%34%25%a0%02%74%ba%01%f8%00%03%b0%00%fe%f0%c4%0b\ + %ff%20%b9%61%75%c8%d7%ff%20%3f%e4%07%fc%80%ff%f8%f2%00%00%00" + + proxy-response: + status: 200 + headers: + fields: + - [Content-Type, {value: image/jpeg, as: equal}] + - [Vary, {value: Accept, as: equal}] + content: + verify: {value: JFIF, as: contains} + + - client-request: + method: GET + version: '1.1' + url: /health + headers: + fields: + - [Host, www.example.com] + - [uuid, health] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, text/plain] + - [Content-Length, 2] + content: + data: OK + + proxy-response: + status: 200 + content: + verify: {value: OK, as: equal} diff --git a/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.test.py b/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.test.py new file mode 100644 index 00000000000..e5c4961b909 --- /dev/null +++ b/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.test.py @@ -0,0 +1,25 @@ +''' +Verify webp_transform rejects invalid image input before invoking ImageMagick. +''' +# 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 = 'Verify webp_transform safely passes through invalid image bodies' + +Test.SkipUnless(Condition.PluginExists('webp_transform.so')) + +Test.ATSReplayTest(replay_file='webp_transform_invalid_input.replay.yaml') From a0a7908c826ed3eedf9192bd1cf2fc61ee541408 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Mon, 20 Jul 2026 15:23:54 -0500 Subject: [PATCH 06/15] Preserve client port in transaction logs (#13379) Client source ports could be logged as zero when the live inbound connection was cleared before transaction log marshalling. This obscured the effective client endpoint in access logs. This reads the port from the effective client endpoint retained by HttpSM, keeping it available after connection cleanup. (cherry picked from commit a92d8babb71c72e1e55f706c795a540be67c496b) Backport note: 10.1.x predates the LogData de-virtualization (#13123), so src/proxy/logging/TransactionLogData.cc does not exist on this branch. The equivalent fix is applied to LogAccess::marshal_client_host_port() in src/proxy/logging/LogAccess.cc, which is the same accessor on this branch. This also matches marshal_client_host_ip(), which already reads t_state.effective_client_addr, so the logged client IP and port now come from the same endpoint. --- src/proxy/logging/LogAccess.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/proxy/logging/LogAccess.cc b/src/proxy/logging/LogAccess.cc index 667f8eaf497..b40b77d5904 100644 --- a/src/proxy/logging/LogAccess.cc +++ b/src/proxy/logging/LogAccess.cc @@ -1629,11 +1629,7 @@ int LogAccess::marshal_client_host_port(char *buf) { if (m_http_sm) { - auto txn = m_http_sm->get_ua_txn(); - if (txn) { - uint16_t port = txn->get_client_port(); - marshal_int(buf, port); - } + marshal_int(buf, m_http_sm->t_state.effective_client_addr.host_order_port()); } return INK_MIN_ALIGN; } From e9b666a691de003bea097a82049e981f4c2449cb Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Wed, 29 Jul 2026 11:53:32 -0500 Subject: [PATCH 07/15] Fix Fedora OTEL and WAMR builds (#13423) Fedora CI could silently omit the OTEL and WAMR plugins, while BoringSSL builds mixed system OpenSSL headers and libraries. This made the updated dependency image appear usable without proving either plugin could load. This makes the Fedora presets require both plugins and selects the matching curl and TLS roots for system OpenSSL and BoringSSL builds. This also distinguishes BoringSSL from OpenSSL 3 during configuration and gives the WASM targets explicit OpenSSL dependencies so both TLS variants build and load consistently. (cherry picked from commit fa297c39988da44f68ee627d5ca3d325645247db) --- CMakeLists.txt | 5 ++++- CMakePresets.json | 12 ++++++++---- plugins/experimental/wasm/CMakeLists.txt | 2 +- plugins/experimental/wasm/lib/CMakeLists.txt | 1 + 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8a829a8d9ac..b517fcf7105 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -304,7 +304,10 @@ else() endif() check_openssl_is_quictls(SSLLIB_IS_QUICTLS "${OPENSSL_INCLUDE_DIR}") -if(OPENSSL_VERSION VERSION_GREATER_EQUAL "3.0.0") +if(NOT SSLLIB_IS_BORINGSSL + AND NOT SSLLIB_IS_AWSLC + AND OPENSSL_VERSION VERSION_GREATER_EQUAL "3.0.0" +) set(SSLLIB_IS_OPENSSL3 TRUE) add_compile_definitions(OPENSSL_API_COMPAT=10002 OPENSSL_IS_OPENSSL3) endif() diff --git a/CMakePresets.json b/CMakePresets.json index 2bf61df3969..3fc994c0b9e 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -176,10 +176,12 @@ "inherits": ["ci"], "cacheVariables": { "ENABLE_PROBES": "ON", - "OPENSSL_ROOT_DIR": "/opt/openssl-quic", + "OPENSSL_ROOT_DIR": "/usr", "opentelemetry_ROOT": "/opt", - "CURL_ROOT": "/opt", + "CURL_ROOT": "/usr", "wamr_ROOT": "/opt", + "ENABLE_OTEL_TRACER": "ON", + "ENABLE_WASM_WAMR": "ON", "ENABLE_CRIPTS": "ON" } }, @@ -201,15 +203,17 @@ "OPENSSL_ROOT_DIR": "/opt/h3-tools-boringssl/boringssl", "quiche_ROOT": "/opt/h3-tools-boringssl/quiche", "opentelemetry_ROOT": "/opt", - "CURL_ROOT": "/opt", + "CURL_ROOT": "/opt/h3-tools-boringssl", "wamr_ROOT": "/opt", + "ENABLE_OTEL_TRACER": "ON", + "ENABLE_WASM_WAMR": "ON", "CMAKE_INSTALL_PREFIX": "/tmp/ats-quiche", "ENABLE_QUICHE": "ON" } }, { "name": "ci-fedora-autest", - "displayName": "CI Fedora Quiche Autest", + "displayName": "CI Fedora Autest", "description": "CI Pipeline config for Fedora Linux (autest build)", "inherits": ["ci-fedora", "autest"] }, diff --git a/plugins/experimental/wasm/CMakeLists.txt b/plugins/experimental/wasm/CMakeLists.txt index 0cccbc2082c..c10011e5fb4 100644 --- a/plugins/experimental/wasm/CMakeLists.txt +++ b/plugins/experimental/wasm/CMakeLists.txt @@ -29,7 +29,7 @@ if(wasmedge_FOUND) list(APPEND WASM_RUNTIME wasmedge::wasmedge) endif() -target_link_libraries(wasm PRIVATE ${WASM_RUNTIME}) +target_link_libraries(wasm PRIVATE OpenSSL::SSL ${WASM_RUNTIME}) if(wamr_FOUND) target_compile_options(wasm PRIVATE -DWAMR) diff --git a/plugins/experimental/wasm/lib/CMakeLists.txt b/plugins/experimental/wasm/lib/CMakeLists.txt index f838461d7bf..1a07cd3a882 100644 --- a/plugins/experimental/wasm/lib/CMakeLists.txt +++ b/plugins/experimental/wasm/lib/CMakeLists.txt @@ -38,6 +38,7 @@ endif() add_library(wasmlib STATIC ${CC_FILES}) target_compile_options(wasmlib PUBLIC -Wno-unused-parameter) +target_link_libraries(wasmlib PRIVATE OpenSSL::Crypto) if(wamr_FOUND) target_compile_options(wasmlib PRIVATE -Wno-missing-field-initializers) target_link_libraries(wasmlib PUBLIC wamr::wamr) From b9bd7b947e113e36cdd4ee9dc72fc11ee7a10659 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Fri, 31 Jul 2026 17:06:50 -0500 Subject: [PATCH 08/15] healthchecks: reference count status file data (#13432) Replaced file data was retired onto a freelist and freed after a timeout, but its deadline came from a timestamp taken before a blocking inotify read. That stale deadline let transactions retain references to data that was already freed. Files exactly 16 KiB long were also reported as empty because a final zero-byte read overwrote the saved length. This holds each immutable snapshot in an atomic shared_ptr. Every transaction pins its snapshot, so data lives exactly as long as it is referenced, without a freelist or request-path mutex. This also preserves the last successful file-read length and adds AuTest coverage for concurrent replacement and the 16 KiB boundary. Fixes: #8735 (cherry picked from commit 16bd59ab4ca21f5d0f1a15cfd9d5be22f106d5f0) --- plugins/healthchecks/healthchecks.cc | 201 +++++++++--------- .../healthchecks/healthchecks.test.py | 54 +++++ 2 files changed, 150 insertions(+), 105 deletions(-) diff --git a/plugins/healthchecks/healthchecks.cc b/plugins/healthchecks/healthchecks.cc index f31a43d0cc1..c5da85da9da 100644 --- a/plugins/healthchecks/healthchecks.cc +++ b/plugins/healthchecks/healthchecks.cc @@ -28,6 +28,8 @@ limitations under the License. #include #include #include +#include +#include /* ToDo: Linux specific */ #include @@ -42,9 +44,8 @@ static const char SEPARATORS[] = " \t\n"; static DbgCtl dbg_ctl{PLUGIN_NAME}; -#define MAX_PATH_LEN 4096 -#define MAX_BODY_LEN 16384 -#define FREELIST_TIMEOUT 300 +#define MAX_PATH_LEN 4096 +#define MAX_BODY_LEN 16384 /* Directories that we are watching for inotify IN_CREATE events. */ typedef struct HCDirEntry_t { @@ -53,66 +54,100 @@ typedef struct HCDirEntry_t { struct HCDirEntry_t *_next; /* Linked list */ } HCDirEntry; -/* Information about a status file. This is never modified (only replaced, see HCFileInfo_t) */ -typedef struct HCFileData_t { - int exists; /* Does this file exist */ - char body[MAX_BODY_LEN]; /* Body from fname. Empty string means file is missing */ - int b_len; /* Length of data */ - time_t remove; /* Used for deciding when the old object can be permanently removed */ - struct HCFileData_t *_next; /* Only used when these guys end up on the freelist */ -} HCFileData; - -/* The only thing that should change in this struct is data, atomically swapping ptrs */ -typedef struct HCFileInfo_t { - char fname[MAX_PATH_LEN]; /* Filename */ - char *basename; /* The "basename" of the file */ - unsigned basename_len = 0; /* The length of the basename */ - char path[PATH_NAME_MAX]; /* URL path for this HC */ - int p_len; /* Length of path */ - const char *ok; /* Header for an OK result */ - int o_len; /* Length of OK header */ - const char *miss; /* Header for miss results */ - int m_len; /* Length of miss header */ - std::atomic data; /* Holds the current data for this health check file */ - int wd; /* Watch descriptor */ - HCDirEntry *dir; /* Reference to the directory this file resides in */ - struct HCFileInfo_t *_next; /* Linked list */ -} HCFileInfo; +/* Information about a status file. This is never modified (only replaced, see HCFileInfo) */ +struct HCFileData { + int exists = 0; /* Does this file exist */ + int b_len = 0; /* Length of data */ + char body[MAX_BODY_LEN] = {}; /* Body from fname. Empty string means file is missing */ +}; + +using HCFileDataPtr = std::shared_ptr; + +/* The only thing that should change in this struct is data, which is replaced (never modified) by + the inotify thread. Readers take a reference to the current data via get_data(), which keeps + that snapshot alive for as long as the transaction needs it. */ +struct HCFileInfo { + char fname[MAX_PATH_LEN] = {}; /* Filename */ + char *basename = nullptr; /* The "basename" of the file */ + unsigned basename_len = 0; /* The length of the basename */ + char path[PATH_NAME_MAX] = {}; /* URL path for this HC */ + int p_len = 0; /* Length of path */ + const char *ok = nullptr; /* Header for an OK result */ + int o_len = 0; /* Length of OK header */ + const char *miss = nullptr; /* Header for miss results */ + int m_len = 0; /* Length of miss header */ + int wd = 0; /* Watch descriptor */ + HCDirEntry *dir = nullptr; /* Reference to the directory this file resides in */ + HCFileInfo *_next = nullptr; /* Linked list */ + + /* Take a reference to the current data for this health check file. */ + HCFileDataPtr + get_data() + { +#if defined(__cpp_lib_atomic_shared_ptr) && __cpp_lib_atomic_shared_ptr >= 201711L + return _data.load(std::memory_order_acquire); +#else + return std::atomic_load_explicit(&_data, std::memory_order_acquire); +#endif + } + + /* Replace the current data for this health check file. Snapshots handed out by get_data() stay + valid until their last reference is dropped. */ + void + set_data(HCFileDataPtr data) + { +#if defined(__cpp_lib_atomic_shared_ptr) && __cpp_lib_atomic_shared_ptr >= 201711L + _data.store(std::move(data), std::memory_order_release); +#else + std::atomic_store_explicit(&_data, std::move(data), std::memory_order_release); +#endif + } + +private: +#if defined(__cpp_lib_atomic_shared_ptr) && __cpp_lib_atomic_shared_ptr >= 201711L + std::atomic _data; /* Holds the current data for this health check file */ +#else + HCFileDataPtr _data; /* Holds the current data for this health check file */ +#endif +}; /* Global configuration */ HCFileInfo *g_config; /* State used for the intercept plugin. ToDo: Can this be improved ? */ -typedef struct HCState_t { - TSVConn net_vc; - TSVIO read_vio; - TSVIO write_vio; +struct HCState { + TSVConn net_vc = nullptr; + TSVIO read_vio = nullptr; + TSVIO write_vio = nullptr; - TSIOBuffer req_buffer; - TSIOBuffer resp_buffer; - TSIOBufferReader resp_reader; + TSIOBuffer req_buffer = nullptr; + TSIOBuffer resp_buffer = nullptr; + TSIOBufferReader resp_reader = nullptr; - int output_bytes; + int output_bytes = 0; - /* We actually need both here, so that our lock free switches works safely */ - HCFileInfo *info; - HCFileData *data; -} HCState; + /* We hold a reference to the data so that it cannot be replaced from under us mid transaction */ + HCFileInfo *info = nullptr; + HCFileDataPtr data; +}; /* Read / check the status files */ -static void -reload_status_file(HCFileInfo *info, HCFileData *data) +static HCFileDataPtr +load_status_file(HCFileInfo *info) { + auto data = std::make_shared(); FILE *fd; - memset(data, 0, sizeof(HCFileData)); if (nullptr != (fd = fopen(info->fname, "r"))) { data->exists = 1; - do { - data->b_len = fread(data->body, 1, MAX_BODY_LEN, fd); - } while (!feof(fd)); /* Only save the last 16KB of the file ... */ + size_t bytes_read; + while ((bytes_read = fread(data->body, 1, MAX_BODY_LEN, fd)) > 0) { + data->b_len = static_cast(bytes_read); + } fclose(fd); } + + return data; } /* Find a HCDirEntry from the linked list */ @@ -198,49 +233,16 @@ event_matches_config(struct inotify_event *event, HCFileInfo *finfo) static void * hc_thread(void *data ATS_UNUSED) { - int inotify_fd = inotify_init(); - HCFileData *fl_head = nullptr; - char buffer[INOTIFY_BUFLEN]; - struct timeval last_free, now; - - gettimeofday(&last_free, nullptr); + int inotify_fd = inotify_init(); + char buffer[INOTIFY_BUFLEN]; /* Setup watchers for the directories, these are a one time setup */ setup_watchers(inotify_fd); // This is a leak, but since we enter an infinite loop this is ok? while (true) { - HCFileData *fdata = fl_head, *fdata_prev = nullptr; - - gettimeofday(&now, nullptr); /* Read the inotify events, blocking until we get something */ int len = read(inotify_fd, buffer, INOTIFY_BUFLEN); - /* The fl_head is a linked list of previously released data entries. They - are ordered "by time", so once we find one that is scheduled for deletion, - we can also delete all entries after it in the linked list. */ - while (fdata) { - if (now.tv_sec > fdata->remove) { - /* Now drop off the "tail" from the freelist */ - if (fdata_prev) { - fdata_prev->_next = nullptr; - } else { - fl_head = nullptr; - } - - /* free() everything in the "tail" */ - do { - HCFileData *next = fdata->_next; - - Dbg(dbg_ctl, "Cleaning up entry from freelist"); - TSfree(fdata); - fdata = next; - } while (fdata); - break; /* Stop the loop, there's nothing else left to examine */ - } - fdata_prev = fdata; - fdata = fdata->_next; - } - if (len >= 0) { int i = 0; @@ -253,9 +255,6 @@ hc_thread(void *data ATS_UNUSED) finfo = finfo->_next; } if (finfo) { - auto *new_data = TSRalloc(); - HCFileData *old_data; - if (event->mask & (IN_CLOSE_WRITE | IN_ATTRIB)) { Dbg(dbg_ctl, "Modify file event (%d) on %s", event->mask, finfo->fname); } else if (event->mask & (IN_CREATE | IN_MOVED_TO)) { @@ -267,16 +266,12 @@ hc_thread(void *data ATS_UNUSED) } else { Dbg(dbg_ctl, "Unhandled event (%d) on %s", event->mask, finfo->fname); } - /* Load the new data and then swap this atomically */ - memset(new_data, 0, sizeof(HCFileData)); - reload_status_file(finfo, new_data); - Dbg(dbg_ctl, "Reloaded %s, len == %d, exists == %d", finfo->fname, new_data->b_len, new_data->exists); - old_data = finfo->data.exchange(new_data); + /* Load the new data and then publish it. The previous data is released once the last + transaction referencing it completes. */ + auto new_data = load_status_file(finfo); - /* Add the old data to the head of the freelist */ - old_data->remove = now.tv_sec + FREELIST_TIMEOUT; - old_data->_next = fl_head; - fl_head = old_data; + Dbg(dbg_ctl, "Reloaded %s, len == %d, exists == %d", finfo->fname, new_data->b_len, new_data->exists); + finfo->set_data(std::move(new_data)); } /* coverity[ -tainted_data_return] */ i += sizeof(struct inotify_event) + event->len; @@ -342,10 +337,9 @@ parse_configs(const char *fname) char *str, *save; char *ok = nullptr, *miss = nullptr, *mime = nullptr; - finfo = TSRalloc(); - memset(static_cast(finfo), 0, sizeof(HCFileInfo)); - if (fgets(buf, sizeof(buf) - 1, fd)) { + finfo = new HCFileInfo(); + str = strtok_r(buf, SEPARATORS, &save); int state = 0; while (nullptr != str) { @@ -388,9 +382,7 @@ parse_configs(const char *fname) Dbg(dbg_ctl, "Parsed: %s %s %s %s %s", finfo->path, finfo->fname, mime, ok, miss); finfo->ok = gen_header(ok, mime, &finfo->o_len); finfo->miss = gen_header(miss, mime, &finfo->m_len); - finfo->data = TSRalloc(); - memset(finfo->data, 0, sizeof(HCFileData)); - reload_status_file(finfo, finfo->data); + finfo->set_data(load_status_file(finfo)); /* Add it the linked list */ Dbg(dbg_ctl, "Adding path=%s to linked list", finfo->path); @@ -401,7 +393,7 @@ parse_configs(const char *fname) } prev_finfo = finfo; } else { - TSfree(finfo); + delete finfo; } } } @@ -434,7 +426,7 @@ cleanup(TSCont contp, HCState *my_state) my_state->net_vc = nullptr; } - TSfree(my_state); + delete my_state; TSContDestroy(contp); } @@ -567,11 +559,10 @@ health_check_origin(TSCont contp ATS_UNUSED, TSEvent event ATS_UNUSED, void *eda TSHttpTxnCntlSet(txnp, TS_HTTP_CNTL_SKIP_REMAPPING, true); /* not strictly necessary, but speed is everything these days */ /* This is us -- register our intercept */ - icontp = TSContCreate(hc_intercept, TSMutexCreate()); - my_state = TSRalloc(); - memset(my_state, 0, sizeof(*my_state)); + icontp = TSContCreate(hc_intercept, TSMutexCreate()); + my_state = new HCState(); my_state->info = info; - my_state->data = info->data; + my_state->data = info->get_data(); TSContDataSet(icontp, my_state); TSHttpTxnIntercept(icontp, txnp); } diff --git a/tests/gold_tests/pluginTest/healthchecks/healthchecks.test.py b/tests/gold_tests/pluginTest/healthchecks/healthchecks.test.py index fb36c1d9548..d6348f88596 100644 --- a/tests/gold_tests/pluginTest/healthchecks/healthchecks.test.py +++ b/tests/gold_tests/pluginTest/healthchecks/healthchecks.test.py @@ -44,6 +44,9 @@ def __init__(self) -> None: self._expect_acme_ssl_404() self._re_add_acme_ssl() self._expect_positive_healthchecks() + self._expect_full_buffer_acme_body() + self._rewrite_acme_while_serving() + self._expect_rewritten_acme_body() def _configure_global_ts(self) -> None: '''Configure a global Traffic Server instance for the test runs. @@ -143,6 +146,57 @@ def _re_add_acme_ssl(self) -> None: p.Command = 'sleep 1' p.ReturnCode = 0 + def _rewrite_acme_while_serving(self) -> None: + '''Rewrite the acme file repeatedly while healthcheck requests are in flight. + + The plugin replaces the health check file data underneath transactions which may still be + reading the previous data. This drives that replacement so that an ASan enabled build + catches the old data being released while it is still referenced. + :return: None + ''' + tr = Test.AddTestRun('Rewrite acme while healthchecks are being served') + acme_file = os.path.join(Test.RunDirectory, 'acme') + url = f'http://127.0.0.1:{self._ts.Variables.port}/acme' + + # Note that autest runs the command through string.Template, so shell variables cannot be + # used here. The loop is therefore unrolled. + commands = [] + for iteration in range(10): + commands.append(f'echo "{CONTENT} {iteration}" > {acme_file};') + commands.append('{curl} -s -o /dev/null ' + url + ' &') + commands.append('{curl} -s -o /dev/null ' + url + ' &') + commands.append('wait') + + tr.MakeCurlCommandMulti(' '.join(commands), ts=self._ts) + tr.Processes.Default.ReturnCode = 0 + + def _expect_full_buffer_acme_body(self) -> None: + '''Verify that a MAX_BODY_LEN-sized file is not reported as empty. + :return: None + ''' + tr = Test.AddTestRun('Expect a full-sized healthcheck response body') + acme_file = os.path.join(Test.RunDirectory, 'acme') + url = f'http://127.0.0.1:{self._ts.Variables.port}/acme' + command = (f'dd if=/dev/zero of={acme_file} bs=16384 count=1 2>/dev/null && sleep 1 && ' + '{curl} -s ' + url + ' | wc -c') + tr.MakeCurlCommandMulti(command, ts=self._ts) + p = tr.Processes.Default + p.ReturnCode = 0 + p.Streams.All += Testers.ContainsExpression('16384', 'Verify the response contains 16 KiB') + + def _expect_rewritten_acme_body(self) -> None: + '''Verify that the most recently written acme content is what gets served. + :return: None + ''' + tr = Test.AddTestRun('Expect the last written acme content in the response body') + acme_file = os.path.join(Test.RunDirectory, 'acme') + url = f'http://127.0.0.1:{self._ts.Variables.port}/acme' + command = f'echo "{CONTENT} final" > {acme_file} && sleep 1 && ' + '{curl} -v ' + url + tr.MakeCurlCommandMulti(command, ts=self._ts) + p = tr.Processes.Default + p.ReturnCode = 0 + p.Streams.All += Testers.ContainsExpression('HTTP/1.1 200', 'Verify 200 response for /acme') + p.Streams.All += Testers.ContainsExpression(f'{CONTENT} final', 'Verify the reloaded acme content is served') + # Instantiate the test TestFileChangeBehavior() From 59d10d5bdb75a2094c7c0ea2b2d86284d97882db Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Tue, 4 Aug 2026 14:32:11 -0500 Subject: [PATCH 09/15] Cache empty chunked responses (#13410) Empty chunked responses can complete without starting a cache write VIO, or can start with an unknown length that is later finalized at zero. ATS treats both as empty unwritten entries and sends later requests back to origin. AuTests can also cross a log-rolling boundary before checking custom logs, producing an unrelated intermittent failure. This patch starts zero-byte cache writes and recognizes successfully closed write VIOs whose final length is zero. This preserves the empty-document state while keeping header-only cache updates distinct. This also disables log rolling for stale-response log assertions and covers negative and successful empty responses. Fixes: #11313 (cherry picked from commit d0119c46475e4d846e86b5f543baecaa9e7a67f9) --- src/iocore/cache/CacheVC.cc | 19 ++++++- src/iocore/cache/P_CacheInternal.h | 1 + src/proxy/http/HttpTunnel.cc | 14 +++-- ...ive-caching-300-second-timeout.replay.yaml | 51 ++++++++++++++++++- .../stale_response/stale_response.test.py | 2 + 5 files changed, 82 insertions(+), 5 deletions(-) diff --git a/src/iocore/cache/CacheVC.cc b/src/iocore/cache/CacheVC.cc index 11fa8699732..7707d9d8bb2 100644 --- a/src/iocore/cache/CacheVC.cc +++ b/src/iocore/cache/CacheVC.cc @@ -215,6 +215,14 @@ CacheVC::do_io_write(Continuation *c, int64_t nbytes, IOBufferReader *abuf, bool #ifdef DEBUG ink_assert(!c || c->mutex->thread_holding); #endif + if (nbytes == 0) { + // A zero-byte write represents an empty document, while closing without a + // write represents a header-only update. + f.allow_empty_doc = 1; + if (alternate.valid()) { + alternate.object_size_set(0); + } + } if (c && !trigger && !recursive) { trigger = c->mutex->thread_holding->schedule_imm_local(this); } @@ -225,6 +233,14 @@ void CacheVC::do_io_close(int alerrno) { ink_assert(mutex->thread_holding == this_ethread()); + if (alerrno == -1 && vio.op == VIO::WRITE && vio.get_reader() != nullptr && vio.nbytes == 0) { + // The write may have started with an unknown length and been finalized + // at zero after the response framing was parsed. + f.allow_empty_doc = 1; + if (alternate.valid()) { + alternate.object_size_set(0); + } + } int previous_closed = closed; closed = (alerrno == -1) ? 1 : -1; // Stupid default arguments DDbg(dbg_ctl_cache_close, "do_io_close %p %d %d", this, alerrno, closed); @@ -1042,7 +1058,8 @@ CacheVC::set_http_info(CacheHTTPInfo *ainfo) } MIMEField *field = ainfo->m_alt->m_response_hdr.field_find(MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH); - if ((field && !field->value_get_int64()) || ainfo->m_alt->m_response_hdr.status_get() == HTTP_STATUS_NO_CONTENT) { + if ((field && !field->value_get_int64()) || ainfo->m_alt->m_response_hdr.status_get() == HTTP_STATUS_NO_CONTENT || + (f.allow_empty_doc && vio.nbytes == 0)) { f.allow_empty_doc = 1; // Set the object size here to zero in case this is a cache replace where the new object // length is zero but the old object was not. diff --git a/src/iocore/cache/P_CacheInternal.h b/src/iocore/cache/P_CacheInternal.h index 34faafe64e2..d90cfad4bb5 100644 --- a/src/iocore/cache/P_CacheInternal.h +++ b/src/iocore/cache/P_CacheInternal.h @@ -300,6 +300,7 @@ CacheVC::die() { if (vio.op == VIO::WRITE) { if (f.update && total_len) { + ink_assert(alternate.valid()); alternate.object_key_set(earliest_key); } if (!is_io_in_progress()) { diff --git a/src/proxy/http/HttpTunnel.cc b/src/proxy/http/HttpTunnel.cc index d859791cbdd..088255743ad 100644 --- a/src/proxy/http/HttpTunnel.cc +++ b/src/proxy/http/HttpTunnel.cc @@ -1205,9 +1205,17 @@ HttpTunnel::producer_run(HttpTunnelProducer *p) } if (c_write == 0) { - // Nothing to do, call back the cleanup handlers - c->write_vio = nullptr; - consumer_handler(VC_EVENT_WRITE_COMPLETE, c); + // Cache writes need a VIO even when the body is empty so that closing the + // cache VC commits the response metadata instead of aborting the write. + if (c->vc_type == HT_CACHE_WRITE) { + c->write_vio = c->vc->do_io_write(this, 0, c->buffer_reader); + if (c->write_vio == nullptr) { + consumer_handler(VC_EVENT_ERROR, c); + } + } else { + c->write_vio = nullptr; + consumer_handler(VC_EVENT_WRITE_COMPLETE, c); + } } else { // In the client half close case, all the data that will be sent // from the client is already in the buffer. Go ahead and set diff --git a/tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml b/tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml index 7cb667b5ede..8083dbc60e7 100644 --- a/tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml +++ b/tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml @@ -40,6 +40,18 @@ meta: # transaction. delay: 100ms + - request_200_item: &request_200_item + client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/200_empty_chunked + headers: + fields: + - [ Host, example.com ] + + delay: 100ms + sessions: - transactions: @@ -47,13 +59,16 @@ sessions: <<: *request_404_item # Populate the cache with a 404 response. + # Verify that an empty chunked response is cached (issue #11313). server-response: status: 404 reason: "Not Found" headers: fields: - - [ Content-Length, 32 ] + - [ Transfer-Encoding, chunked ] - [ Cache-Control, max-age=300 ] + content: + size: 0 proxy-response: status: 404 @@ -74,3 +89,37 @@ sessions: # Expect the cached 404 response. proxy-response: status: 404 + + - all: { headers: { fields: [[ uuid, 23 ]]}} + <<: *request_200_item + + # The empty chunked-body behavior is not specific to negative responses. + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Transfer-Encoding, chunked ] + - [ Cache-Control, max-age=300 ] + content: + size: 0 + + proxy-response: + status: 200 + + - all: { headers: { fields: [[ uuid, 24 ]]}} + <<: *request_200_item + + proxy-request: + expect: absent + + server-response: + status: 502 + reason: Bad Gateway + headers: + fields: + - [ Content-Length, 0 ] + + # Expect the cached 200 response. + proxy-response: + status: 200 diff --git a/tests/gold_tests/pluginTest/stale_response/stale_response.test.py b/tests/gold_tests/pluginTest/stale_response/stale_response.test.py index 1b77d368990..b294715dd1b 100644 --- a/tests/gold_tests/pluginTest/stale_response/stale_response.test.py +++ b/tests/gold_tests/pluginTest/stale_response/stale_response.test.py @@ -131,6 +131,8 @@ def setupTS(self) -> None: "proxy.config.http.server_session_sharing.pool": "global", # Turn off negative revalidating so that we can test stale-if-error. "proxy.config.http.negative_revalidating_enabled": 0, + # Keep the active log filename available for the final content check if the test spans UTC midnight. + "proxy.config.log.rolling_enabled": 0, }) ts.Disk.remap_config.AddLine(f"map / http://127.0.0.1:{self._server.Variables.http_port}/ {remap_plugin_config}") From b1d9e581420cdf55101a20610d30ce7c9bb3bd9e Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Thu, 3 Sep 2026 20:43:03 -0500 Subject: [PATCH 10/15] Proxy Verifier v3.2.0 (#13642) This includes a body delay feature Masaori worked on. See: https://github.com/yahoo/proxy-verifier#content-delay-specification (cherry picked from commit 0265a523cb1f83a5a1af58e7eebee90961e176a8) --- tests/proxy-verifier-checksum.txt | 2 +- tests/proxy-verifier-version.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/proxy-verifier-checksum.txt b/tests/proxy-verifier-checksum.txt index 9c2ec8528be..90b31901ff1 100644 --- a/tests/proxy-verifier-checksum.txt +++ b/tests/proxy-verifier-checksum.txt @@ -1 +1 @@ -342286244d441329c12de1122520da6c86c581fd +5485a3cea4359e86458bf6d369dc18cdcfbd433d diff --git a/tests/proxy-verifier-version.txt b/tests/proxy-verifier-version.txt index 66cfae52b28..6d260c3af09 100644 --- a/tests/proxy-verifier-version.txt +++ b/tests/proxy-verifier-version.txt @@ -1 +1 @@ -v3.1.3 +v3.2.0 From df0c2e6ac4b319e3565c5e81d0f3c299d4844aad Mon Sep 17 00:00:00 2001 From: Javid Khan Date: Fri, 4 Sep 2026 07:43:05 +0530 Subject: [PATCH 11/15] dns: fix remaining-space tracking when copying address records (#13558) When decoding a DNS response, the path that copies an unaligned A or AAAA record into the host entry buffer moved the write position without updating the count of space remaining, so the two disagreed for the rest of the response. Update the count after the copy, the same way the name, CNAME and PTR paths already do. (cherry picked from commit 80e89e5130bb4adaea2624c527284dd803e3b0a3) --- src/iocore/dns/DNS.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/iocore/dns/DNS.cc b/src/iocore/dns/DNS.cc index aab3f4cd3e5..b1c8a68ad8a 100644 --- a/src/iocore/dns/DNS.cc +++ b/src/iocore/dns/DNS.cc @@ -1889,8 +1889,9 @@ dns_process(DNSHandler *handler, HostEnt *buf, int len) memcpy((*hap++ = bp), cp, n); Dbg(dbg_ctl_dns, "received %s = %s", QtypeName(type), inet_ntop(T_AAAA == type ? AF_INET6 : AF_INET, bp, ip_string, sizeof(ip_string))); - bp += n; - cp += n; + bp += n; + cp += n; + buflen = sizeof(buf->hostbuf) - (bp - buf->hostbuf); } } else { goto Lerror; From 82d4b4c466e079951debf10354a1161380449542 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Thu, 27 Aug 2026 17:32:11 -0500 Subject: [PATCH 12/15] Fix Clang 21 analyzer findings (#13593) Newer Clang releases expose latent ownership and error-handling issues, while the analyzer preset currently produces a GCC compilation database that Clang cannot reliably consume. This patch selects Clang explicitly for the analyzer preset, fixes the reported leaks, unchecked stream calls, and directory scanning under a mutex, and reshapes the remaining flagged code so the analyzer can follow it. That gives ATS a clean diagnostic baseline before the job moves to Ubuntu 26.04. Co-authored-by: Claude Opus 5 --- CMakePresets.json | 2 + plugins/healthchecks/healthchecks.cc | 110 +++++++++++++------------- src/iocore/hostdb/HostDB.cc | 4 + src/iocore/net/OCSPStapling.cc | 21 +++-- src/iocore/net/QUICNetVConnection.cc | 3 +- src/proxy/http/HttpBodyFactory.cc | 38 ++++----- src/proxy/http/HttpProxyServerMain.cc | 29 ++++--- src/proxy/http3/Http3Frame.cc | 3 +- src/proxy/http3/QPACK.cc | 6 +- 9 files changed, 116 insertions(+), 100 deletions(-) diff --git a/CMakePresets.json b/CMakePresets.json index 3fc994c0b9e..18676c1fec3 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -254,6 +254,8 @@ "description": "CI Pipeline config for running clang-analyzer", "inherits": ["ci"], "cacheVariables": { + "CMAKE_C_COMPILER": "clang", + "CMAKE_CXX_COMPILER": "clang++", "CMAKE_EXPORT_COMPILE_COMMANDS": "ON", "ENABLE_CCACHE": "OFF", "ENABLE_EXAMPLE": "OFF", diff --git a/plugins/healthchecks/healthchecks.cc b/plugins/healthchecks/healthchecks.cc index c5da85da9da..ece4319aee2 100644 --- a/plugins/healthchecks/healthchecks.cc +++ b/plugins/healthchecks/healthchecks.cc @@ -140,9 +140,9 @@ load_status_file(HCFileInfo *info) if (nullptr != (fd = fopen(info->fname, "r"))) { data->exists = 1; - size_t bytes_read; - while ((bytes_read = fread(data->body, 1, MAX_BODY_LEN, fd)) > 0) { - data->b_len = static_cast(bytes_read); + data->b_len = static_cast(fread(data->body, 1, MAX_BODY_LEN, fd)); + if (ferror(fd)) { + data->b_len = 0; } fclose(fd); } @@ -333,68 +333,66 @@ parse_configs(const char *fname) return nullptr; } - while (!feof(fd)) { + while (fgets(buf, sizeof(buf) - 1, fd) != nullptr) { char *str, *save; char *ok = nullptr, *miss = nullptr, *mime = nullptr; - if (fgets(buf, sizeof(buf) - 1, fd)) { - finfo = new HCFileInfo(); - - str = strtok_r(buf, SEPARATORS, &save); - int state = 0; - while (nullptr != str) { - if (strlen(str) > 0) { - switch (state) { - case 0: - if ('/' == *str) { - ++str; - } - strncpy(finfo->path, str, PATH_NAME_MAX - 1); - finfo->path[PATH_NAME_MAX - 1] = '\0'; - finfo->p_len = strlen(finfo->path); - break; - case 1: - strncpy(finfo->fname, str, MAX_PATH_LEN - 1); - finfo->fname[MAX_PATH_LEN - 1] = '\0'; - finfo->basename = strrchr(finfo->fname, '/'); - if (finfo->basename) { - ++(finfo->basename); - finfo->basename_len = strlen(finfo->basename); - } - break; - case 2: - mime = str; - break; - case 3: - ok = str; - break; - case 4: - miss = str; - break; + finfo = new HCFileInfo(); + + str = strtok_r(buf, SEPARATORS, &save); + int state = 0; + while (nullptr != str) { + if (strlen(str) > 0) { + switch (state) { + case 0: + if ('/' == *str) { + ++str; + } + strncpy(finfo->path, str, PATH_NAME_MAX - 1); + finfo->path[PATH_NAME_MAX - 1] = '\0'; + finfo->p_len = strlen(finfo->path); + break; + case 1: + strncpy(finfo->fname, str, MAX_PATH_LEN - 1); + finfo->fname[MAX_PATH_LEN - 1] = '\0'; + finfo->basename = strrchr(finfo->fname, '/'); + if (finfo->basename) { + ++(finfo->basename); + finfo->basename_len = strlen(finfo->basename); } - ++state; + break; + case 2: + mime = str; + break; + case 3: + ok = str; + break; + case 4: + miss = str; + break; } - str = strtok_r(nullptr, SEPARATORS, &save); + ++state; } + str = strtok_r(nullptr, SEPARATORS, &save); + } - /* Fill in the info if everything was ok */ - if (state > 4) { - Dbg(dbg_ctl, "Parsed: %s %s %s %s %s", finfo->path, finfo->fname, mime, ok, miss); - finfo->ok = gen_header(ok, mime, &finfo->o_len); - finfo->miss = gen_header(miss, mime, &finfo->m_len); - finfo->set_data(load_status_file(finfo)); - - /* Add it the linked list */ - Dbg(dbg_ctl, "Adding path=%s to linked list", finfo->path); - if (nullptr == head_finfo) { - head_finfo = finfo; - } else { - prev_finfo->_next = finfo; - } - prev_finfo = finfo; + /* Fill in the info if everything was ok */ + if (state > 4) { + Dbg(dbg_ctl, "Parsed: %s %s %s %s %s", finfo->path, finfo->fname, mime, ok, miss); + finfo->ok = gen_header(ok, mime, &finfo->o_len); + finfo->miss = gen_header(miss, mime, &finfo->m_len); + finfo->set_data(load_status_file(finfo)); + + /* Add it the linked list */ + Dbg(dbg_ctl, "Adding path=%s to linked list", finfo->path); + if (nullptr == head_finfo) { + head_finfo = finfo; } else { - delete finfo; + prev_finfo->_next = finfo; } + prev_finfo = finfo; + } else { + delete finfo; } } fclose(fd); diff --git a/src/iocore/hostdb/HostDB.cc b/src/iocore/hostdb/HostDB.cc index 8bf388f2a3b..f4ab635e486 100644 --- a/src/iocore/hostdb/HostDB.cc +++ b/src/iocore/hostdb/HostDB.cc @@ -1042,6 +1042,10 @@ HostDBContinuation::dnsEvent(int event, HostEnt *e) ts::LocalBuffer q_buf(valid_records); SRV **q = q_buf.data(); ink_assert(valid_records <= static_cast(hostdb_round_robin_max_count)); + // The loop below assigns every element, but ts::LocalBuffer hands back raw storage and the + // static analyzer cannot follow the loop well enough to see that. Pre-fill so the sort below + // is never reported as reading an uninitialized pointer. + std::fill_n(q, valid_records, nullptr); for (int i = 0; i < valid_records; ++i) { q[i] = &e->srv_hosts.hosts[i]; } diff --git a/src/iocore/net/OCSPStapling.cc b/src/iocore/net/OCSPStapling.cc index 3e5631f9974..6d246eff6c7 100644 --- a/src/iocore/net/OCSPStapling.cc +++ b/src/iocore/net/OCSPStapling.cc @@ -21,6 +21,8 @@ #include "P_OCSPStapling.h" +#include + #include #include #include @@ -903,19 +905,22 @@ ssl_stapling_init_cert(SSL_CTX *ctx, X509 *cert, const char *certname, const cha Dbg(dbg_ctl_ssl_ocsp, "using OCSP prefetched response file %s", rsp_file); FILE *fp = fopen(rsp_file, "r"); if (fp) { - fseek(fp, 0, SEEK_END); - long rsp_buf_len = ftell(fp); - if (rsp_buf_len >= 0) { - rewind(fp); - unsigned char *rsp_buf = static_cast(malloc(rsp_buf_len)); - auto read_len = fread(rsp_buf, 1, rsp_buf_len, fp); + long rsp_buf_len = -1; + + if (fseek(fp, 0, SEEK_END) == 0) { + rsp_buf_len = ftell(fp); + } + + if (rsp_buf_len > 0 && fseek(fp, 0, SEEK_SET) == 0) { + std::vector rsp_buf(rsp_buf_len); + auto read_len = fread(rsp_buf.data(), 1, rsp_buf.size(), fp); + if (read_len == static_cast(rsp_buf_len)) { - const unsigned char *p = rsp_buf; + const unsigned char *p = rsp_buf.data(); rsp = d2i_TS_OCSP_RESPONSE(nullptr, &p, rsp_buf_len); } else { Error("stapling_refresh_response: failed to read prefetched response file: %s", rsp_file); } - free(rsp_buf); } else { Error("stapling_refresh_response: failed to check the size of prefetched response file: %s", rsp_file); } diff --git a/src/iocore/net/QUICNetVConnection.cc b/src/iocore/net/QUICNetVConnection.cc index d0850992f79..1b239eb9b78 100644 --- a/src/iocore/net/QUICNetVConnection.cc +++ b/src/iocore/net/QUICNetVConnection.cc @@ -328,7 +328,8 @@ QUICNetVConnection::acceptEvent(int event, Event *e) MUTEX_TRY_LOCK(lock, h->mutex, t); if (!lock.is_locked()) { - if (event == EVENT_NONE) { + // Direct calls can have no event; reschedule on the thread in that case. + if (event == EVENT_NONE || e == nullptr) { t->schedule_in(this, HRTIME_MSECONDS(net_retry_delay)); return EVENT_DONE; } else { diff --git a/src/proxy/http/HttpBodyFactory.cc b/src/proxy/http/HttpBodyFactory.cc index d62380a5170..5f6cb07087e 100644 --- a/src/proxy/http/HttpBodyFactory.cc +++ b/src/proxy/http/HttpBodyFactory.cc @@ -262,6 +262,7 @@ HttpBodyFactory::reconfigure() unlock(); return; } // callbacks not setup right + unlock(); //////////////////////////////////////////// // extract relevant records.yaml values // @@ -272,15 +273,15 @@ HttpBodyFactory::reconfigure() all_found = true; // enable_customizations if records.yaml set - rec_err = RecGetRecordInt("proxy.config.body_factory.enable_customizations", &e); - enable_customizations = ((rec_err == REC_ERR_OKAY) ? e : 0); - all_found = all_found && (rec_err == REC_ERR_OKAY); - Dbg(dbg_ctl_body_factory, "enable_customizations = %d (found = %" PRId64 ")", enable_customizations, e); + rec_err = RecGetRecordInt("proxy.config.body_factory.enable_customizations", &e); + int new_enable_customizations = ((rec_err == REC_ERR_OKAY) ? e : 0); + all_found = all_found && (rec_err == REC_ERR_OKAY); + Dbg(dbg_ctl_body_factory, "enable_customizations = %d (found = %" PRId64 ")", new_enable_customizations, e); - rec_err = RecGetRecordInt("proxy.config.body_factory.enable_logging", &e); - enable_logging = ((rec_err == REC_ERR_OKAY) ? (e ? true : false) : false); - all_found = all_found && (rec_err == REC_ERR_OKAY); - Dbg(dbg_ctl_body_factory, "enable_logging = %d (found = %" PRId64 ")", enable_logging, e); + rec_err = RecGetRecordInt("proxy.config.body_factory.enable_logging", &e); + bool new_enable_logging = ((rec_err == REC_ERR_OKAY) ? (e ? true : false) : false); + all_found = all_found && (rec_err == REC_ERR_OKAY); + Dbg(dbg_ctl_body_factory, "enable_logging = %d (found = %" PRId64 ")", new_enable_logging, e); ats_scoped_str directory_of_template_sets; @@ -306,21 +307,16 @@ HttpBodyFactory::reconfigure() Warning("config changed, but can't fetch all proxy.config.body_factory values"); } - ///////////////////////////////////////////// - // clear out previous template hash tables // - ///////////////////////////////////////////// - - nuke_template_tables(); - - ///////////////////////////////////////////////////////////// - // at this point, the body hash table is gone, so we start // - // building a new one, by scanning the template directory. // - ///////////////////////////////////////////////////////////// - + std::unique_ptr new_table_of_sets; if (directory_of_template_sets) { - table_of_sets = load_sets_from_directory(directory_of_template_sets); + new_table_of_sets = load_sets_from_directory(directory_of_template_sets); } + lock(); + enable_customizations = new_enable_customizations; + enable_logging = new_enable_logging; + nuke_template_tables(); + table_of_sets = std::move(new_table_of_sets); unlock(); } @@ -728,7 +724,6 @@ HttpBodyFactory::nuke_template_tables() } } -// LOCKING: must be called with lock taken std::unique_ptr HttpBodyFactory::load_sets_from_directory(char *set_dir) { @@ -798,7 +793,6 @@ HttpBodyFactory::load_sets_from_directory(char *set_dir) return new_table_of_sets; } -// LOCKING: must be called with lock taken HttpBodySet * HttpBodyFactory::load_body_set_from_directory(char *set_name, char *tmpl_dir) { diff --git a/src/proxy/http/HttpProxyServerMain.cc b/src/proxy/http/HttpProxyServerMain.cc index 876b38bc83e..7308d52d987 100644 --- a/src/proxy/http/HttpProxyServerMain.cc +++ b/src/proxy/http/HttpProxyServerMain.cc @@ -192,19 +192,30 @@ MakeHttpProxyAcceptor(HttpProxyAcceptor &acceptor, HttpProxyPort &port, unsigned // XXX the protocol probe should be a configuration option. - ProtocolProbeSessionAccept *probe = new ProtocolProbeSessionAccept(); + // A QUIC port is dispatched to the QUIC acceptor below, which has no probe fallback, so building a + // probe for one only leaks it. Every other port type ends up behind the probe, either directly or + // as the SSL acceptor's fallback. Without QUIC compiled in no port can be a QUIC port, so this is + // always true there. + bool const needs_probe = !port.isQUIC(); + + ProtocolProbeSessionAccept *probe = nullptr; HttpSessionAccept *http = nullptr; // don't allocate this unless it will be used. - probe->proxyPort = &port; - probe->proxy_protocol_ipmap = &HttpConfig::m_master.config_proxy_protocol_ip_addrs; - if (port.m_session_protocol_preference.intersects(HTTP_PROTOCOL_SET)) { - http = new HttpSessionAccept(accept_opt); - probe->registerEndpoint(ProtocolProbeSessionAccept::PROTO_HTTP, http); - } + if (needs_probe) { + probe = new ProtocolProbeSessionAccept(); + probe->proxyPort = &port; + probe->proxy_protocol_ipmap = &HttpConfig::m_master.config_proxy_protocol_ip_addrs; + + if (port.m_session_protocol_preference.intersects(HTTP_PROTOCOL_SET)) { + http = new HttpSessionAccept(accept_opt); + probe->registerEndpoint(ProtocolProbeSessionAccept::PROTO_HTTP, http); + } - if (port.m_session_protocol_preference.intersects(HTTP2_PROTOCOL_SET)) { - probe->registerEndpoint(ProtocolProbeSessionAccept::PROTO_HTTP2, new Http2SessionAccept(accept_opt)); + if (port.m_session_protocol_preference.intersects(HTTP2_PROTOCOL_SET)) { + probe->registerEndpoint(ProtocolProbeSessionAccept::PROTO_HTTP2, new Http2SessionAccept(accept_opt)); + } } + ProtocolSessionCreateMap.insert({TS_ALPN_PROTOCOL_INDEX_HTTP_1_0, create_h1_server_session}); ProtocolSessionCreateMap.insert({TS_ALPN_PROTOCOL_INDEX_HTTP_1_1, create_h1_server_session}); ProtocolSessionCreateMap.insert({TS_ALPN_PROTOCOL_INDEX_HTTP_2_0, create_h2_server_session}); diff --git a/src/proxy/http3/Http3Frame.cc b/src/proxy/http3/Http3Frame.cc index b3de9d2bffe..a4c8adbc970 100644 --- a/src/proxy/http3/Http3Frame.cc +++ b/src/proxy/http3/Http3Frame.cc @@ -576,8 +576,7 @@ Http3FrameFactory::create_headers_frame(IOBufferReader *header_block_reader, siz { ats_unique_buf buf = ats_unique_malloc(header_block_len); - int64_t nread; - while ((nread = header_block_reader->read(buf.get(), header_block_len)) > 0) { + while (header_block_reader->read(buf.get(), header_block_len) > 0) { ; } diff --git a/src/proxy/http3/QPACK.cc b/src/proxy/http3/QPACK.cc index 80db3b497a8..fed8e7ecef0 100644 --- a/src/proxy/http3/QPACK.cc +++ b/src/proxy/http3/QPACK.cc @@ -291,11 +291,13 @@ QPACK::decode(uint64_t stream_id, const uint8_t *header_block, size_t header_blo if (largest_reference != 0 && (this->_dynamic_table.is_empty() || this->_dynamic_table.largest_index() < largest_reference)) { // Blocked - if (this->_add_to_blocked_list( - new DecodeRequest(largest_reference, thread, cont, stream_id, header_block, header_block_len, hdr))) { + auto *decode_request = new DecodeRequest(largest_reference, thread, cont, stream_id, header_block, header_block_len, hdr); + + if (this->_add_to_blocked_list(decode_request)) { return 1; } else { // Number of blocked streams exceed the limit + delete decode_request; return -2; } } From f93d95234e8d177c5ffa5d0a4d6051315d3aa12b Mon Sep 17 00:00:00 2001 From: Hiroaki Nakamura Date: Tue, 13 May 2025 07:50:31 +0900 Subject: [PATCH 13/15] Clean up Clang analyzer warnings (#12226) Uninitialized values and redundant initializations cause analyzer warnings in the 10.1.x branch. This patch initializes the affected locals and uses a value-initialized container for volume sorting. It backports apache/trafficserver#12226. (cherry picked from commit 0cf0f3e5e48fb55a71b534b69487c8ba3cdae747) --- plugins/experimental/memcache/tsmemcache.cc | 2 +- src/iocore/cache/CacheProcessor.cc | 10 ++++------ src/iocore/net/UnixNetVConnection.cc | 2 +- src/proxy/http/HttpTransact.cc | 2 +- src/tscore/ink_queue.cc | 1 + 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/plugins/experimental/memcache/tsmemcache.cc b/plugins/experimental/memcache/tsmemcache.cc index cce2ab829c5..2d33cdde760 100644 --- a/plugins/experimental/memcache/tsmemcache.cc +++ b/plugins/experimental/memcache/tsmemcache.cc @@ -249,7 +249,7 @@ MC::add_binary_header(uint16_t err, uint8_t hdr_len, uint16_t key_len, uint32_t int MC::write_binary_error(protocol_binary_response_status err, int swallow) { - const char *errstr = "Unknown error"; + const char *errstr{nullptr}; switch (err) { case PROTOCOL_BINARY_RESPONSE_ENOMEM: errstr = "Out of memory"; diff --git a/src/iocore/cache/CacheProcessor.cc b/src/iocore/cache/CacheProcessor.cc index b1acbfc6e08..b27c176cb8f 100644 --- a/src/iocore/cache/CacheProcessor.cc +++ b/src/iocore/cache/CacheProcessor.cc @@ -53,9 +53,11 @@ #include #include #include +#include #include #include #include +#include static void CachePeriodicMetricsUpdate(); static int64_t cache_bytes_used(int index); @@ -980,10 +982,8 @@ cplist_reconfigure() // else the size is greater... /* search the cp_list */ - int *sorted_vols = new int[gndisks]; - for (int i = 0; i < gndisks; i++) { - sorted_vols[i] = i; - } + std::vector sorted_vols(gndisks); + std::iota(sorted_vols.begin(), sorted_vols.end(), 0); for (int i = 0; i < gndisks - 1; i++) { int smallest = sorted_vols[i]; int smallest_ndx = i; @@ -1043,8 +1043,6 @@ cplist_reconfigure() size_to_alloc = size_in_blocks - cp->size; } - delete[] sorted_vols; - if (size_to_alloc) { if (create_volume(volume_number, size_to_alloc, cp->scheme, cp)) { return -1; diff --git a/src/iocore/net/UnixNetVConnection.cc b/src/iocore/net/UnixNetVConnection.cc index 3edf9eca083..d30c5b43c14 100644 --- a/src/iocore/net/UnixNetVConnection.cc +++ b/src/iocore/net/UnixNetVConnection.cc @@ -668,7 +668,7 @@ UnixNetVConnection::net_write_io(NetHandler *nh) nh->write_ready_list.remove(this); } - int err, ret; + int err{0}, ret{0}; if (this->get_context() == NET_VCONNECTION_OUT) { ret = this->sslStartHandShake(SSL_EVENT_CLIENT, err); diff --git a/src/proxy/http/HttpTransact.cc b/src/proxy/http/HttpTransact.cc index be7fd0a3262..16aa7efd345 100644 --- a/src/proxy/http/HttpTransact.cc +++ b/src/proxy/http/HttpTransact.cc @@ -7713,7 +7713,7 @@ void HttpTransact::handle_server_down(State *s) { const char *reason = nullptr; - const char *body_type = "UNKNOWN"; + const char *body_type = nullptr; HTTPStatus status = HTTP_STATUS_BAD_GATEWAY; //////////////////////////////////////////////////////// diff --git a/src/tscore/ink_queue.cc b/src/tscore/ink_queue.cc index 9ddee365415..86cab240010 100644 --- a/src/tscore/ink_queue.cc +++ b/src/tscore/ink_queue.cc @@ -171,6 +171,7 @@ ink_freelist_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32 } Dbg(dbg_ctl_freelist_init, "<%s> Alignment request/actual (%" PRIu32 "/%" PRIu32 ")", name, alignment, f->alignment); Dbg(dbg_ctl_freelist_init, "<%s> Type Size request/actual (%" PRIu32 "/%" PRIu32 ")", name, type_size, f->type_size); + ink_assert(f->type_size != 0); if (f->use_hugepages) { f->chunk_size = INK_ALIGN(chunk_size * f->type_size, ats_hugepage_size()) / f->type_size; } else { From 7fb5ab1a08de3f62d81e46c6835280728a38ed42 Mon Sep 17 00:00:00 2001 From: bneradt Date: Mon, 14 Sep 2026 22:04:30 -0500 Subject: [PATCH 14/15] Restore Clang analyzer checks on 10.1.x Clang 21 still flags the unguarded insertion-sort path used for SRV records, even after the pointer array is initialized. This keeps the 10.1.x analyzer job failing after the earlier analyzer fixes. This branch backports apache/trafficserver#13593 and apache/trafficserver#12226, and uses stable sorting with the existing SRV comparator. The sort preserves priority/key ordering while avoiding the analyzer's unguarded-sort false positive. --- src/iocore/hostdb/HostDB.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iocore/hostdb/HostDB.cc b/src/iocore/hostdb/HostDB.cc index f4ab635e486..11230a6982b 100644 --- a/src/iocore/hostdb/HostDB.cc +++ b/src/iocore/hostdb/HostDB.cc @@ -1049,7 +1049,7 @@ HostDBContinuation::dnsEvent(int event, HostEnt *e) for (int i = 0; i < valid_records; ++i) { q[i] = &e->srv_hosts.hosts[i]; } - std::sort(q, q + valid_records, [](SRV *lhs, SRV *rhs) -> bool { return *lhs < *rhs; }); + std::stable_sort(q, q + valid_records, [](SRV *lhs, SRV *rhs) -> bool { return *lhs < *rhs; }); SRV **cur_srv = q; for (auto &item : rr_info) { From d99b11ac3b06c23cc9307f10140e53c943b61a67 Mon Sep 17 00:00:00 2001 From: Chris McFarlen Date: Wed, 16 Sep 2026 10:48:54 -0700 Subject: [PATCH 15/15] autest: make the await-file-contains helper safe for regex needles AddAwaitFileContainsTestRun() interpolated the needle into the waiting process's shell command. autest runs string.Template.substitute() over that command, so a needle containing '$' -- e.g. an anchored regex like '\*/18 hit-fresh, none$' -- raises "Invalid placeholder in string" and the test run dies before starting. Drop the needle from the echoed command; it is still passed to When.FileContains(), which is what actually does the matching. This is a partial backport of apache/trafficserver#13034 (master commit 064014b40f2f2388e47eb2c0f69619dccc9b6828, "Makes the await helper safe for regex needles"). Only the when.test.ext hardening is taken here -- the rest of that commit converts other autests from condwait to this helper, which is not needed on this branch. #13034 is therefore only partially present on 10.1.x; it is on 10.2.x in full as 239d7010d0. Needed by the cherry-pick of #13322, whose cache.log needle is anchored with '$'. --- tests/gold_tests/autest-site/when.test.ext | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/gold_tests/autest-site/when.test.ext b/tests/gold_tests/autest-site/when.test.ext index ad147e9b756..8975d27cb18 100644 --- a/tests/gold_tests/autest-site/when.test.ext +++ b/tests/gold_tests/autest-site/when.test.ext @@ -89,7 +89,7 @@ def AddAwaitFileContainsTestRun(test, name, file_path, needle, desired_count=1) ''' tr = test.AddTestRun(name) p = tr.Processes.Default - p.Command = f'echo waiting for {needle} in {file_path}' + p.Command = f'echo waiting for file content in {file_path}' await_process = tr.Processes.Process('await', 'sleep 60') await_process.Ready = When.FileContains(file_path, needle, desired_count) await_process.StartupTimeout = 30