diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 950d0ab6ec6..a33fb243135 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -2080,7 +2080,7 @@ HttpSM::state_read_server_response_header(int event, void *data) t_state.api_next_action = HttpTransact::SM_ACTION_API_READ_RESPONSE_HDR; // if exceeded limit deallocate postdata buffers and disable redirection - if (!(enable_redirection && (redirection_tries <= t_state.txn_conf->number_of_redirections))) { + if (!(enable_redirection && (redirection_tries < t_state.txn_conf->number_of_redirections))) { this->disable_redirect(); } @@ -7768,11 +7768,11 @@ HttpSM::set_next_state() void HttpSM::do_redirect() { - SMDebug("http_redirect", "[HttpSM::do_redirect]"); - if (!enable_redirection || redirection_tries > t_state.txn_conf->number_of_redirections) { + SMDebug("http_redirect", "[HttpSM::do_redirect] enable_redirection %u", enable_redirection); + if (!enable_redirection || redirection_tries >= t_state.txn_conf->number_of_redirections) { this->postbuf_clear(); - if (enable_redirection && redirection_tries > t_state.txn_conf->number_of_redirections) { + if (enable_redirection && redirection_tries >= t_state.txn_conf->number_of_redirections) { t_state.squid_codes.subcode = SQUID_SUBCODE_NUM_REDIRECTIONS_EXCEEDED; } @@ -8117,7 +8117,7 @@ HttpSM::is_private() inline bool HttpSM::is_redirect_required() { - bool redirect_required = (enable_redirection && (redirection_tries <= t_state.txn_conf->number_of_redirections) && + bool redirect_required = (enable_redirection && (redirection_tries < t_state.txn_conf->number_of_redirections) && !HttpTransact::is_fresh_cache_hit(t_state.cache_lookup_result)); SMDebug("http_redirect", "is_redirect_required %u", redirect_required); diff --git a/tests/gold_tests/redirect/gold/number_of_redirections_0.gold b/tests/gold_tests/redirect/gold/number_of_redirections_0.gold new file mode 100644 index 00000000000..66e43b735f2 --- /dev/null +++ b/tests/gold_tests/redirect/gold/number_of_redirections_0.gold @@ -0,0 +1,38 @@ +`` +> GET http://a.test/ping HTTP/1.1 +> Host: a.test +> User-Agent: curl/`` +> Accept: */* +> Proxy-Connection: Keep-Alive +> uuid: redirect_test_1 +`` +< HTTP/1.1 302 Redirect +< location: http://b.test:`` +< Date: `` +< Age: `` +< Transfer-Encoding: chunked +< Proxy-Connection: keep-alive +< Server: ATS/`` +`` +< HTTP/1.1 302 Redirect +< location: http://c.test:`` +< Date: `` +< Age: `` +< Transfer-Encoding: chunked +< Proxy-Connection: keep-alive +< Server: ATS/`` +`` +> GET http://c.test:`` +> Host: c.test:`` +> User-Agent: curl/`` +> Accept: */* +> Proxy-Connection: Keep-Alive +> uuid: redirect_test_1 +`` +< HTTP/1.1 200 OK +< date: `` +< Age: `` +< Transfer-Encoding: chunked +< Proxy-Connection: keep-alive +< Server: ATS/`` +`` \ No newline at end of file diff --git a/tests/gold_tests/redirect/gold/number_of_redirections_1.gold b/tests/gold_tests/redirect/gold/number_of_redirections_1.gold new file mode 100644 index 00000000000..032e7e78e24 --- /dev/null +++ b/tests/gold_tests/redirect/gold/number_of_redirections_1.gold @@ -0,0 +1,9 @@ +`` +< HTTP/1.1 302 Redirect +< location: http://c.test:`` +`` +< HTTP/1.1 200 OK +< date: `` +< Age: `` +< Server: ATS/`` +`` diff --git a/tests/gold_tests/redirect/gold/number_of_redirections_2.gold b/tests/gold_tests/redirect/gold/number_of_redirections_2.gold new file mode 100644 index 00000000000..86360afb3cd --- /dev/null +++ b/tests/gold_tests/redirect/gold/number_of_redirections_2.gold @@ -0,0 +1,15 @@ +`` +> GET http://a.test/ping HTTP/1.1 +> Host: a.test +> User-Agent: curl/`` +> Accept: */* +> Proxy-Connection: Keep-Alive +> uuid: redirect_test_1 +`` +< HTTP/1.1 200 OK +< date: `` +< Age: `` +< Transfer-Encoding: chunked +< Proxy-Connection: keep-alive +< Server: ATS/`` +`` \ No newline at end of file diff --git a/tests/gold_tests/redirect/number_of_redirects.test.py b/tests/gold_tests/redirect/number_of_redirects.test.py new file mode 100644 index 00000000000..69f301daf07 --- /dev/null +++ b/tests/gold_tests/redirect/number_of_redirects.test.py @@ -0,0 +1,99 @@ +''' +Specific test for number_of_redirections config. +''' +# 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. + +import os +Test.Summary = ''' +Test redirection/location & number of redirects(number_of_redirections config) +''' + + +class NumberOfRedirectionsTest: + ''' + Handy class to test with number_of_redirections values. Three servers will be created and request + will flow between them. Depending on the configured number_of_redirections, some request may be + followed by ATS and some directly by the client(curl) + ''' + + def __init__(self, testName, numberOfRedirections): + self._numberOfRedirections = numberOfRedirections + self._tr = Test.AddTestRun(testName) + self.setup_ts(f'ts{self._numberOfRedirections}') + self.setup_dns() + self.setup_verifier_servers() + self.add_config() + + def setup_ts(self, name="ts"): + self._ts = Test.MakeATSProcess(name, enable_cache=False) + + def setup_verifier_servers(self): + self._srv3 = Test.MakeVerifierServerProcess(f"srv3_{self._numberOfRedirections}", "replay/redirect_srv3_replay.yaml") + self._srv2 = Test.MakeVerifierServerProcess( + f"srv2_{self._numberOfRedirections}", + "replay/redirect_srv2_replay.yaml", + context={ + "vs_http_port": self._srv3.Variables.http_port}) + self._srv1 = Test.MakeVerifierServerProcess( + f"srv1_{self._numberOfRedirections}", + "replay/redirect_srv1_replay.yaml", + context={ + "vs_http_port": self._srv2.Variables.http_port}) + + def setup_dns(self): + self._dns = Test.MakeDNServer(f"dns_{self._numberOfRedirections}") + self._dns.addRecords(records={"a.test": ["127.0.0.1"]}) + self._dns.addRecords(records={"b.test": ["127.0.0.1"]}) + self._dns.addRecords(records={"c.test": ["127.0.0.1"]}) + + def add_config(self): + self._ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http|dns|redirect|http_redirect', + 'proxy.config.http.number_of_redirections': self._numberOfRedirections, + 'proxy.config.dns.nameservers': f'127.0.0.1:{self._dns.Variables.Port}', + 'proxy.config.dns.resolv_conf': 'NULL', + 'proxy.config.url_remap.remap_required': 0, # need this so the domain gets a chance to be evaluated through DNS + 'proxy.config.http.redirect.actions': 'self:follow', # redirects to self are not followed by default + }) + self._ts.Disk.remap_config.AddLines([ + 'map a.test/ping http://a.test:{0}/'.format(self._srv1.Variables.http_port), + 'map b.test/pong http://b.test:{0}/'.format(self._srv2.Variables.http_port), + 'map c.test/pang http://c.test:{0}/'.format(self._srv3.Variables.http_port), + ]) + + def run(self): + self._tr.Processes.Default.StartBefore(self._srv1) + self._tr.Processes.Default.StartBefore(self._srv2) + self._tr.Processes.Default.StartBefore(self._srv3) + self._tr.Processes.Default.StartBefore(self._dns) + self._tr.Processes.Default.StartBefore(self._ts) + self._tr.Command = "curl -L -vvv a.test/ping --proxy 127.0.0.1:{0} -H 'uuid: redirect_test_1'".format( + self._ts.Variables.port) + self._tr.Processes.Default.Streams.All = f"gold/number_of_redirections_{self._numberOfRedirections}.gold" + self._tr.ReturnCode = 0 + self._tr.StillRunningAfter = self._ts + + +# No redirect, curl will get 302 and do the rest of the redirection. +NumberOfRedirectionsTest("Test number_of_redirections=0", 0).run() +# Single redirect, ATS will follow 1 redirect and the client the last one. +NumberOfRedirectionsTest("Test number_of_redirections=1", 1).run() +# The client will just get 200OK and no redirect will be done by it, TS will follow the two +# 302 Redirect. +NumberOfRedirectionsTest("Test number_of_redirections=2", 2).run() +# If adding more, need to touch the server side as well. It wont be enough. diff --git a/tests/gold_tests/redirect/replay/redirect_srv1_replay.yaml b/tests/gold_tests/redirect/replay/redirect_srv1_replay.yaml new file mode 100644 index 00000000000..6447748e60c --- /dev/null +++ b/tests/gold_tests/redirect/replay/redirect_srv1_replay.yaml @@ -0,0 +1,37 @@ +# 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. + +meta: + version: "1.0" + +sessions: +- transactions: + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /ping + headers: + fields: + - [ uuid, redirect_test_1 ] + - [ Host, a.test ] + + server-response: + status: 302 + reason: Redirect + headers: + fields: + - [ Location, "http://b.test:${vs_http_port}/pong" ] diff --git a/tests/gold_tests/redirect/replay/redirect_srv2_replay.yaml b/tests/gold_tests/redirect/replay/redirect_srv2_replay.yaml new file mode 100644 index 00000000000..6ca671bdad9 --- /dev/null +++ b/tests/gold_tests/redirect/replay/redirect_srv2_replay.yaml @@ -0,0 +1,37 @@ +# 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. + +meta: + version: "1.0" + +sessions: +- transactions: + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /pong + headers: + fields: + - [ uuid, redirect_test_1 ] + - [ Host, b.test ] + + server-response: + status: 302 + reason: Redirect + headers: + fields: + - [ Location, "http://c.test:${vs_http_port}/pang" ] diff --git a/tests/gold_tests/redirect/replay/redirect_srv3_replay.yaml b/tests/gold_tests/redirect/replay/redirect_srv3_replay.yaml new file mode 100644 index 00000000000..f290c64c507 --- /dev/null +++ b/tests/gold_tests/redirect/replay/redirect_srv3_replay.yaml @@ -0,0 +1,37 @@ +# 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. + +meta: + version: "1.0" + +sessions: +- transactions: + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /pang + headers: + fields: + - [ uuid, redirect_test_1 ] + - [ Host, c.test ] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Date, "Sat, 16 Mar 2019 03:11:36 GMT" ]