This is an automated email from the ASF dual-hosted git repository.
wkaras 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 94dfd0e0f5 Cleanup of header_rewrite redirect operator when invoked
globally. (#11551)
94dfd0e0f5 is described below
commit 94dfd0e0f571a4e91bb0bcaa49af1c039db94e31
Author: Walt Karas <[email protected]>
AuthorDate: Mon Aug 5 15:03:59 2024 -0400
Cleanup of header_rewrite redirect operator when invoked globally. (#11551)
Also includes new Au test coverage.
---
doc/admin-guide/plugins/header_rewrite.en.rst | 6 +-
plugins/header_rewrite/operators.cc | 51 +++--------------
plugins/header_rewrite/operators.h | 2 +
.../header_rewrite/gold/set-redirect-glob.gold | 8 +++
.../header_rewrite/header_rewrite_url_glob.test.py | 64 ++++++++++++++++++++++
.../header_rewrite/rules/glob_set_redirect.conf | 24 ++++++++
6 files changed, 111 insertions(+), 44 deletions(-)
diff --git a/doc/admin-guide/plugins/header_rewrite.en.rst
b/doc/admin-guide/plugins/header_rewrite.en.rst
index 7d971a0f4d..97431776b6 100644
--- a/doc/admin-guide/plugins/header_rewrite.en.rst
+++ b/doc/admin-guide/plugins/header_rewrite.en.rst
@@ -839,8 +839,10 @@ set-redirect
When invoked, sends a redirect response to the client, with HTTP status
``<code>``, and a new location of ``<destination>``. If the ``QSA`` flag is
enabled, the original query string will be preserved and added to the new
-location automatically. This operator supports
-`String concatenations`_ for ``<destination>``.
+location automatically. This operator supports `String concatenations`_ for
+``<destination>``. This operator can only execute on the
+``READ_RESPONSE_HDR_HOOK`` (the default when the plugin is global), the
+``SEND_RESPONSE_HDR_HOOK``, or the ``REMAP_PSEUDO_HOOK``.
set-status
~~~~~~~~~~
diff --git a/plugins/header_rewrite/operators.cc
b/plugins/header_rewrite/operators.cc
index 0bb87f89c3..7729a82a52 100644
--- a/plugins/header_rewrite/operators.cc
+++ b/plugins/header_rewrite/operators.cc
@@ -488,6 +488,14 @@ OperatorSetRedirect::initialize(Parser &p)
require_resources(RSRC_RESPONSE_STATUS);
}
+void
+OperatorSetRedirect::initialize_hooks()
+{
+ add_allowed_hook(TS_HTTP_READ_RESPONSE_HDR_HOOK);
+ add_allowed_hook(TS_HTTP_SEND_RESPONSE_HDR_HOOK);
+ add_allowed_hook(TS_REMAP_PSEUDO_HOOK);
+}
+
void
EditRedirectResponse(TSHttpTxn txnp, const std::string &location, TSHttpStatus
status, TSMBuffer bufp, TSMLoc hdr_loc)
{
@@ -515,35 +523,6 @@ EditRedirectResponse(TSHttpTxn txnp, const std::string
&location, TSHttpStatus s
TSHttpTxnErrorBodySet(txnp, TSstrdup(msg.c_str()), msg.length(),
TSstrdup("text/html"));
}
-static int
-cont_add_location(TSCont contp, TSEvent event, void *edata)
-{
- TSHttpTxn txnp = static_cast<TSHttpTxn>(edata);
-
- OperatorSetRedirect *osd = static_cast<OperatorSetRedirect
*>(TSContDataGet(contp));
- // Set the new status code and reason.
- TSHttpStatus status = osd->get_status();
- switch (event) {
- case TS_EVENT_HTTP_SEND_RESPONSE_HDR: {
- TSMBuffer bufp;
- TSMLoc hdr_loc;
- if (TSHttpTxnClientRespGet(txnp, &bufp, &hdr_loc) == TS_SUCCESS) {
- EditRedirectResponse(txnp, osd->get_location(), status, bufp, hdr_loc);
- } else {
- Dbg(pi_dbg_ctl, "Could not retrieve the response header");
- }
-
- } break;
-
- case TS_EVENT_HTTP_TXN_CLOSE:
- TSContDestroy(contp);
- break;
- default:
- break;
- }
- return 0;
-}
-
void
OperatorSetRedirect::exec(const Resources &res) const
{
@@ -610,21 +589,9 @@ OperatorSetRedirect::exec(const Resources &res) const
const_cast<Resources &>(res).changed_url = true;
res._rri->redirect = 1;
} else {
+ Dbg(pi_dbg_ctl, "OperatorSetRedirect::exec() hook=%d", int(get_hook()));
// Set the new status code and reason.
TSHttpStatus status = static_cast<TSHttpStatus>(_status.get_int_value());
- switch (get_hook()) {
- case TS_HTTP_PRE_REMAP_HOOK: {
- TSHttpTxnStatusSet(res.txnp, status);
- TSCont contp = TSContCreate(cont_add_location, nullptr);
- TSContDataSet(contp, const_cast<OperatorSetRedirect *>(this));
- TSHttpTxnHookAdd(res.txnp, TS_HTTP_SEND_RESPONSE_HDR_HOOK, contp);
- TSHttpTxnHookAdd(res.txnp, TS_HTTP_TXN_CLOSE_HOOK, contp);
- TSHttpTxnReenable(res.txnp, TS_EVENT_HTTP_CONTINUE);
- return;
- } break;
- default:
- break;
- }
TSHttpHdrStatusSet(res.bufp, res.hdr_loc, status);
EditRedirectResponse(res.txnp, value, status, res.bufp, res.hdr_loc);
}
diff --git a/plugins/header_rewrite/operators.h
b/plugins/header_rewrite/operators.h
index 2c4712a67b..25debe1d7e 100644
--- a/plugins/header_rewrite/operators.h
+++ b/plugins/header_rewrite/operators.h
@@ -159,6 +159,8 @@ public:
}
protected:
+ void initialize_hooks() override;
+
void exec(const Resources &res) const override;
private:
diff --git
a/tests/gold_tests/pluginTest/header_rewrite/gold/set-redirect-glob.gold
b/tests/gold_tests/pluginTest/header_rewrite/gold/set-redirect-glob.gold
new file mode 100644
index 0000000000..8e7c106ff6
--- /dev/null
+++ b/tests/gold_tests/pluginTest/header_rewrite/gold/set-redirect-glob.gold
@@ -0,0 +1,8 @@
+``
+> HEAD / HTTP/1.1
+> Host: 127.0.0.1:``
+``
+< HTTP/1.1 301 Moved Permanently
+``
+< Location: http://redirect.com/here
+``
diff --git
a/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_url_glob.test.py
b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_url_glob.test.py
new file mode 100644
index 0000000000..006acaabb3
--- /dev/null
+++ b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_url_glob.test.py
@@ -0,0 +1,64 @@
+'''
+Test global header_rewrite with set-redirect operator.
+'''
+# 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 global header_rewrite with set-redirect operator.
+'''
+
+Test.ContinueOnFail = True
+ts = Test.MakeATSProcess("ts", block_for_debug=False)
+server = Test.MakeOriginServer("server")
+
+# Configure the server to return 200 responses. The rewrite rules below set a
+# non-200 status, so if curl gets a 200 response something went wrong.
+Test.testName = ""
+request_header = {"headers": "GET / HTTP/1.1\r\nHost: no_path.com\r\n\r\n",
"timestamp": "1469733493.993", "body": ""}
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n",
"timestamp": "1469733493.993", "body": ""}
+server.addResponse("sessionfile.log", request_header, response_header)
+
+ts.Disk.records_config.update(
+ {
+ 'proxy.config.url_remap.remap_required': 0,
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.show_location': 0,
+ 'proxy.config.diags.debug.tags': 'header',
+ })
+ts.Setup.CopyAs('rules/glob_set_redirect.conf', Test.RunDirectory)
+
+ts.Disk.plugin_config.AddLine(f'header_rewrite.so
{Test.RunDirectory}/glob_set_redirect.conf')
+
+# Run operator on Read Response Hdr hook (ID:REQUEST == 0).
+tr = Test.AddTestRun()
+tr.Processes.Default.StartBefore(ts)
+tr.Processes.Default.StartBefore(server)
+tr.Processes.Default.Command = (f'curl --head 127.0.0.1:{ts.Variables.port} -H
"Host: 127.0.0.1:{server.Variables.Port}" --verbose')
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stderr = "gold/set-redirect-glob.gold"
+tr.StillRunningAfter = server
+tr.StillRunningAfter = ts
+ts.Disk.traffic_out.Content = "gold/header_rewrite-tag.gold"
+
+# Run operator on Send Response Hdr hook (ID:REQUEST == 1).
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = (f'curl --head 127.0.0.1:{ts.Variables.port} -H
"Host: 127.0.0.1:{server.Variables.Port}" --verbose')
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stderr = "gold/set-redirect-glob.gold"
+tr.StillRunningAfter = server
+tr.StillRunningAfter = ts
+ts.Disk.traffic_out.Content = "gold/header_rewrite-tag.gold"
diff --git
a/tests/gold_tests/pluginTest/header_rewrite/rules/glob_set_redirect.conf
b/tests/gold_tests/pluginTest/header_rewrite/rules/glob_set_redirect.conf
new file mode 100644
index 0000000000..7dfa502693
--- /dev/null
+++ b/tests/gold_tests/pluginTest/header_rewrite/rules/glob_set_redirect.conf
@@ -0,0 +1,24 @@
+#
+# 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.
+
+cond %{READ_RESPONSE_HDR_HOOK} [AND]
+cond %{ID:REQUEST} =0
+ set-redirect 301 http://redirect.com/here
+
+cond %{SEND_RESPONSE_HDR_HOOK} [AND]
+cond %{ID:REQUEST} =1
+ set-redirect 301 http://redirect.com/here
