You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2022/12/14 23:28:58 UTC
[trafficserver] branch 9.2.x updated: Add autest to reproduce the conditional get cache body drain issue (#9244)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/9.2.x by this push:
new 0d49c8f43 Add autest to reproduce the conditional get cache body drain issue (#9244)
0d49c8f43 is described below
commit 0d49c8f43df02e04253768200c973b02d48fc975
Author: Chris McFarlen <ch...@mcfarlen.us>
AuthorDate: Tue Dec 13 17:28:58 2022 -0600
Add autest to reproduce the conditional get cache body drain issue (#9244)
Add recommended fix to drain request body on cache noop action
Co-authored-by: Chris McFarlen <cm...@apple.com>
(cherry picked from commit f51f9df4f279754597fb84b8e222abf86bb7c0b6)
---
proxy/http/HttpSM.cc | 3 +
tests/gold_tests/cache/conditional-get-hit.test.py | 40 +++++++++
.../cache/replay/conditional-get-cache-hit.yaml | 99 ++++++++++++++++++++++
3 files changed, 142 insertions(+)
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index cb5cc1a46..226baf3da 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -7822,6 +7822,9 @@ HttpSM::set_next_state()
if (server_entry != nullptr && server_entry->in_tunnel == false) {
release_server_session();
}
+
+ do_drain_request_body(t_state.hdr_info.client_response);
+
// If we're in state SEND_API_RESPONSE_HDR, it means functions
// registered to hook SEND_RESPONSE_HDR have already been called. So we do not
// need to call do_api_callout. Otherwise TS loops infinitely in this state !
diff --git a/tests/gold_tests/cache/conditional-get-hit.test.py b/tests/gold_tests/cache/conditional-get-hit.test.py
new file mode 100644
index 000000000..f392b6a0e
--- /dev/null
+++ b/tests/gold_tests/cache/conditional-get-hit.test.py
@@ -0,0 +1,40 @@
+'''
+Test that conditional GETs with body don't interfere with subsequent request
+'''
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+Test.Summary = '''
+Test conditional get with body drains the body from client"
+'''
+
+ts = Test.MakeATSProcess("ts-conditional-get-caching")
+ts.Disk.records_config.update({
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http|cache',
+
+ 'proxy.config.http.cache.max_stale_age': 6,
+})
+tr = Test.AddTestRun("Verify conditional get with cache hit drain client body")
+replay_file = "replay/conditional-get-cache-hit.yaml"
+server = tr.AddVerifierServerProcess("server1", replay_file)
+server_port = server.Variables.http_port
+tr.AddVerifierClientProcess("client1", replay_file, http_ports=[ts.Variables.port])
+ts.Disk.remap_config.AddLine(
+ 'map / http://127.0.0.1:{0}'.format(server_port)
+)
+tr.Processes.Default.StartBefore(ts)
+tr.StillRunningAfter = ts
diff --git a/tests/gold_tests/cache/replay/conditional-get-cache-hit.yaml b/tests/gold_tests/cache/replay/conditional-get-cache-hit.yaml
new file mode 100644
index 000000000..a1d838f5d
--- /dev/null
+++ b/tests/gold_tests/cache/replay/conditional-get-cache-hit.yaml
@@ -0,0 +1,99 @@
+# 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"
+
+sessions:
+- transactions:
+ # Cache a request
+ - client-request:
+ method: "GET"
+ version: "1.1"
+ url: /path/request
+ headers:
+ fields:
+ - [ uuid, 1]
+ - [ Host, example.com ]
+ - [ Accept, "*/*" ]
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ Content-Length, 4 ]
+ - [ Cache-Control, max-age=10 ]
+ - [ ETag, "42" ]
+ content:
+ encoding: plain
+ data: "good"
+
+
+ proxy-response:
+ status: 200
+
+ # conditionally get it but with a body
+ - client-request:
+ method: "GET"
+ version: "1.1"
+ url: /path/request
+ headers:
+ fields:
+ - [ uuid, 2 ]
+ - [ Host, example.com ]
+ - [ Accept, "*/*" ]
+ - [ If-None-Match, "42" ]
+ - [ Content-Length, 1 ]
+ content:
+ encoding: plain
+ data: "\n"
+
+ server-response:
+ status: 500
+ reason: ERR
+
+ # Since there is a cached ETag of 42, ATS should reply with a 304 as a proxy and
+ # the server should not get the request.
+ proxy-response:
+ status: 304
+
+
+ # Re-request the object and make sure the previous request's body does not
+ # confuse the parsing mechanism for this next request.
+ - client-request:
+ method: "GET"
+ version: "1.1"
+ url: /path/request
+ headers:
+ fields:
+ - [ uuid, 3 ]
+ - [ Host, example.com ]
+ - [ Accept, "*/*" ]
+
+ # ATS should reply with the cached resource as a proxy and the origin
+ # should not receive the request.
+ server-response:
+ status: 500
+ reason: NOTUSED
+
+ proxy-response:
+ status: 200
+ reason: OK
+ content:
+ verify: {value: "good", as: equal}
+
+