You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by sh...@apache.org on 2018/01/08 14:16:44 UTC
[trafficserver] branch master updated: fixes parse of redirect uris
missing leading /
This is an automated email from the ASF dual-hosted git repository.
shinrich 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 6017b58 fixes parse of redirect uris missing leading /
6017b58 is described below
commit 6017b58a1db0e305cb206b6cfcbda9ac4fce19b8
Author: Derek Dagit <de...@oath.com>
AuthorDate: Tue Dec 19 19:32:34 2017 +0000
fixes parse of redirect uris missing leading /
RFC7230 § 5.5
These are a variant of origin-form URIs where the origin neglected to begin the path with a / character.
---
proxy/http/HttpSM.cc | 33 ++++++++-
tests/gold_tests/redirect/.gitignore | 1 +
tests/gold_tests/redirect/gold/redirect.gold | 9 ++-
.../gold/{redirect.gold => redirect_post.gold} | 0
tests/gold_tests/redirect/redirect.test.py | 82 ++++++++++++++++------
tests/gold_tests/redirect/redirect_post.test.py | 2 +-
6 files changed, 97 insertions(+), 30 deletions(-)
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index f84f023..1fee0ff 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -7743,14 +7743,14 @@ HttpSM::redirect_request(const char *redirect_url, const int redirect_len)
if (tmpOrigHost) {
memcpy(origHost, tmpOrigHost, origHost_len);
- origHost[origHost_len] = '\0';
+ origHost[std::min(origHost_len, MAXDNAME - 1)] = '\0';
} else {
valid_origHost = false;
}
char *tmpOrigMethod = (char *)t_state.hdr_info.server_request.method_get(&origMethod_len);
if (tmpOrigMethod) {
- memcpy(origMethod, tmpOrigMethod, origMethod_len);
+ memcpy(origMethod, tmpOrigMethod, std::min(origMethod_len, static_cast<int>(sizeof(origMethod))));
} else {
valid_origHost = false;
}
@@ -7772,6 +7772,22 @@ HttpSM::redirect_request(const char *redirect_url, const int redirect_len)
// redirectUrl.user_set(redirect_url, redirect_len);
redirectUrl.parse(redirect_url, redirect_len);
+ {
+ int _scheme_len = -1;
+ int _host_len = -1;
+ if (redirectUrl.scheme_get(&_scheme_len) == nullptr && redirectUrl.host_get(&_host_len) != nullptr && redirect_url[0] != '/') {
+ // RFC7230 § 5.5
+ // The redirect URL lacked a scheme and so it is a relative URL.
+ // The redirect URL did not begin with a slash, so we parsed some or all
+ // of the the relative URI path as the host.
+ // Prepend a slash and parse again.
+ char redirect_url_leading_slash[redirect_len + 1];
+ redirect_url_leading_slash[0] = '/';
+ memcpy(redirect_url_leading_slash + 1, redirect_url, redirect_len + 1);
+ url_nuke_proxy_stuff(redirectUrl.m_url_impl);
+ redirectUrl.parse(redirect_url_leading_slash, redirect_len + 1);
+ }
+ }
// copy the client url to the original url
URL &origUrl = t_state.redirect_info.original_url;
@@ -7821,6 +7837,10 @@ HttpSM::redirect_request(const char *redirect_url, const int redirect_len)
bool noPortInHost = HttpConfig::m_master.redirection_host_no_port;
+ bool isRedirectUrlOriginForm = clientUrl.m_url_impl->m_len_scheme <= 0 && clientUrl.m_url_impl->m_len_user <= 0 &&
+ clientUrl.m_url_impl->m_len_password <= 0 && clientUrl.m_url_impl->m_len_host <= 0 &&
+ clientUrl.m_url_impl->m_len_port <= 0;
+
// check to see if the client request passed a host header, if so copy the host and port from the redirect url and
// make a new host header
if (t_state.hdr_info.client_request.presence(MIME_PRESENCE_HOST)) {
@@ -7893,10 +7913,17 @@ HttpSM::redirect_request(const char *redirect_url, const int redirect_len)
// Cleanup of state etc.
url_nuke_proxy_stuff(clientUrl.m_url_impl);
url_nuke_proxy_stuff(t_state.hdr_info.client_request.m_url_cached.m_url_impl);
- t_state.hdr_info.client_request.method_set(origMethod, origMethod_len);
+ t_state.hdr_info.client_request.method_set(origMethod, std::min(origMethod_len, static_cast<int>(sizeof(origMethod))));
t_state.hdr_info.client_request.m_target_cached = false;
t_state.hdr_info.server_request.m_target_cached = false;
clientUrl.scheme_set(scheme_str, scheme_len);
+ if (isRedirectUrlOriginForm) {
+ // build the rest of the effictive URL: the authority part
+ clientUrl.user_set(origUrl.m_url_impl->m_ptr_user, origUrl.m_url_impl->m_len_user);
+ clientUrl.password_set(origUrl.m_url_impl->m_ptr_password, origUrl.m_url_impl->m_len_password);
+ clientUrl.host_set(origUrl.m_url_impl->m_ptr_host, origUrl.m_url_impl->m_len_host);
+ clientUrl.port_set(origUrl.port_get());
+ }
} else {
LhostError:
// the server request didn't have a host, so remove it from the headers
diff --git a/tests/gold_tests/redirect/.gitignore b/tests/gold_tests/redirect/.gitignore
new file mode 100644
index 0000000..90b27be
--- /dev/null
+++ b/tests/gold_tests/redirect/.gitignore
@@ -0,0 +1 @@
+generated_test_data/
diff --git a/tests/gold_tests/redirect/gold/redirect.gold b/tests/gold_tests/redirect/gold/redirect.gold
index 3738278..d6616ba 100644
--- a/tests/gold_tests/redirect/gold/redirect.gold
+++ b/tests/gold_tests/redirect/gold/redirect.gold
@@ -1,5 +1,4 @@
-HTTP/1.1 204 No Content
-Date: ``
-Age: ``
-Connection: keep-alive
-Server: ATS/``
+HTTP/1.1 204 No Content
+Age: ``
+Connection: keep-alive
+
diff --git a/tests/gold_tests/redirect/gold/redirect.gold b/tests/gold_tests/redirect/gold/redirect_post.gold
similarity index 100%
copy from tests/gold_tests/redirect/gold/redirect.gold
copy to tests/gold_tests/redirect/gold/redirect_post.gold
diff --git a/tests/gold_tests/redirect/redirect.test.py b/tests/gold_tests/redirect/redirect.test.py
index 18e0011..8f51935 100644
--- a/tests/gold_tests/redirect/redirect.test.py
+++ b/tests/gold_tests/redirect/redirect.test.py
@@ -1,4 +1,5 @@
'''
+Test redirection
'''
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
@@ -18,15 +19,9 @@
import os
Test.Summary = '''
-Test basic redirection
+Test redirection
'''
-MAX_REDIRECT = 99
-
-Test.SkipUnless(
- Condition.HasProgram("curl", "Curl need to be installed on system for this test to work")
-)
-
Test.ContinueOnFail = True
ts = Test.MakeATSProcess("ts")
@@ -35,37 +30,82 @@ dest_serv = Test.MakeOriginServer("dest_server")
dns = Test.MakeDNServer("dns")
ts.Disk.records_config.update({
- 'proxy.config.http.number_of_redirections': MAX_REDIRECT,
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http|dns|redirect',
+ 'proxy.config.http.redirection_enabled': 1,
+ 'proxy.config.http.number_of_redirections': 1,
'proxy.config.http.cache.http': 0,
'proxy.config.dns.nameservers': '127.0.0.1:{0}'.format(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
})
+Test.Setup.Copy(os.path.join(Test.Variables.AtsTestToolsDir,'tcp_client.py'))
+
redirect_request_header = {"headers": "GET /redirect HTTP/1.1\r\nHost: *\r\n\r\n", "timestamp": "5678", "body": ""}
redirect_response_header = {"headers": "HTTP/1.1 302 Found\r\nLocation: http://127.0.0.1:{0}/redirectDest\r\n\r\n".format(
dest_serv.Variables.Port), "timestamp": "5678", "body": ""}
-dest_request_header = {"headers": "GET /redirectDest HTTP/1.1\r\nHost: *\r\n\r\n", "timestamp": "11", "body": ""}
-dest_response_header = {"headers": "HTTP/1.1 204 No Content\r\n\r\n", "timestamp": "22", "body": ""}
-
redirect_serv.addResponse("sessionfile.log", redirect_request_header, redirect_response_header)
-dest_serv.addResponse("sessionfile.log", dest_request_header, dest_response_header)
+dest_request_header = {"headers": "GET /redirectDest HTTP/1.1\r\nHost: *\r\n\r\n", "timestamp": "11", "body": ""}
+dest_response_header = {"headers": "HTTP/1.1 204 No Content\r\nConnection: close\r\n\r\n", "timestamp": "22", "body": ""}
+dest_serv.addResponse("sessionfile.log", dest_request_header, dest_response_header)
-dns.addRecords(records={"iwillredirect.com.": ["127.0.0.1"]})
-# dns.addRecords(jsonFile="zone.json")
+dns.addRecords(records={"iwillredirect.test.": ["127.0.0.1"]})
-# if we don't disable remap_required, we can also just remap a domain to the domain recognized by DNS
-# ts.Disk.remap_config.AddLine(
-# 'map http://example.com http://iwillredirect.com:{1}/redirect'.format(redirect_serv.Variables.Port)
-# )
+data_dirname = 'generated_test_data'
+data_path = os.path.join(Test.TestDirectory, data_dirname)
+os.makedirs(data_path, exist_ok=True)
-tr = Test.AddTestRun()
-tr.Processes.Default.Command = 'curl -i --proxy 127.0.0.1:{0} "http://iwillredirect.com:{1}/redirect"'.format(
- ts.Variables.port, redirect_serv.Variables.Port)
+# Here and below: spaces are deliberately omitted from the test run names because autest creates directories using these names.
+tr = Test.AddTestRun("FollowsRedirectWithAbsoluteLocationURI")
+# Here and below: because autest's Copy does not behave like standard cp, it's easiest to write all of our files out and copy last.
+with open(os.path.join(data_path, tr.Name), 'w') as f:
+ f.write('GET /redirect HTTP/1.1\r\nHost: iwillredirect.test:{port}\r\n\r\n'.format(port=redirect_serv.Variables.Port))
+tr.Processes.Default.Command = "python tcp_client.py 127.0.0.1 {0} {1} | egrep -v '^(Date: |Server: ATS/)'".format(ts.Variables.port, os.path.join(data_dirname, tr.Name))
tr.Processes.Default.StartBefore(ts)
tr.Processes.Default.StartBefore(redirect_serv)
tr.Processes.Default.StartBefore(dest_serv)
tr.Processes.Default.StartBefore(dns)
tr.Processes.Default.Streams.stdout = "gold/redirect.gold"
tr.Processes.Default.ReturnCode = 0
+
+
+
+redirect_request_header = {"headers": "GET /redirect-relative-path HTTP/1.1\r\nHost: *\r\n\r\n", "timestamp": "5678", "body": ""}
+redirect_response_header = {"headers": "HTTP/1.1 302 Found\r\nLocation: /redirect\r\n\r\n", "timestamp": "5678", "body": ""}
+redirect_serv.addResponse("sessionfile.log", redirect_request_header, redirect_response_header)
+
+redirect_request_header = {"headers": "GET /redirect HTTP/1.1\r\nHost: *\r\n\r\n", "timestamp": "5678", "body": ""}
+redirect_response_header = {"headers": "HTTP/1.1 204 No Content\r\nConnection: close\r\n\r\n", "timestamp": "22", "body": ""}
+redirect_serv.addResponse("sessionfile.log", redirect_request_header, redirect_response_header)
+
+tr = Test.AddTestRun("FollowsRedirectWithRelativeLocationURI")
+with open(os.path.join(data_path, tr.Name), 'w') as f:
+ f.write('GET /redirect-relative-path HTTP/1.1\r\nHost: iwillredirect.test:{port}\r\n\r\n'.format(port=redirect_serv.Variables.Port))
+tr.Processes.Default.Command = "python tcp_client.py 127.0.0.1 {0} {1} | egrep -v '^(Date: |Server: ATS/)'".format(ts.Variables.port, os.path.join(data_dirname, tr.Name))
+tr.StillRunningAfter = ts
+tr.StillRunningAfter = redirect_serv
+tr.StillRunningAfter = dest_serv
+tr.StillRunningAfter = dns
+tr.Processes.Default.Streams.stdout = "gold/redirect.gold"
+tr.Processes.Default.ReturnCode = 0
+
+
+
+redirect_request_header = {"headers": "GET /redirect-relative-path-no-leading-slash HTTP/1.1\r\nHost: *\r\n\r\n", "timestamp": "5678", "body": ""}
+redirect_response_header = {"headers": "HTTP/1.1 302 Found\r\nLocation: redirect\r\n\r\n", "timestamp": "5678", "body": ""}
+redirect_serv.addResponse("sessionfile.log", redirect_request_header, redirect_response_header)
+
+tr = Test.AddTestRun("FollowsRedirectWithRelativeLocationURIMissingLeadingSlash")
+with open(os.path.join(data_path, tr.Name), 'w') as f:
+ f.write('GET /redirect-relative-path-no-leading-slash HTTP/1.1\r\nHost: iwillredirect.test:{port}\r\n\r\n'.format(port=redirect_serv.Variables.Port))
+tr.Processes.Default.Command = "python tcp_client.py 127.0.0.1 {0} {1} | egrep -v '^(Date: |Server: ATS/)'".format(ts.Variables.port, os.path.join(data_dirname, tr.Name))
+tr.StillRunningAfter = ts
+tr.StillRunningAfter = redirect_serv
+tr.StillRunningAfter = dest_serv
+tr.StillRunningAfter = dns
+tr.Processes.Default.Streams.stdout = "gold/redirect.gold"
+tr.Processes.Default.ReturnCode = 0
+
+Test.Setup.Copy(data_path)
diff --git a/tests/gold_tests/redirect/redirect_post.test.py b/tests/gold_tests/redirect/redirect_post.test.py
index 50843ce..f23d900 100644
--- a/tests/gold_tests/redirect/redirect_post.test.py
+++ b/tests/gold_tests/redirect/redirect_post.test.py
@@ -68,5 +68,5 @@ tr.Processes.Default.StartBefore(ts)
tr.Processes.Default.StartBefore(redirect_serv1)
tr.Processes.Default.StartBefore(redirect_serv2)
tr.Processes.Default.StartBefore(dest_serv)
-tr.Processes.Default.Streams.stdout = "gold/redirect.gold"
+tr.Processes.Default.Streams.stdout = "gold/redirect_post.gold"
tr.Processes.Default.ReturnCode = 0
--
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].