This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.2.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 2b0ccc94ad041cfb0d6d28c7b3fb610a4a3f2671 Author: Serris Santos <[email protected]> AuthorDate: Thu Jun 18 12:31:47 2026 -0700 Cripts: make URL::Query copyable via deep-copy of _state (#13269) (cherry picked from commit 2441fa711474b334e66b376e78b1a333a5e6041b) --- include/cripts/Urls.hpp | 38 +++++++++++++++ src/cripts/tests/query_test.cc | 15 ++++++ tests/gold_tests/cripts/cripts.test.py | 44 +++++++++++++++++- tests/gold_tests/cripts/files/query_copy.cript | 64 ++++++++++++++++++++++++++ tests/gold_tests/cripts/gold/basic_cript.gold | 1 + tests/gold_tests/cripts/gold/certs_cript.gold | 1 + 6 files changed, 162 insertions(+), 1 deletion(-) diff --git a/include/cripts/Urls.hpp b/include/cripts/Urls.hpp index 73580aa068..3ed143bae2 100644 --- a/include/cripts/Urls.hpp +++ b/include/cripts/Urls.hpp @@ -335,6 +335,25 @@ public: using Component::Component; + // _state is deep-copied, but the segment `string_view`s (and Component::_owner) still + // reference the source URL. The copy's views dangle if the source URL's path is rewritten, + // and Flush()/operator= on the copy write back to the source URL via TSUrlPathSet. + Path(const self_type &o) : Component(o), _state(o._state ? std::make_unique<State>(*o._state) : nullptr) {} + + self_type & + operator=(const self_type &o) + { + if (this != &o) { + auto new_state = o._state ? std::make_unique<State>(*o._state) : nullptr; + Component::operator=(o); + _state = std::move(new_state); + } + return *this; + } + + Path(self_type &&) = default; + self_type &operator=(self_type &&) = default; + void Reset() override; cripts::string_view GetSV() override; @@ -485,6 +504,25 @@ public: _state->standalone = true; } + // _state is deep-copied, but the parameter `string_view`s (and Component::_owner) still + // reference the source URL. The copy's views dangle if the source URL's query is rewritten, + // and Flush()/operator= on the copy write back to the source URL via TSUrlHttpQuerySet. + Query(const self_type &o) : Component(o), _state(o._state ? std::make_unique<State>(*o._state) : nullptr) {} + + self_type & + operator=(const self_type &o) + { + if (this != &o) { + auto new_state = o._state ? std::make_unique<State>(*o._state) : nullptr; + Component::operator=(o); + _state = std::move(new_state); + } + return *this; + } + + Query(self_type &&) = default; + self_type &operator=(self_type &&) = default; + void Reset() override; cripts::string_view GetSV() override; diff --git a/src/cripts/tests/query_test.cc b/src/cripts/tests/query_test.cc index 8c29598201..83500dd23a 100644 --- a/src/cripts/tests/query_test.cc +++ b/src/cripts/tests/query_test.cc @@ -31,6 +31,21 @@ do_remap() { borrow url = Client::URL::get(); + // Force the live Query to parse so _state is populated; + // otherwise copies won't exercise the deep-copy path. + url.query["foo"]; + + // Copy-construct Query to exercise the explicit copy ctor (Query owns a std::unique_ptr<State>). + // Note: this remains tied to the same underlying URL; it is not an independent snapshot. + cripts::Url::Query query_copy = url.query; + query_copy.Erase("foo"); + + // Print the copy before mutating the live URL. After url.query.Flush() below, + // url.query is Reset()'d and TSUrlHttpQuerySet may invalidate the TS-owned + // buffer that query_copy's State string_views point into. + CDebug("Live: {}", url.query); + CDebug("Copy (foo erased): {}", query_copy); + switch (AsInteger(instance.data[0])) { case 0: url.query.Erase({"foo", "bar"}, true); diff --git a/tests/gold_tests/cripts/cripts.test.py b/tests/gold_tests/cripts/cripts.test.py index ce46154ee5..501a6aac17 100644 --- a/tests/gold_tests/cripts/cripts.test.py +++ b/tests/gold_tests/cripts/cripts.test.py @@ -40,12 +40,24 @@ class CriptsBasicTest: request_header = {"headers": "GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n", "timestamp": "1469733493.993", "body": ""} response_header = { - "headers": "HTTP/1.1 200 OK\r\responseHeader: unchanged\r\n\r\n", + "headers": "HTTP/1.1 200 OK\r\nresponseHeader: unchanged\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": "" } self.server.addResponse("sessionfile.log", request_header, response_header) + query_request_header = { + "headers": "GET /path/to/resource?foo=1&bar=2&baz=3 HTTP/1.1\r\nHost: does.not.matter\r\n\r\n", + "timestamp": "1469733493.993", + "body": "" + } + query_response_header = { + "headers": "HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + "timestamp": "1469733493.993", + "body": "" + } + self.server.addResponse("sessionfile.log", query_request_header, query_response_header) + def setUpTS(self): self.ts = Test.MakeATSProcess("ts_in", enable_tls=True, enable_cache=False, enable_cripts=True) @@ -53,6 +65,7 @@ class CriptsBasicTest: self.ts.Disk.ssl_multicert_config.AddLine("dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key") self.ts.Setup.Copy('files/basic.cript', self.ts.Variables.CONFIGDIR) + self.ts.Setup.Copy('files/query_copy.cript', self.ts.Variables.CONFIGDIR) self.ts.Disk.records_config.update( { @@ -66,6 +79,8 @@ class CriptsBasicTest: self.ts.Disk.remap_config.AddLine( f'map https://www.example.com:{self.ts.Variables.ssl_port} http://127.0.0.1:{self.server.Variables.Port} @plugin=basic.cript' ) + self.ts.Disk.remap_config.AddLine( + f'map http://query.example.com http://127.0.0.1:{self.server.Variables.Port} @plugin=query_copy.cript') def runHeaderTest(self): tr = Test.AddTestRun('Exercise traffic through cripts.') @@ -85,8 +100,35 @@ class CriptsBasicTest: tr.Processes.Default.Streams.stderr = "gold/certs_cript.gold" tr.StillRunningAfter = self.server + def runQueryCopyTest(self): + tr = Test.AddTestRun('Exercise Query/Path copy ctor / copy-assign in cripts.') + tr.MakeCurlCommand( + f'-v -H "Host: query.example.com" "http://127.0.0.1:{self.ts.Variables.port}/path/to/resource?foo=1&bar=2&baz=3";', + ts=self.ts) + tr.Processes.Default.ReturnCode = 0 + # Live URL is untouched: original query is preserved. + tr.Processes.Default.Streams.stderr = Testers.ContainsExpression( + r"X-Original-Query: foo=1&bar=2&baz=3", "Original query must round-trip unchanged") + # Mutating the copy (Erase "foo") must not have leaked into the original. + tr.Processes.Default.Streams.stderr += Testers.ContainsExpression( + r"X-Copy-Query: bar=2&baz=3", "Erase on the copy must drop foo without affecting the original") + # Copy-assignment path produces a snapshot equal to the original. + tr.Processes.Default.Streams.stderr += Testers.ContainsExpression( + r"X-Assigned-Query: foo=1&bar=2&baz=3", "Copy-assigned query must equal the original") + # Live URL path is untouched: original path is preserved. + tr.Processes.Default.Streams.stderr += Testers.ContainsExpression( + r"X-Original-Path: path/to/resource", "Original path must round-trip unchanged") + # Mutating the copy (segment[0] = "edited") must not have leaked into the original. + tr.Processes.Default.Streams.stderr += Testers.ContainsExpression( + r"X-Copy-Path: edited/to/resource", "Segment edit on the copy must not affect the original") + # Copy-assignment path produces a snapshot equal to the original. + tr.Processes.Default.Streams.stderr += Testers.ContainsExpression( + r"X-Assigned-Path: path/to/resource", "Copy-assigned path must equal the original") + tr.StillRunningAfter = self.server + def run(self): self.runHeaderTest() + self.runQueryCopyTest() if not Condition.CurlUsingUnixDomainSocket(): self.runCertsTest() diff --git a/tests/gold_tests/cripts/files/query_copy.cript b/tests/gold_tests/cripts/files/query_copy.cript new file mode 100644 index 0000000000..3121a6c4fd --- /dev/null +++ b/tests/gold_tests/cripts/files/query_copy.cript @@ -0,0 +1,64 @@ +/* + 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. +*/ + +#include <cripts/Preamble.hpp> + +do_remap() +{ + borrow url = cripts::Client::URL::Get(); + + // Force the live Query / Path to parse so _state is populated; otherwise the + // copies below take the nullptr branch and the deep-copy logic isn't exercised. + url.query["foo"]; + url.path[0]; + + // Copy-construct to exercise the explicit copy ctor (each owns a std::unique_ptr<State>). + // Note: this remains tied to the same underlying URL; it is not an independent snapshot. + cripts::Url::Query query_copy = url.query; + cripts::Url::Query query_assigned; + cripts::Url::Path path_copy = url.path; + cripts::Url::Path path_assigned; + + query_assigned = url.query; + path_assigned = url.path; + + query_copy.Erase("foo"); + path_copy[0] = "edited"; + + // Materialize into owned strings so do_send_response sees stable values. + txn_data[0] = cripts::string(url.query.GetSV()); + txn_data[1] = cripts::string(query_copy.GetSV()); + txn_data[2] = cripts::string(query_assigned.GetSV()); + txn_data[3] = cripts::string(url.path.GetSV()); + txn_data[4] = cripts::string(path_copy.GetSV()); + txn_data[5] = cripts::string(path_assigned.GetSV()); +} + +do_send_response() +{ + borrow resp = cripts::Client::Response::Get(); + + resp["X-Original-Query"] = AsString(txn_data[0]); + resp["X-Copy-Query"] = AsString(txn_data[1]); + resp["X-Assigned-Query"] = AsString(txn_data[2]); + resp["X-Original-Path"] = AsString(txn_data[3]); + resp["X-Copy-Path"] = AsString(txn_data[4]); + resp["X-Assigned-Path"] = AsString(txn_data[5]); +} + +#include <cripts/Epilogue.hpp> diff --git a/tests/gold_tests/cripts/gold/basic_cript.gold b/tests/gold_tests/cripts/gold/basic_cript.gold index f3a6ea87f2..f06580416c 100644 --- a/tests/gold_tests/cripts/gold/basic_cript.gold +++ b/tests/gold_tests/cripts/gold/basic_cript.gold @@ -7,6 +7,7 @@ `` < HTTP/1.1 200 OK < responseHeader: changed +< Content-Length: 0 < Date: `` < Age: `` < Connection: keep-alive diff --git a/tests/gold_tests/cripts/gold/certs_cript.gold b/tests/gold_tests/cripts/gold/certs_cript.gold index bfc31a9e71..7bb8621565 100644 --- a/tests/gold_tests/cripts/gold/certs_cript.gold +++ b/tests/gold_tests/cripts/gold/certs_cript.gold @@ -6,6 +6,7 @@ `` < HTTP/1.1 200 OK < responseHeader: changed +< Content-Length: 0 < Date: `` < Age: `` < Connection: keep-alive
