You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "JakeChampion (via GitHub)" <gi...@apache.org> on 2026/10/09 12:27:11 UTC
[PR] http: recognize QUERY as a well-known method [trafficserver]
JakeChampion opened a new pull request, #13820:
URL: https://github.com/apache/trafficserver/pull/13820
The HTTP `QUERY` method ([draft-ietf-httpbis-safe-method-w-body](https://datatracker.ietf.org/doc/draft-ietf-httpbis-safe-method-w-body/)) is currently treated as an extension method. As a result:
- it is tunneled rather than handled like the other standard methods
- it cannot be named in `ip_allow.yaml` or `remap.config` ACLs
- it is not counted in the per-method request stats
This change registers `QUERY` in the well-known token table and gives it the semantics the draft defines: it is **safe**, **idempotent**, and **carries request content**.
### Retry behavior
Because `QUERY` is safe, it now takes the retry-after-bytes-sent path that `GET` and `HEAD` use.
Unlike those methods it has a body, so after the request has been sent a retry is only allowed if the request body was buffered and can be replayed. Otherwise it is handled like an unsafe method.
### Not in this PR
Caching of `QUERY` responses is left for a follow-up, since it needs the request body to be part of the cache key.
The plugin API and logstats changes will follow as separate PRs - if we want them included in this PR, let me know.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub) via github" <gi...@trafficserver.apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4441036107
##########
src/proxy/http/HttpSM.cc:
##########
@@ -6712,7 +6713,13 @@ HttpSM::handle_server_setup_error(int event, void *data)
t_state.set_connect_fail(EPIPE);
break;
case VC_EVENT_ERROR:
- t_state.current.state = HttpTransact::CONNECTION_ERROR;
+ // A QUERY body may have been consumed before a TLS read error. Only an
+ // admitted not-processed retry can rearm it after the request was sent.
+ if (t_state.method == HTTP_WKSIDX_QUERY && server_request_hdr_bytes > 0) {
+ t_state.current.state = this->prepare_for_origin_retry() ? HttpTransact::CONNECTION_ERROR : HttpTransact::CONNECTION_CLOSED;
Review Comment:
`server_request_hdr_bytes` is populated before I/O, so this condition also matches a first write that fails without sending any bytes. For an HTTP/1 origin, `prepare_for_origin_retry()` returns false, converting that failure into `CONNECTION_CLOSED` and rejecting a retry of the still-intact QUERY. Track actual header-write progress separately from the queued length, and preserve the `CONNECTION_ERROR` retry path when nothing was sent.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4232209159
##########
src/proxy/http/HttpTransactHeaders.cc:
##########
@@ -125,7 +127,7 @@ HttpTransactHeaders::insert_supported_methods_in_response(HTTPHdr *response, int
const char *methods[] = {
HTTP_METHOD_CONNECT.c_str(), HTTP_METHOD_DELETE.c_str(), HTTP_METHOD_GET.c_str(), HTTP_METHOD_HEAD.c_str(),
HTTP_METHOD_OPTIONS.c_str(), HTTP_METHOD_POST.c_str(), HTTP_METHOD_PURGE.c_str(), HTTP_METHOD_PUT.c_str(),
- HTTP_METHOD_PUSH.c_str(), HTTP_METHOD_TRACE.c_str(),
+ HTTP_METHOD_PUSH.c_str(), HTTP_METHOD_QUERY.c_str(), HTTP_METHOD_TRACE.c_str(),
};
char inline_buffer[64];
Review Comment:
Adding `QUERY` likely pushes the serialized `Allow` value past 64 bytes (depending on delimiter formatting), which will force heap allocation for `value_buffer` in more cases. Consider increasing `inline_buffer` (e.g., to 128) to keep common `Allow` values on-stack and avoid extra allocations.
##########
doc/release-notes/whats-new.en.rst:
##########
@@ -107,6 +108,11 @@ Features
* Add an optional ``avg_obj_size`` to ``storage.yaml`` to control the directory
entry sizing.
* The ``proxy.config.http.cache.post_method`` is now an overridable config.
+* The HTTP ``QUERY`` method (draft-ietf-httpbis-safe-method-w-body) is now a
Review Comment:
In RST output, `(draft-ietf-httpbis-safe-method-w-body)` won’t render as a clickable reference. Consider converting this into an actual link (e.g., inline external link) so readers can navigate directly to the draft from the release notes.
##########
tests/gold_tests/autest-site/ats_replay.test.ext:
##########
@@ -162,6 +162,11 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti
if logging_yaml != None:
ts.Disk.logging_yaml.AddLines(yaml.dump(logging_yaml).split('\n'))
+ # Configure ip_allow.yaml if specified.
+ ip_allow_yaml = ats_config.get('ip_allow_yaml')
+ if ip_allow_yaml is not None:
Review Comment:
This change introduces `is not None` checks next to an existing `!= None` check. For consistent and idiomatic Python (and to avoid `__eq__` surprises), update the pre-existing `if logging_yaml != None:` to `if logging_yaml is not None:` (and keep the rest consistent).
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4246199007
##########
tests/gold_tests/query/replay/query-pass-through.replay.yaml:
##########
@@ -0,0 +1,135 @@
+# 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 QUERY requests are proxied with their content'
+
+ server:
+ name: 'server'
+
+ client:
+ name: 'client'
+
+ ats:
+ name: 'ts'
+ process_config:
+ enable_cache: true
+ records_config:
+ proxy.config.diags.debug.enabled: 1
+ proxy.config.diags.debug.tags: 'http_trans|http_cache'
+ remap_config:
+ - from: "http://example.com/"
+ to: "http://127.0.0.1:{SERVER_HTTP_PORT}/"
+ metric_checks:
+ - metric: 'proxy.process.http.query_requests'
+ value: 3
+
+sessions:
+- transactions:
+
+ # A QUERY with content is forwarded to the origin, content and all.
+ - client-request:
+ method: QUERY
+ url: /search
+ version: '1.1'
+ headers:
+ fields:
+ - [Host, example.com]
+ - [Content-Type, application/sql]
+ - [Content-Length, "23"]
+ - [uuid, query-first]
+ content:
+ data: "SELECT * FROM t WHERE a"
+
+ proxy-request:
+ method: QUERY
+ headers:
+ fields:
+ - [Content-Length, {value: "23", as: equal}]
Review Comment:
This test claims it verifies that `QUERY` content is proxied, but it only asserts the `Content-Length` header. To prevent regressions where the proxy forwards headers but drops/changes the body, add an assertion that validates the forwarded request body content (e.g., verify the origin receives the expected payload, if ATS replay supports request-body verification).
##########
src/proxy/http/HttpTransact.cc:
##########
@@ -6945,10 +6947,15 @@ HttpTransact::is_request_retryable(State *s)
return false;
}
+ // A QUERY is safe, but unlike the other safe methods it carries content. Once
+ // the request has gone out we can only replay it if that content was
+ // buffered.
+ bool const content_is_replayable = s->method != HTTP_WKSIDX_QUERY || s->state_machine->is_buffering_request_body;
+
// If safe requests are retryable, it should be safe to retry safe requests irrespective of bytes sent or connection state
// according to RFC the following methods are safe (https://tools.ietf.org/html/rfc7231#section-4.2.1)
// Otherwise, if there was no error establishing the connection (and we sent bytes)-- we cannot retry
- if (!HttpTransactHeaders::is_method_safe(s->method) && s->current.state != CONNECTION_ERROR &&
+ if ((!HttpTransactHeaders::is_method_safe(s->method) || !content_is_replayable) && s->current.state != CONNECTION_ERROR &&
s->state_machine->server_request_hdr_bytes > 0) {
Review Comment:
The PR description calls out nuanced retry semantics for `QUERY` (retry like safe methods, but only after send if the body can be replayed). There isn’t a gold test exercising this new branch (buffered vs non-buffered body) and it’s easy to regress because it depends on connection-error timing and buffering config. Consider adding a targeted replay test that forces a retry scenario and asserts: (1) `QUERY` retries when buffering is enabled and the request body is replayable, and (2) `QUERY` does not retry (or follows unsafe-method behavior) when buffering is disabled.
##########
src/proxy/http/HttpTransact.cc:
##########
@@ -6945,10 +6947,15 @@ HttpTransact::is_request_retryable(State *s)
return false;
}
+ // A QUERY is safe, but unlike the other safe methods it carries content. Once
+ // the request has gone out we can only replay it if that content was
+ // buffered.
+ bool const content_is_replayable = s->method != HTTP_WKSIDX_QUERY || s->state_machine->is_buffering_request_body;
+
// If safe requests are retryable, it should be safe to retry safe requests irrespective of bytes sent or connection state
// according to RFC the following methods are safe (https://tools.ietf.org/html/rfc7231#section-4.2.1)
// Otherwise, if there was no error establishing the connection (and we sent bytes)-- we cannot retry
Review Comment:
The comment references RFC 7231 and implies a fixed list of safe methods, but this PR updates safety semantics (RFC 9110 + draft `QUERY`). Please update the comment and link to match current semantics (and mention `QUERY`’s body replay constraint), so the code and documentation don’t diverge.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub) via github" <gi...@trafficserver.apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4436951703
##########
src/proxy/http/HttpTransact.cc:
##########
@@ -7063,11 +7065,15 @@ HttpTransact::is_request_retryable(State *s)
return false;
}
- // If safe requests are retryable, it should be safe to retry safe requests irrespective of bytes sent or connection state
- // according to RFC the following methods are safe (https://tools.ietf.org/html/rfc7231#section-4.2.1)
+ // Safe methods (RFC 9110, section 9.2.1) can be retried irrespective of bytes sent or connection state. QUERY
+ // (draft-ietf-httpbis-safe-method-w-body) is safe too, but its content is not kept for replay on this path, so once
+ // any part of its request has been sent it is treated like an unsafe method, whether or not the content itself went
+ // out. A buffered QUERY can still be retried when the origin reports it was not processed, see
+ // HttpSM::prepare_for_origin_retry.
// Otherwise, if there was no error establishing the connection (and we sent bytes)-- we cannot retry
- if (!HttpTransactHeaders::is_method_safe(s->method) && s->current.state != CONNECTION_ERROR &&
- s->state_machine->server_request_hdr_bytes > 0) {
+ bool const retry_only_before_sending = !HttpTransactHeaders::is_method_safe(s->method) || s->method == HTTP_WKSIDX_QUERY;
+
+ if (retry_only_before_sending && s->current.state != CONNECTION_ERROR && s->state_machine->server_request_hdr_bytes > 0) {
Review Comment:
After a completed unbuffered QUERY upload, an origin-side TLS read error reaches `handle_server_setup_error(VC_EVENT_ERROR)`, which sets `CONNECTION_ERROR` without checking body replayability. This exception then permits a retry with an exhausted body reader, so ATS cannot resend the content promised by the request headers. Gate post-upload QUERY retries on body replayability while preserving pre-send failures and explicitly admitted not-processed retries, and add a TLS-error regression case.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4249835093
##########
doc/release-notes/whats-new.en.rst:
##########
@@ -107,6 +108,12 @@ Features
* Add an optional ``avg_obj_size`` to ``storage.yaml`` to control the directory
entry sizing.
* The ``proxy.config.http.cache.post_method`` is now an overridable config.
+* The HTTP ``QUERY`` method
+ (`draft-ietf-httpbis-safe-method-w-body <https://datatracker.ietf.org/doc/draft-ietf-httpbis-safe-method-w-body/>`_)
+ is now a well-known method. It can be named in :file:`ip_allow.yaml` and
+ :file:`remap.config` ACLs, it is reported by
+ ``proxy.process.http.query_requests``, and like the other content-carrying
+ methods it requires a ``Content-Length`` or chunked framing.
Review Comment:
“chunked framing” is HTTP/1.1-specific and can be ambiguous (e.g., vs HTTP/2 framing). Consider clarifying this as an HTTP/1.1 requirement and/or wording it as “`Content-Length` or `Transfer-Encoding: chunked`” to match protocol terminology.
##########
tests/gold_tests/query/replay/query-pass-through.replay.yaml:
##########
@@ -0,0 +1,130 @@
+# 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 QUERY requests are proxied with their content'
+
+ server:
+ name: 'server'
+
+ client:
+ name: 'client'
+
+ ats:
+ name: 'ts'
+ process_config:
+ enable_cache: true
+ records_config:
+ proxy.config.diags.debug.enabled: 1
+ proxy.config.diags.debug.tags: 'http_trans|http_cache'
+ remap_config:
+ - from: "http://example.com/"
+ to: "http://127.0.0.1:{SERVER_HTTP_PORT}/"
+ metric_checks:
+ - metric: 'proxy.process.http.query_requests'
+ value: 3
+
+sessions:
+- transactions:
+
+ # A QUERY with content is forwarded to the origin, content and all.
+ - client-request:
+ method: QUERY
+ url: /search
+ version: '1.1'
+ headers:
+ fields:
+ - [Host, example.com]
+ - [Content-Type, application/sql]
+ - [Content-Length, "23"]
+ - [uuid, query-first]
+ content:
+ data: "SELECT * FROM t WHERE a"
+
+ proxy-request:
+ method: QUERY
+ headers:
+ fields:
+ - [Content-Length, {value: "23", as: equal}]
+ content:
+ verify: {value: "SELECT * FROM t WHERE a", as: equal}
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [Content-Length, "8"]
+ - [Cache-Control, "max-age=300"]
+ content:
+ data: "result-1"
+
+ proxy-response:
+ status: 200
+
+ # Caching of QUERY is off by default, so the identical request goes to the
+ # origin again rather than being served from cache.
Review Comment:
This comment can read as if caching is globally disabled, but this test enables cache (`enable_cache: true`). Consider clarifying that it’s specifically `QUERY` response caching that’s off by default (e.g., “Caching of QUERY responses is off by default…”), to better align with the test setup and the PR description.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4248201766
##########
tests/gold_tests/query/query_retry_origin.py:
##########
@@ -0,0 +1,76 @@
+'''
+An origin that closes the first connection after reading the request, without
+responding, and answers every later request with a 200.
+'''
+# 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.
+
+import argparse
+import socket
+import sys
+
+
+def read_request(conn: socket.socket) -> tuple[str, bytes]:
+ data = b''
+ while b'\r\n\r\n' not in data:
+ chunk = conn.recv(4096)
+ if not chunk:
+ return '', b''
+ data += chunk
+ head, _, body = data.partition(b'\r\n\r\n')
+ lines = head.decode('latin-1').split('\r\n')
+ method = lines[0].split(' ')[0]
+ length = 0
+ for line in lines[1:]:
+ name, _, value = line.partition(':')
+ if name.strip().lower() == 'content-length':
+ length = int(value.strip())
+ while len(body) < length:
+ chunk = conn.recv(4096)
+ if not chunk:
+ break
+ body += chunk
+ return method, body[:length]
+
+
+def main() -> int:
+ parser = argparse.ArgumentParser()
+ parser.add_argument('port', type=int)
+ args = parser.parse_args()
+
+ with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as listener:
+ listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+ listener.bind(('127.0.0.1', args.port))
+ listener.listen()
+ attempt = 0
+ while True:
+ conn, _ = listener.accept()
+ with conn:
+ method, body = read_request(conn)
+ if not method:
+ continue
+ attempt += 1
+ print(f'request attempt={attempt} method={method} body={body.decode()}', flush=True)
Review Comment:
`body.decode()` uses the default UTF-8 codec and can raise `UnicodeDecodeError` if the request body contains non-UTF8 bytes, which would crash the test origin process. Consider decoding with an explicit codec and error handling (e.g., `latin-1` or `utf-8` with `errors="replace"`), or printing a safe representation (e.g., `repr(body)`), so the helper remains robust even if the request payload changes.
##########
tests/gold_tests/query/query_retry_origin.py:
##########
@@ -0,0 +1,76 @@
+'''
+An origin that closes the first connection after reading the request, without
+responding, and answers every later request with a 200.
+'''
+# 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.
+
+import argparse
+import socket
+import sys
+
+
+def read_request(conn: socket.socket) -> tuple[str, bytes]:
+ data = b''
+ while b'\r\n\r\n' not in data:
+ chunk = conn.recv(4096)
+ if not chunk:
+ return '', b''
+ data += chunk
Review Comment:
Building `data` via repeated `bytes` concatenation (`data += chunk`) is O(n²) due to repeated reallocations/copies as the header grows. Using a `bytearray` (or accumulating chunks into a list and `b''.join(...)`) avoids quadratic behavior and makes this helper more resilient if headers are large.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4249237419
##########
src/proxy/http/HttpTransact.cc:
##########
@@ -6945,10 +6947,12 @@ HttpTransact::is_request_retryable(State *s)
return false;
}
- // If safe requests are retryable, it should be safe to retry safe requests irrespective of bytes sent or connection state
- // according to RFC the following methods are safe (https://tools.ietf.org/html/rfc7231#section-4.2.1)
+ // Safe methods (RFC 9110, section 9.2.1) can be retried irrespective of bytes sent or connection state. QUERY
+ // (draft-ietf-httpbis-safe-method-w-body) is safe too, but its content is not kept for replay on this path, so once
+ // any of its request has been sent it is treated like an unsafe method, whether or not the content itself went out. A
+ // buffered QUERY can still be retried when the origin reports it was not processed, see HttpSM::prepare_for_origin_retry.
// Otherwise, if there was no error establishing the connection (and we sent bytes)-- we cannot retry
- if (!HttpTransactHeaders::is_method_safe(s->method) && s->current.state != CONNECTION_ERROR &&
+ if ((!HttpTransactHeaders::is_method_safe(s->method) || s->method == HTTP_WKSIDX_QUERY) && s->current.state != CONNECTION_ERROR &&
Review Comment:
The retry gating condition is correct but hard to read because it mixes the general safe/unsafe rule with the `QUERY` exception inline. Consider refactoring into a clearly named boolean (e.g., `treat_as_unsafe_after_sending_bytes`) or explicitly structuring it as `(is_safe && method != QUERY)` vs `!is_safe` to make the special-case behavior self-evident and reduce the chance of future regressions when additional safe methods are added.
##########
src/proxy/http/HttpTransact.cc:
##########
@@ -6945,10 +6947,12 @@ HttpTransact::is_request_retryable(State *s)
return false;
}
- // If safe requests are retryable, it should be safe to retry safe requests irrespective of bytes sent or connection state
- // according to RFC the following methods are safe (https://tools.ietf.org/html/rfc7231#section-4.2.1)
+ // Safe methods (RFC 9110, section 9.2.1) can be retried irrespective of bytes sent or connection state. QUERY
+ // (draft-ietf-httpbis-safe-method-w-body) is safe too, but its content is not kept for replay on this path, so once
+ // any of its request has been sent it is treated like an unsafe method, whether or not the content itself went out. A
Review Comment:
The comment has a grammatical issue: 'any of its request' reads incorrectly. Reword to something like 'once any part of its request has been sent' (or 'once any of its request bytes have been sent') to match the intent.
##########
src/proxy/http/HttpTransactHeaders.cc:
##########
@@ -125,9 +127,9 @@ HttpTransactHeaders::insert_supported_methods_in_response(HTTPHdr *response, int
const char *methods[] = {
HTTP_METHOD_CONNECT.c_str(), HTTP_METHOD_DELETE.c_str(), HTTP_METHOD_GET.c_str(), HTTP_METHOD_HEAD.c_str(),
HTTP_METHOD_OPTIONS.c_str(), HTTP_METHOD_POST.c_str(), HTTP_METHOD_PURGE.c_str(), HTTP_METHOD_PUT.c_str(),
- HTTP_METHOD_PUSH.c_str(), HTTP_METHOD_TRACE.c_str(),
+ HTTP_METHOD_PUSH.c_str(), HTTP_METHOD_QUERY.c_str(), HTTP_METHOD_TRACE.c_str(),
};
- char inline_buffer[64];
+ char inline_buffer[128];
Review Comment:
Bumping `inline_buffer` from 64 to 128 addresses the immediate size issue, but it introduces another arbitrary limit. If feasible in this function, prefer computing the required buffer size from the method strings (and any existing `Allow` value being appended to) and allocating when needed, so future method additions don't require another manual bump.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4259005128
##########
src/proxy/http/HttpTransact.cc:
##########
@@ -5819,9 +5819,9 @@ HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr)
return RequestError_t::UNACCEPTABLE_TE_REQUIRED;
}
- // Require Content-Length/Transfer-Encoding for POST/PUSH/PUT
+ // Require Content-Length/Transfer-Encoding for POST/PUSH/PUT/QUERY
if ((scheme == URL_WKSIDX_HTTP || scheme == URL_WKSIDX_HTTPS) &&
- (method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT) &&
+ (method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT || method == HTTP_WKSIDX_QUERY) &&
Review Comment:
The set of ‘content-carrying’ methods is now duplicated across multiple call sites (e.g., request validity checks here, HTTP/2 header framing logic, `Expect: 100-continue` handling, and server-session privacy decisions). This increases the risk of future inconsistencies when additional methods are added/changed. Consider factoring this into a single helper (e.g., `HttpTransactHeaders::method_allows_request_body()` / `is_method_content_carrying()`) and reusing it across these code paths.
##########
tests/gold_tests/query/query-retry.test.py:
##########
@@ -0,0 +1,79 @@
+'''
+Verify QUERY is not retried once its request, content included, has been sent
+to an origin that then closes the connection, whether or not request buffering
+is enabled.
+'''
+# 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.
+
+import re
+import sys
+
+from ports import get_port
+
+Test.Summary = '''
+Verify QUERY is not retried once its request, content included, has been sent
+to an origin that then closes the connection, whether or not request buffering
+is enabled.
+'''
+
+Test.ContinueOnFail = True
+
+BODY = 'SELECT * FROM t WHERE a'
+
+
+def add_scenario(name: str, method: str, buffering: bool) -> None:
+ tr = Test.AddTestRun(f'{method} after the origin closes, request buffering {"enabled" if buffering else "disabled"}')
+ tr.Setup.Copy('query_retry_origin.py')
+
+ server = Test.Processes.Process(f'origin-{name}')
+ server_port = get_port(server, 'http_port')
+ server.Command = f'{sys.executable} {tr.RunDirectory}/query_retry_origin.py {server_port}'
+ server.Ready = When.PortOpen(server_port)
+
+ ts = Test.MakeATSProcess(f'ts-{name}')
+ ts.Disk.records_config.update(
+ {
+ 'proxy.config.http.request_buffer_enabled': 1 if buffering else 0,
+ 'proxy.config.http.connect_attempts_max_retries': 3,
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http_trans',
+ })
+ ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{server_port}/')
+
+ content = f"-H 'Content-Type: application/sql' --data-binary '{BODY}' " if method == 'QUERY' else ''
+ tr.Processes.Default.StartBefore(server)
+ tr.Processes.Default.StartBefore(ts)
+ tr.Processes.Default.Command = (
+ f"curl -s -o /dev/null -w '%{{http_code}}\\n' -X {method} {content}http://127.0.0.1:{ts.Variables.port}/search")
Review Comment:
Building a shell command by interpolating request body content into single quotes is brittle (e.g., if `BODY` ever contains a `'`, newlines, or other shell-significant characters). To make the test more robust, consider passing the payload via stdin or a temporary file (e.g., `--data-binary @-` or `@file`) and avoiding direct body interpolation in the command string.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "JakeChampion (via GitHub)" <gi...@apache.org>.
JakeChampion commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4248886908
##########
src/proxy/http/HttpTransact.cc:
##########
@@ -5819,9 +5819,9 @@ HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr)
return RequestError_t::UNACCEPTABLE_TE_REQUIRED;
}
- // Require Content-Length/Transfer-Encoding for POST/PUSH/PUT
+ // Require Content-Length/Transfer-Encoding for POST/PUSH/PUT/QUERY
if ((scheme == URL_WKSIDX_HTTP || scheme == URL_WKSIDX_HTTPS) &&
- (method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT) &&
+ (method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT || method == HTTP_WKSIDX_QUERY) &&
s->client_info.transfer_encoding != TransferEncoding_t::CHUNKED) {
Review Comment:
A QUERY with no content isn't a meaningful request.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4259263190
##########
tests/gold_tests/query/query-retry.test.py:
##########
@@ -0,0 +1,80 @@
+'''
+Verify QUERY is not retried once its request, content included, has been sent
+to an origin that then closes the connection, whether or not request buffering
+is enabled.
+'''
+# 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.
+
+import re
+import shlex
+import sys
+
+from ports import get_port
+
+Test.Summary = '''
+Verify QUERY is not retried once its request, content included, has been sent
+to an origin that then closes the connection, whether or not request buffering
+is enabled.
+'''
+
+Test.ContinueOnFail = True
+
+BODY = 'SELECT * FROM t WHERE a'
+
+
+def add_scenario(name: str, method: str, buffering: bool) -> None:
+ tr = Test.AddTestRun(f'{method} after the origin closes, request buffering {"enabled" if buffering else "disabled"}')
+ tr.Setup.Copy('query_retry_origin.py')
+
+ server = Test.Processes.Process(f'origin-{name}')
+ server_port = get_port(server, 'http_port')
+ server.Command = f'{sys.executable} {tr.RunDirectory}/query_retry_origin.py {server_port}'
+ server.Ready = When.PortOpen(server_port)
+
+ ts = Test.MakeATSProcess(f'ts-{name}')
+ ts.Disk.records_config.update(
+ {
+ 'proxy.config.http.request_buffer_enabled': 1 if buffering else 0,
+ 'proxy.config.http.connect_attempts_max_retries': 3,
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http_trans',
+ })
+ ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{server_port}/')
+
+ content = f"-H 'Content-Type: application/sql' --data-binary {shlex.quote(BODY)} " if method == 'QUERY' else ''
+ tr.Processes.Default.StartBefore(server)
+ tr.Processes.Default.StartBefore(ts)
+ tr.Processes.Default.Command = (
+ f"curl -s -o /dev/null -w '%{{http_code}}\\n' -X {method} {content}http://127.0.0.1:{ts.Variables.port}/search")
Review Comment:
The test curl invocation has no explicit timeout; if ATS or the origin hangs, this can stall the test run indefinitely. Add a bounded timeout (e.g., curl's max-time and/or connect-timeout) so failures terminate quickly and predictably.
##########
tests/gold_tests/autest-site/ats_replay.test.ext:
##########
@@ -159,9 +159,14 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti
# Configure logging.yaml if specified.
logging_yaml = ats_config.get('logging_yaml')
- if logging_yaml != None:
+ if logging_yaml is not None:
ts.Disk.logging_yaml.AddLines(yaml.dump(logging_yaml).split('\n'))
+ # Configure ip_allow.yaml if specified.
+ ip_allow_yaml = ats_config.get('ip_allow_yaml')
+ if ip_allow_yaml is not None:
+ ts.Disk.ip_allow_yaml.AddLines(yaml.dump(ip_allow_yaml).split('\n'))
Review Comment:
For serializing YAML config content, prefer `yaml.safe_dump(...)` over `yaml.dump(...)` to avoid emitting/accepting unsafe Python-specific tags (even in tests this helps keep output predictable). Also consider using `splitlines()` instead of `split('\\n')` to avoid an extra trailing empty line entry.
##########
tests/gold_tests/query/query_retry_origin.py:
##########
@@ -0,0 +1,82 @@
+'''
+An origin that closes the first connection after reading the request, without
+responding, and answers every later request with a 200.
+'''
+# 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.
+
+import argparse
+import socket
+import sys
+
+
+def read_request(conn: socket.socket) -> tuple[str, bytes]:
+ data = bytearray()
+ while b'\r\n\r\n' not in data:
+ chunk = conn.recv(4096)
+ if not chunk:
+ return '', b''
+ data += chunk
+ head, _, body = data.partition(b'\r\n\r\n')
+ lines = head.decode('latin-1').split('\r\n')
+ method = lines[0].split(' ')[0]
+ length = 0
+ for line in lines[1:]:
+ name, _, value = line.partition(':')
+ if name.strip().lower() == 'content-length':
+ length = int(value.strip())
+ while len(body) < length:
+ chunk = conn.recv(4096)
+ if not chunk:
+ break
+ body += chunk
+ return method, bytes(body[:length])
Review Comment:
In the body-read loop, `body` is a `bytes` object and `body += chunk` repeatedly reallocates/copies, which can become quadratic for larger payloads. Use a `bytearray` (or reuse the existing `data` buffer) for incremental reads and convert/slice once at the end.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "JakeChampion (via GitHub)" <gi...@apache.org>.
JakeChampion commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4259751517
##########
src/proxy/http/HttpTransact.cc:
##########
@@ -5819,9 +5819,9 @@ HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr)
return RequestError_t::UNACCEPTABLE_TE_REQUIRED;
}
- // Require Content-Length/Transfer-Encoding for POST/PUSH/PUT
+ // Require Content-Length/Transfer-Encoding for POST/PUSH/PUT/QUERY
if ((scheme == URL_WKSIDX_HTTP || scheme == URL_WKSIDX_HTTPS) &&
- (method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT) &&
+ (method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT || method == HTTP_WKSIDX_QUERY) &&
Review Comment:
These lists aren't the same set today: request validation and HTTP/2 header framing use POST/PUSH/PUT/QUERY, while Expect: 100-continue handling and the keep_alive_post_out session check use POST/PUT/QUERY without PUSH
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub) via github" <gi...@trafficserver.apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4442129675
##########
tests/uranium_tests/query/test_query_retry.py:
##########
@@ -0,0 +1,128 @@
+# 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.
+"""Verify QUERY content is not lost by retrying after an origin closes."""
+
+from pathlib import Path
+import re
+import shlex
+import sys
+
+import pytest
+
+from tools.uranium.services import ATSFactory, Curl, ServiceFactory, wait_for_metric
+
+from ..h2.origin_test_helpers import configure_h2_origin, finish_origin
+
+BODY = 'SELECT * FROM t WHERE a'
+
+
+@pytest.mark.parametrize('method,buffering', [('GET', False), ('QUERY', True), ('QUERY', False)])
+@pytest.mark.parametrize('failure', ['close', 'tls-read-error', 'tls-handshake-error'])
+def test_query_retry(
+ ats_factory: ATSFactory, services: ServiceFactory, curl: Curl, method: str, buffering: bool, failure: str) -> None:
+ """Retry GET after an origin closes, but never replay QUERY without content.
+
+ :param ats_factory: Proxy factory.
+ :param services: Custom origin factory.
+ :param curl: Transport-aware HTTP client.
+ :param method: Request method; GET is the safe retry control.
+ :param buffering: Whether ATS buffers the request content.
+ :param failure: Origin close, TLS read error, or TLS failure before sending.
Review Comment:
The docstring and parameter descriptions are misleading now that this test covers both GET and QUERY with buffering-dependent behavior. Please update the docstring/`:param` text to describe QUERY’s retry rules (e.g., retry allowed only before sending, and after sending only when the body can be replayed via the not-processed retry path) to avoid future confusion when extending failures/methods.
##########
tests/uranium_tests/query/query_retry_origin.py:
##########
@@ -0,0 +1,125 @@
+'''
+An origin that closes the first connection after reading the request, without
+responding, and answers every later request with a 200.
+'''
+# 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.
+
+import argparse
+from pathlib import Path
+import socket
+import ssl
+import sys
+import time
+
+
+def read_request(conn: socket.socket) -> tuple[str, bytes]:
+ """Read one request and its declared content.
+
+ :param conn: Connected origin socket.
+ """
+ data = bytearray()
+ while b'\r\n\r\n' not in data:
+ chunk = conn.recv(4096)
+ if not chunk:
+ return '', b''
+ data += chunk
+ head, _, rest = data.partition(b'\r\n\r\n')
+ body = bytearray(rest)
+ lines = head.decode('latin-1').split('\r\n')
+ method = lines[0].split(' ')[0]
+ print(f'request headers method={method}', flush=True)
+ length = 0
+ for line in lines[1:]:
+ name, _, value = line.partition(':')
+ if name.strip().lower() == 'content-length':
+ length = int(value.strip())
+ while len(body) < length:
+ chunk = conn.recv(4096)
+ if not chunk:
+ break
+ body += chunk
+ return method, bytes(body[:length])
+
+
+def main() -> int:
+ """Close or corrupt the first origin response, then serve retries."""
+ parser = argparse.ArgumentParser()
+ parser.add_argument('port', type=int)
+ parser.add_argument('--tls', nargs=2, metavar=('CERTIFICATE', 'KEY'))
+ parser.add_argument('--upload-log', type=Path)
+ parser.add_argument('--handshake-error', action='store_true')
+ args = parser.parse_args()
+ context = None
+ if args.tls:
+ context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
+ context.load_cert_chain(*args.tls)
+ context.set_alpn_protocols(['http/1.1'])
+
+ with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as listener:
+ listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+ listener.bind(('127.0.0.1', args.port))
+ listener.listen()
+ attempt = 0
+ handshake_failed = False
+ while True:
+ conn, _ = listener.accept()
+ conn.settimeout(10)
+ if context:
+ if args.handshake_error and not handshake_failed:
+ with conn:
+ if not conn.recv(1, socket.MSG_PEEK):
+ continue
+ conn.sendall(b'not a TLS record\r\n')
+ handshake_failed = True
+ print('TLS handshake failed before request', flush=True)
+ continue
+ try:
+ conn = context.wrap_socket(conn, server_side=True)
+ except ssl.SSLError:
+ conn.close()
+ continue
+ with conn:
+ try:
+ method, body = read_request(conn)
+ except OSError as e:
+ print(f'request read failed: {e}', flush=True)
+ continue
+ if not method:
+ continue
+ attempt += 1
+ text = body.decode('utf-8', errors='replace')
+ print(f'request attempt={attempt} method={method} body={text}', flush=True)
+ if attempt == 1 and not args.handshake_error:
+ if context:
+ if args.upload_log and body:
+ deadline = time.monotonic() + 5
+ while '[tunnel_handler_post_server, VC_EVENT_WRITE_COMPLETE' not in args.upload_log.read_text():
Review Comment:
This loop can raise an exception if `args.upload_log` does not exist yet (or is temporarily unreadable) when the origin reaches this code path (e.g., timing differences between starting ATS and writing the log). Make the polling robust by checking `args.upload_log.exists()` before `read_text()` or catching `FileNotFoundError`/`OSError` and continuing until the deadline.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "JakeChampion (via GitHub)" <gi...@apache.org>.
JakeChampion commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4259731835
##########
tests/gold_tests/query/query_retry_origin.py:
##########
@@ -0,0 +1,82 @@
+'''
+An origin that closes the first connection after reading the request, without
+responding, and answers every later request with a 200.
+'''
+# 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.
+
+import argparse
+import socket
+import sys
+
+
+def read_request(conn: socket.socket) -> tuple[str, bytes]:
+ data = bytearray()
+ while b'\r\n\r\n' not in data:
+ chunk = conn.recv(4096)
+ if not chunk:
+ return '', b''
+ data += chunk
+ head, _, body = data.partition(b'\r\n\r\n')
+ lines = head.decode('latin-1').split('\r\n')
+ method = lines[0].split(' ')[0]
+ length = 0
+ for line in lines[1:]:
+ name, _, value = line.partition(':')
+ if name.strip().lower() == 'content-length':
+ length = int(value.strip())
+ while len(body) < length:
+ chunk = conn.recv(4096)
+ if not chunk:
+ break
+ body += chunk
+ return method, bytes(body[:length])
Review Comment:
body is already a bytearray
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4250123792
##########
src/proxy/http2/Http2ConnectionState.cc:
##########
@@ -2714,7 +2714,8 @@ Http2ConnectionState::send_headers_frame(Http2Stream *stream)
// explicit zero length and no end stream flag, causing the request to
// fail. We emulate chromium behaviour here prevent such RSTs. Transfer-encoding
// implies there’s a body, regardless of whether it is chunked or not.
- bool content_method = method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT;
+ bool content_method =
+ method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT || method == HTTP_WKSIDX_QUERY;
Review Comment:
The “content-carrying methods” list is now updated in multiple places in this PR (HTTP/2 handling, request validity checks, etc.). To reduce the risk of future omissions when adding methods, consider centralizing this check behind a shared helper (e.g., a `HttpTransactHeaders` predicate) and using it consistently instead of repeating method lists.
##########
tests/gold_tests/autest-site/ats_replay.test.ext:
##########
@@ -159,9 +159,14 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti
# Configure logging.yaml if specified.
logging_yaml = ats_config.get('logging_yaml')
- if logging_yaml != None:
+ if logging_yaml is not None:
ts.Disk.logging_yaml.AddLines(yaml.dump(logging_yaml).split('\n'))
+ # Configure ip_allow.yaml if specified.
+ ip_allow_yaml = ats_config.get('ip_allow_yaml')
+ if ip_allow_yaml is not None:
+ ts.Disk.ip_allow_yaml.AddLines(yaml.dump(ip_allow_yaml).split('\n'))
Review Comment:
`yaml.dump()` can emit Python-specific tags and can vary output across PyYAML versions, which can make tests brittle. For generated config files like `ip_allow.yaml`, prefer `yaml.safe_dump(...)` (and, if needed, explicit dump options like `sort_keys=False`) to keep output stable and strictly YAML.
##########
tests/gold_tests/query/query_retry_origin.py:
##########
@@ -0,0 +1,77 @@
+'''
+An origin that closes the first connection after reading the request, without
+responding, and answers every later request with a 200.
+'''
+# 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.
+
+import argparse
+import socket
+import sys
+
+
+def read_request(conn: socket.socket) -> tuple[str, bytes]:
+ data = bytearray()
+ while b'\r\n\r\n' not in data:
+ chunk = conn.recv(4096)
+ if not chunk:
+ return '', b''
+ data += chunk
+ head, _, body = data.partition(b'\r\n\r\n')
+ lines = head.decode('latin-1').split('\r\n')
+ method = lines[0].split(' ')[0]
+ length = 0
+ for line in lines[1:]:
+ name, _, value = line.partition(':')
+ if name.strip().lower() == 'content-length':
+ length = int(value.strip())
+ while len(body) < length:
+ chunk = conn.recv(4096)
+ if not chunk:
+ break
+ body += chunk
+ return method, bytes(body[:length])
Review Comment:
The test origin can hang indefinitely on `accept()`/`recv()` if ATS (or a failing test) opens a connection but never finishes sending headers/body. This can stall CI runs. Consider setting reasonable socket timeouts (e.g., on the accepted `conn`, and optionally on the listener) and handling `socket.timeout` by closing the connection / emitting a diagnostic.
##########
tests/gold_tests/query/query_retry_origin.py:
##########
@@ -0,0 +1,77 @@
+'''
+An origin that closes the first connection after reading the request, without
+responding, and answers every later request with a 200.
+'''
+# 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.
+
+import argparse
+import socket
+import sys
+
+
+def read_request(conn: socket.socket) -> tuple[str, bytes]:
+ data = bytearray()
+ while b'\r\n\r\n' not in data:
+ chunk = conn.recv(4096)
+ if not chunk:
+ return '', b''
+ data += chunk
+ head, _, body = data.partition(b'\r\n\r\n')
+ lines = head.decode('latin-1').split('\r\n')
+ method = lines[0].split(' ')[0]
+ length = 0
+ for line in lines[1:]:
+ name, _, value = line.partition(':')
+ if name.strip().lower() == 'content-length':
+ length = int(value.strip())
+ while len(body) < length:
+ chunk = conn.recv(4096)
+ if not chunk:
+ break
+ body += chunk
+ return method, bytes(body[:length])
+
+
+def main() -> int:
+ parser = argparse.ArgumentParser()
+ parser.add_argument('port', type=int)
+ args = parser.parse_args()
+
+ with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as listener:
+ listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+ listener.bind(('127.0.0.1', args.port))
+ listener.listen()
+ attempt = 0
+ while True:
+ conn, _ = listener.accept()
+ with conn:
+ method, body = read_request(conn)
Review Comment:
The test origin can hang indefinitely on `accept()`/`recv()` if ATS (or a failing test) opens a connection but never finishes sending headers/body. This can stall CI runs. Consider setting reasonable socket timeouts (e.g., on the accepted `conn`, and optionally on the listener) and handling `socket.timeout` by closing the connection / emitting a diagnostic.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4259816222
##########
tests/gold_tests/autest-site/ats_replay.test.ext:
##########
@@ -159,13 +159,18 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti
# Configure logging.yaml if specified.
logging_yaml = ats_config.get('logging_yaml')
- if logging_yaml != None:
- ts.Disk.logging_yaml.AddLines(yaml.dump(logging_yaml).split('\n'))
+ if logging_yaml is not None:
+ ts.Disk.logging_yaml.AddLines(yaml.safe_dump(logging_yaml).split('\n'))
+
+ # Configure ip_allow.yaml if specified.
+ ip_allow_yaml = ats_config.get('ip_allow_yaml')
+ if ip_allow_yaml is not None:
+ ts.Disk.ip_allow_yaml.AddLines(yaml.safe_dump(ip_allow_yaml).split('\n'))
# Configure storage.yaml if specified.
storage_yaml = ats_config.get('storage_yaml')
if storage_yaml is not None:
- ts.Disk.storage_yaml.AddLines(yaml.dump(storage_yaml).split('\n'))
+ ts.Disk.storage_yaml.AddLines(yaml.safe_dump(storage_yaml).split('\n'))
Review Comment:
Using `.split('\\n')` on the YAML string will include a trailing empty element when the dump ends with a newline (common), and it also won’t handle other newline conventions. Prefer `splitlines()` (or `splitlines(False)`) here to avoid adding an extra blank line and to be newline-agnostic.
##########
tests/gold_tests/autest-site/ats_replay.test.ext:
##########
@@ -159,13 +159,18 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti
# Configure logging.yaml if specified.
logging_yaml = ats_config.get('logging_yaml')
- if logging_yaml != None:
- ts.Disk.logging_yaml.AddLines(yaml.dump(logging_yaml).split('\n'))
+ if logging_yaml is not None:
+ ts.Disk.logging_yaml.AddLines(yaml.safe_dump(logging_yaml).split('\n'))
Review Comment:
Using `.split('\\n')` on the YAML string will include a trailing empty element when the dump ends with a newline (common), and it also won’t handle other newline conventions. Prefer `splitlines()` (or `splitlines(False)`) here to avoid adding an extra blank line and to be newline-agnostic.
##########
tests/gold_tests/autest-site/ats_replay.test.ext:
##########
@@ -131,8 +131,8 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti
# Configure sni.yaml if specified.
sni_yaml = ats_config.get('sni_yaml')
- if sni_yaml != None:
- ts.Disk.sni_yaml.AddLines(yaml.dump(sni_yaml).split('\n'))
+ if sni_yaml is not None:
+ ts.Disk.sni_yaml.AddLines(yaml.safe_dump(sni_yaml).split('\n'))
Review Comment:
Using `.split('\\n')` on the YAML string will include a trailing empty element when the dump ends with a newline (common), and it also won’t handle other newline conventions. Prefer `splitlines()` (or `splitlines(False)`) here to avoid adding an extra blank line and to be newline-agnostic.
##########
tests/gold_tests/query/query_retry_origin.py:
##########
@@ -0,0 +1,82 @@
+'''
+An origin that closes the first connection after reading the request, without
+responding, and answers every later request with a 200.
+'''
+# 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.
+
+import argparse
+import socket
+import sys
+
+
+def read_request(conn: socket.socket) -> tuple[str, bytes]:
+ data = bytearray()
+ while b'\r\n\r\n' not in data:
+ chunk = conn.recv(4096)
+ if not chunk:
+ return '', b''
+ data += chunk
+ head, _, body = data.partition(b'\r\n\r\n')
+ lines = head.decode('latin-1').split('\r\n')
+ method = lines[0].split(' ')[0]
+ length = 0
+ for line in lines[1:]:
+ name, _, value = line.partition(':')
+ if name.strip().lower() == 'content-length':
+ length = int(value.strip())
+ while len(body) < length:
+ chunk = conn.recv(4096)
+ if not chunk:
+ break
+ body += chunk
+ return method, bytes(body[:length])
Review Comment:
This loop repeatedly concatenates `bytes` (`body += chunk`), which copies the buffer each iteration and can become quadratic as request bodies grow. Use a `bytearray` accumulator (or append to the existing `data` bytearray and slice) and convert to `bytes` once at the end.
##########
tests/gold_tests/autest-site/ats_replay.test.ext:
##########
@@ -159,13 +159,18 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti
# Configure logging.yaml if specified.
logging_yaml = ats_config.get('logging_yaml')
- if logging_yaml != None:
- ts.Disk.logging_yaml.AddLines(yaml.dump(logging_yaml).split('\n'))
+ if logging_yaml is not None:
+ ts.Disk.logging_yaml.AddLines(yaml.safe_dump(logging_yaml).split('\n'))
+
+ # Configure ip_allow.yaml if specified.
+ ip_allow_yaml = ats_config.get('ip_allow_yaml')
+ if ip_allow_yaml is not None:
+ ts.Disk.ip_allow_yaml.AddLines(yaml.safe_dump(ip_allow_yaml).split('\n'))
Review Comment:
Using `.split('\\n')` on the YAML string will include a trailing empty element when the dump ends with a newline (common), and it also won’t handle other newline conventions. Prefer `splitlines()` (or `splitlines(False)`) here to avoid adding an extra blank line and to be newline-agnostic.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4248767836
##########
tests/gold_tests/query/query-retry.test.py:
##########
@@ -0,0 +1,78 @@
+'''
+Verify QUERY is not retried once its request, content included, has been sent
+to an origin that then closes the connection, whether or not request buffering
+is enabled.
+'''
+# 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.
+
+import re
+import sys
+
+from ports import get_port
+
+Test.Summary = '''
+Verify QUERY is not retried once its request, content included, has been sent
+to an origin that then closes the connection, whether or not request buffering
+is enabled.
+'''
+
+Test.ContinueOnFail = True
+
+BODY = 'SELECT * FROM t WHERE a'
+
+
+def add_scenario(name: str, method: str, buffering: bool) -> None:
+ tr = Test.AddTestRun(f'{method} after the origin closes, request buffering {"enabled" if buffering else "disabled"}')
+ tr.Setup.Copy('query_retry_origin.py')
+
+ server = Test.Processes.Process(f'origin-{name}')
+ server_port = get_port(server, 'http_port')
+ server.Command = f'{sys.executable} {tr.RunDirectory}/query_retry_origin.py {server_port}'
+ server.Ready = When.PortOpen(server_port)
+
+ ts = Test.MakeATSProcess(f'ts-{name}')
+ ts.Disk.records_config.update(
+ {
+ 'proxy.config.http.request_buffer_enabled': 1 if buffering else 0,
+ 'proxy.config.http.connect_attempts_max_retries': 3,
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http_trans',
+ })
+ ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{server_port}/')
+
+ content = f"-H 'Content-Type: application/sql' --data-binary '{BODY}' " if method == 'QUERY' else ''
+ tr.Processes.Default.StartBefore(server)
+ tr.Processes.Default.StartBefore(ts)
+ tr.Processes.Default.Command = (
+ f"curl -s -o /dev/null -w '%{{http_code}}\\n' -X {method} {content}http://127.0.0.1:{ts.Variables.port}/search")
+ tr.Processes.Default.ReturnCode = 0
+
+ if method == 'QUERY':
+ server.Streams.stdout += Testers.ContainsExpression(
+ f'request attempt=1 method=QUERY body={re.escape(BODY)}', 'The origin must receive the QUERY and its content.')
+ server.Streams.stdout += Testers.ExcludesExpression('request attempt=2', 'ATS must not retry a QUERY after sending it.')
Review Comment:
The exclude pattern `'request attempt=2'` can also match later attempts like `attempt=20` because it is not anchored/bounded. Using a more specific pattern (e.g., anchoring to start-of-line and/or adding a word boundary) would make the assertion more robust and prevent accidental false failures if the test ever evolves to generate higher attempt counts.
##########
src/proxy/http/HttpTransact.cc:
##########
@@ -5819,9 +5819,9 @@ HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr)
return RequestError_t::UNACCEPTABLE_TE_REQUIRED;
}
- // Require Content-Length/Transfer-Encoding for POST/PUSH/PUT
+ // Require Content-Length/Transfer-Encoding for POST/PUSH/PUT/QUERY
if ((scheme == URL_WKSIDX_HTTP || scheme == URL_WKSIDX_HTTPS) &&
- (method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT) &&
+ (method == HTTP_WKSIDX_POST || method == HTTP_WKSIDX_PUSH || method == HTTP_WKSIDX_PUT || method == HTTP_WKSIDX_QUERY) &&
s->client_info.transfer_encoding != TransferEncoding_t::CHUNKED) {
Review Comment:
The current validity check makes `QUERY` require `Content-Length` or chunked framing even when no request body is sent, which leads to `411` for a body-less `QUERY`. The draft’s intent is generally to *allow* a body on a safe method, not necessarily to *require* one; rejecting a payload-free `QUERY` may reduce interoperability with clients that omit `Content-Length: 0`. Consider relaxing this for `QUERY` (treat missing `Content-Length` and non-chunked `Transfer-Encoding` as \"no request body\") while still enforcing proper framing when a body is actually present.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] http: recognize QUERY as a well-known method [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13820:
URL: https://github.com/apache/trafficserver/pull/13820#discussion_r4249032503
##########
src/proxy/http/HttpTransact.cc:
##########
@@ -6945,10 +6947,12 @@ HttpTransact::is_request_retryable(State *s)
return false;
}
- // If safe requests are retryable, it should be safe to retry safe requests irrespective of bytes sent or connection state
- // according to RFC the following methods are safe (https://tools.ietf.org/html/rfc7231#section-4.2.1)
+ // Safe methods (RFC 9110, section 9.2.1) can be retried irrespective of bytes sent or connection state. QUERY
+ // (draft-ietf-httpbis-safe-method-w-body) is safe too, but its content has already been sent and cannot be replayed
+ // here, so once its request has gone out it is treated like an unsafe method. A buffered QUERY can still be retried
+ // when the origin reports it was not processed, see HttpSM::prepare_for_origin_retry.
// Otherwise, if there was no error establishing the connection (and we sent bytes)-- we cannot retry
- if (!HttpTransactHeaders::is_method_safe(s->method) && s->current.state != CONNECTION_ERROR &&
+ if ((!HttpTransactHeaders::is_method_safe(s->method) || s->method == HTTP_WKSIDX_QUERY) && s->current.state != CONNECTION_ERROR &&
s->state_machine->server_request_hdr_bytes > 0) {
return false;
}
Review Comment:
The comment says QUERY is treated as unsafe “once its request has gone out” because its *content* has been sent and can’t be replayed, but the guard uses `server_request_hdr_bytes > 0` (headers only). This will also block retries in cases where headers were sent but the request body was not (or not fully) transmitted. Consider keying the QUERY special-case on body bytes sent / request content send completion (and/or an explicit “request body replayable/buffered” signal) rather than header bytes, so the condition matches the stated semantics.
##########
tests/gold_tests/query/replay/query-remap-acl.replay.yaml:
##########
@@ -0,0 +1,90 @@
+# 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 a remap.config ACL can deny the QUERY method'
+
+ server:
+ name: 'server'
+
+ client:
+ name: 'client'
+
+ ats:
+ name: 'ts'
+ process_config:
+ enable_cache: false
+ records_config:
+ proxy.config.diags.debug.enabled: 1
+ proxy.config.diags.debug.tags: 'url_rewrite|ip_allow'
+ remap_config:
+ - 'map http://example.com/ http://127.0.0.1:{SERVER_HTTP_PORT}/ @action=deny @method=QUERY'
+
+sessions:
+- transactions:
+
+ - client-request:
+ method: QUERY
+ url: /denied
+ version: '1.1'
+ headers:
+ fields:
+ - [Host, example.com]
+ - [Content-Type, application/sql]
+ - [Content-Length, "8"]
+ - [uuid, remap-acl-denied]
+ content:
+ data: "select-a"
+
+ proxy-request:
+ expect: absent
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [Content-Length, "0"]
+
Review Comment:
This transaction asserts `proxy-request: expect: absent` (origin should never be contacted) but still defines a `server-response`. If the replay harness ignores `server-response` when the proxy-request is absent, consider removing the `server-response` block to make the test intent clearer and avoid implying origin involvement in an ACL-denied flow. (If the harness requires it, a brief comment explaining that would also help.)
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org