Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions doc/admin-guide/plugins/header_rewrite.en.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
~~~~~~~~~~
Expand Down
51 changes: 9 additions & 42 deletions plugins/header_rewrite/operators.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the only 3 hooks on which this operator actually works currently. (By default, without a hook condition, operators execute on the READ_RESPONSE hook.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to update the documentation?

}

void
EditRedirectResponse(TSHttpTxn txnp, const std::string &location, TSHttpStatus status, TSMBuffer bufp, TSMLoc hdr_loc)
{
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's no longer used.

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
{
Expand Down Expand Up @@ -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()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is logging the int meaningful? Would it be more readable to have the string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't have any trouble figuring it out, from looking at the code:

#define DERIVE_HOOK_VALUE_FROM_EVENT(COMMON) TS_HTTP_##COMMON##_HOOK = TS_EVENT_HTTP_##COMMON - TS_EVENT_HTTP_READ_REQUEST_HDR

// 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);
}
Expand Down
2 changes: 2 additions & 0 deletions plugins/header_rewrite/operators.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ class OperatorSetRedirect : public Operator
}

protected:
void initialize_hooks() override;

void exec(const Resources &res) const override;

private:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
``
> HEAD / HTTP/1.1
> Host: 127.0.0.1:``
``
< HTTP/1.1 301 Moved Permanently
``
< Location: http://redirect.com/here
``
Original file line number Diff line number Diff line change
@@ -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()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment saying what each test is checking

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()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are both test cases the same?
Add a test case for if the redirect fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes they are the same but meet different ID:REQUEST conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don' t understand. You mean set proxy.config.http.number_of_redirections to non-zero?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean when the redirect returns an error code. Checking what the plugin does.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plugin doesn't do the redirect. If the configured number of redirections is 0 (the default), the client is expected to do redirect to the URL in the response Location header. If redirections is greater than 0, maybe core ATS would do the redirection, but I'm not sure.

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"
Original file line number Diff line number Diff line change
@@ -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