From afeb0dd10455ab9f92c4fa7737027da1eb6f0fed Mon Sep 17 00:00:00 2001 From: Walt Karas Date: Tue, 16 Jul 2024 01:58:43 +0000 Subject: [PATCH] Cleanup of header_rewrite redirect operator when invoked globally. 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 + .../gold/set-redirect-glob.gold | 8 +++ .../header_rewrite_url_glob.test.py | 64 +++++++++++++++++++ .../rules/glob_set_redirect.conf | 24 +++++++ 6 files changed, 111 insertions(+), 44 deletions(-) create mode 100644 tests/gold_tests/pluginTest/header_rewrite/gold/set-redirect-glob.gold create mode 100644 tests/gold_tests/pluginTest/header_rewrite/header_rewrite_url_glob.test.py create mode 100644 tests/gold_tests/pluginTest/header_rewrite/rules/glob_set_redirect.conf diff --git a/doc/admin-guide/plugins/header_rewrite.en.rst b/doc/admin-guide/plugins/header_rewrite.en.rst index 7d971a0f4df..97431776b65 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 ````, and a new location of ````. 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 ````. +location automatically. This operator supports `String concatenations`_ for +````. 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 0bb87f89c32..7729a82a523 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(edata); - - OperatorSetRedirect *osd = static_cast(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(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(_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(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 2c4712a67bb..25debe1d7e3 100644 --- a/plugins/header_rewrite/operators.h +++ b/plugins/header_rewrite/operators.h @@ -159,6 +159,8 @@ class OperatorSetRedirect : public Operator } 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 00000000000..8e7c106ff62 --- /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 00000000000..006acaabb37 --- /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 00000000000..7dfa502693f --- /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