This is an automated email from the ASF dual-hosted git repository.
serrislew pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 2441fa7114 Cripts: make URL::Query copyable via deep-copy of _state
(#13269)
2441fa7114 is described below
commit 2441fa711474b334e66b376e78b1a333a5e6041b
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)
---
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 bffe3c4c87..6c93336547 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)
@@ -59,6 +71,7 @@ ssl_multicert:
""".split("\n"))
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(
{
@@ -72,6 +85,8 @@ ssl_multicert:
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.')
@@ -91,8 +106,35 @@ ssl_multicert:
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
