Copilot commented on code in PR #12962:
URL: https://github.com/apache/trafficserver/pull/12962#discussion_r2930081275
##########
tests/gold_tests/autest-site/ats_replay.test.ext:
##########
@@ -35,9 +35,37 @@ def configure_ats(obj: 'TestRun', server: 'Process',
ats_config: dict, dns: Opti
name = ats_config.get('name', 'ts')
process_config = ats_config.get('process_config', {})
ts = obj.MakeATSProcess(name, **process_config)
+
+ # Configure records_config if specified.
records_config = ats_config.get('records_config', {})
ts.Disk.records_config.update(records_config)
+ # TLS configs
+ enable_tls = process_config.get('enable_tls', False)
+ if enable_tls:
+ # Configure ssl_multicert.config if specified.
+ ssl_multicert_config = ats_config.get('ssl_multicert_config', [])
+
+ # setup default cert and key ssl_multicert_config is empty
Review Comment:
This comment is a bit unclear/grammatically incorrect: it reads like part of
the condition is missing. Consider rewording to something like “Set up default
cert and key when ssl_multicert_config is empty” so it’s clear why the default
certificate material is being installed.
##########
tests/gold_tests/chunked_encoding/replays/large_chunked.replay.yaml:
##########
@@ -0,0 +1,170 @@
+# 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"
+
+# Configuration section for autest integration
+autest:
+ description: 'Large chunked response from origin server'
+
+ dns:
+ name: 'dns-chunked-origin'
+
+ server:
+ name: 'server-chunked-origin'
+
+ client:
+ name: 'client-chunked-origin'
+
+ ats:
+ name: 'ts-chunked-large'
+ process_config:
+ enable_tls: true
+ enable_cache: true
+
+ records_config:
+ proxy.config.diags.debug.enabled: 1
+ proxy.config.diags.debug.tags: 'http'
+ proxy.config.http.response_via_str: 2
+
+ remap_config:
+ - from: "http://www.example.com/";
+ to: "http://backend.example.com:{SERVER_HTTP_PORT}/";
+ - from: "https://www.example.com/";
+ to: "http://backend.example.com:{SERVER_HTTP_PORT}/";
+
+sessions:
+
+# HTTP/1.0 - dechunk
+
+- transactions:
+ - client-request:
+ method: GET
+ url: /stream/1.0/
+ version: '1.0'
+ headers:
+ fields:
+ - [ Host, www.example.com ]
+ - [ uuid, http-1.0-chunk ]
+
+ server-response:
+ status: 200
+ version: '1.1'
+ headers:
+ fields:
+ - [ Content-Type, text/plain ]
+ - [ Transfer-Encoding, chunked ]
+ - [ Cache-Control, "public, max-age=3600" ]
+ content:
+ size: 131072
+
+ proxy-response:
+ status: 200
+ headers:
+ fields:
+ - [ Transfer-Encoding, { as: absent } ]
+ content:
+ size: 131072
+
+# HTTP/1.1 - pass-through for user agent & dechunk for cache write
+
+- transactions:
+ - client-request:
+ method: GET
+ url: /stream/1.1/
+ version: '1.1'
+ headers:
+ fields:
+ - [ Host, www.example.com ]
+ - [ uuid, http-1.1-chunk-0 ]
+
+ server-response:
+ status: 200
+ version: '1.1'
+ headers:
+ fields:
+ - [ Content-Type, text/plain ]
+ - [ Transfer-Encoding, chunked ]
+ - [ Cache-Control, "public, max-age=3600" ]
+ content:
+ size: 131072
+
+ proxy-response:
+ status: 200
+ headers:
+ fields:
+ - [ Transfer-Encoding, { value: chunked, as: equal } ]
+ content:
+ size: {value: 131072, as: equal}
+
+ # Expect: hit dechunked contents in the cache
+ - client-request:
+ method: GET
+ url: /stream/1.1/
+ version: '1.1'
+ headers:
+ fields:
+ - [ Host, www.example.com ]
+ - [ uuid, http-1.1-chunk-1 ]
+
+ server-response:
+ status: 503
+ version: '1.1'
+
+ proxy-response:
+ status: 200
+ headers:
+ fields:
+ - [ Content-Length, { value: 131072, as: equal} ]
+ - [ Transfer-Encoding, { as: absent } ]
+ content:
+ size: {value: 131072, as: equal}
+
+# HTTP/2 - dechunk
+
+- protocol:
Review Comment:
The HTTP/2 session declares `:scheme` as `https` and this replay enables TLS
for ATS, but the `protocol` stack does not include a `tls` layer. As written
this will exercise cleartext HTTP/2 (h2c) on the HTTP port (or potentially
fail), not HTTP/2 over TLS as intended. Add `- name: tls` (and typically
`tcp`/`ip`) under `protocol` for this session so the client connects to the
HTTPS port and negotiates h2 via ALPN.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
