From 2f09a0f78e7d18d8b83ef2696efa940f74222c4e Mon Sep 17 00:00:00 2001 From: Ross Schlaikjer Date: Tue, 3 Jul 2018 17:01:19 +0000 Subject: Add mixed test group, http -> https redirect test --- lib/inets/test/httpc_SUITE.erl | 73 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 3 deletions(-) diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl index d43e2cc179..0440cb302c 100644 --- a/lib/inets/test/httpc_SUITE.erl +++ b/lib/inets/test/httpc_SUITE.erl @@ -59,7 +59,8 @@ all() -> {group, http_unix_socket}, {group, https}, {group, sim_https}, - {group, misc} + {group, misc}, + {group, sim_mixed} % HTTP and HTTPS sim servers ]. groups() -> @@ -74,7 +75,8 @@ groups() -> {http_unix_socket, [], simulated_unix_socket()}, {https, [], real_requests()}, {sim_https, [], only_simulated()}, - {misc, [], misc()} + {misc, [], misc()}, + {sim_mixed, [], sim_mixed()} ]. real_requests()-> @@ -170,6 +172,11 @@ misc() -> wait_for_whole_response ]. +sim_mixed() -> + [ + redirect_http_to_https + ]. + %%-------------------------------------------------------------------- init_per_suite(Config) -> @@ -195,7 +202,8 @@ init_per_group(misc = Group, Config) -> Config; -init_per_group(Group, Config0) when Group =:= sim_https; Group =:= https-> +init_per_group(Group, Config0) when Group =:= sim_https; Group =:= https; + Group =:= sim_mixed -> catch crypto:stop(), try crypto:start() of ok -> @@ -238,6 +246,13 @@ end_per_group(http_unix_socket,_Config) -> end_per_group(_, _Config) -> ok. +do_init_per_group(Group=sim_mixed, Config0) -> + % The mixed group uses two server ports (http and https), so we use + % different config names here. + Config1 = init_ssl(Config0), + Config2 = proplists:delete(http_port, proplists:delete(https_port, Config1)), + {HttpPort, HttpsPort} = server_start(Group, server_config(sim_https, Config2)), + [{http_port, HttpPort} | [{https_port, HttpsPort} | Config2]]; do_init_per_group(Group, Config0) -> Config1 = case Group of @@ -734,6 +749,24 @@ redirect_loop(Config) when is_list(Config) -> = httpc:request(get, {URL, []}, [], []). %%------------------------------------------------------------------------- +redirect_http_to_https() -> + [{doc, "Test that a 30X redirect from one scheme to another is handled " + "correctly."}]. +redirect_http_to_https(Config) when is_list(Config) -> + URL301 = mixed_url(http, "/301_custom_url.html", Config), + TargetUrl = mixed_url(https, "/dummy.html", Config), + Headers = [{"x-test-301-url", TargetUrl}], + + {ok, {{_,200,_}, [_ | _], [_|_]}} + = httpc:request(get, {URL301, Headers}, [], []), + + {ok, {{_,200,_}, [_ | _], []}} + = httpc:request(head, {URL301, Headers}, [], []), + + {ok, {{_,200,_}, [_ | _], [_|_]}} + = httpc:request(post, {URL301, Headers, "text/plain", "foobar"}, + [], []). +%%------------------------------------------------------------------------- cookie() -> [{doc, "Test cookies."}]. cookie(Config) when is_list(Config) -> @@ -1559,6 +1592,21 @@ url(sim_http, UserInfo, End, Config) -> url(sim_https, UserInfo, End, Config) -> url(https, UserInfo, End, Config). +% Only for use in the `mixed` test group, where both http and https +% URLs are possible. +mixed_url(http, End, Config) -> + mixed_url(http_port, End, Config); +mixed_url(https, End, Config) -> + mixed_url(https_port, End, Config); +mixed_url(PortType, End, Config) -> + Port = proplists:get_value(PortType, Config), + {ok, Host} = inet:gethostname(), + Start = case PortType of + http_port -> ?URL_START; + https_port -> ?TLS_URL_START + end, + Start ++ Host ++ ":" ++ integer_to_list(Port) ++ End. + group_name(Config) -> GroupProp = proplists:get_value(tc_group_properties, Config), proplists:get_value(name, GroupProp). @@ -1587,6 +1635,9 @@ server_start(http_ipv6, HttpdConfig) -> Serv = inets:services_info(), {value, {_, _, Info}} = lists:keysearch(Pid, 2, Serv), proplists:get_value(port, Info); +server_start(sim_mixed, Config) -> + % For the mixed http/https case, we start two servers and return both ports. + {server_start(sim_http, []), server_start(sim_https, Config)}; server_start(_, HttpdConfig) -> {ok, Pid} = inets:start(httpd, HttpdConfig), Serv = inets:services_info(), @@ -1645,6 +1696,8 @@ start_apps(https) -> inets_test_lib:start_apps([crypto, public_key, ssl]); start_apps(sim_https) -> inets_test_lib:start_apps([crypto, public_key, ssl]); +start_apps(sim_mixed) -> + inets_test_lib:start_apps([crypto, public_key, ssl]); start_apps(_) -> ok. @@ -2089,6 +2142,20 @@ handle_uri(_,"/301_rel_uri.html",_,_,_,_) -> "Content-Length:" ++ integer_to_list(length(Body)) ++ "\r\n\r\n" ++ Body; +handle_uri("HEAD","/301_custom_url.html",_,Headers,_,_) -> + NewUri = proplists:get_value("x-test-301-url", Headers), + "HTTP/1.1 301 Moved Permanently\r\n" ++ + "Location:" ++ NewUri ++ "\r\n" ++ + "Content-Length:0\r\n\r\n"; + +handle_uri(_,"/301_custom_url.html",_,Headers,_,_) -> + NewUri = proplists:get_value("x-test-301-url", Headers), + Body = "New place", + "HTTP/1.1 301 Moved Permanently\r\n" ++ + "Location:" ++ NewUri ++ "\r\n" ++ + "Content-Length:" ++ integer_to_list(length(Body)) + ++ "\r\n\r\n" ++ Body; handle_uri("HEAD","/302.html",Port,_,Socket,_) -> NewUri = url_start(Socket) ++ -- cgit v1.2.3 From 9993d349a8abcb7ba07bebcc8cc71ecec291ca68 Mon Sep 17 00:00:00 2001 From: Ross Schlaikjer Date: Mon, 2 Jul 2018 10:26:15 -0400 Subject: Do not assert that new URI port is same as old port When handling 301 redirects from http -> https on Erlang 21.0.1, the following error is encountered: ``` 8> Options = []. 9> httpc:request(head, {"http://rhye.org", []}, Options, []). {error,{shutdown,{{error,{badmatch,443}}, [{httpc_response,resolve_uri,7, [{file,"/usr/local/lib/erlang/lib/inets-7.0/src/http_client/httpc_response.erl"}, {line,431}]}, {httpc_response,redirect,2, [{file,"/usr/local/lib/erlang/lib/inets-7.0/src/http_client/httpc_response.erl"}, {line,396}]}, {httpc_handler,handle_response,1, [{file,"httpc_handler.erl"},{line,1052}]}, {httpc_handler,handle_info,2, [{file,"httpc_handler.erl"},{line,283}]}, {gen_server,try_dispatch,4, [{file,"gen_server.erl"},{line,637}]}, {gen_server,handle_msg,6, [{file,"gen_server.erl"},{line,711}]}, {proc_lib,init_p_do_apply,3, [{file,"proc_lib.erl"},{line,249}]}]}}} ``` This seems to be caused by the following code in `resolve_uri`: ``` resolve_uri(Scheme, Host, Port, Path, Query, URI, Map0) -> case maps:is_key(scheme, URI) of true -> Port = get_port(URI) ``` The value of `Port` passed in to `resolve_uri` here is 80, since the original URL is http. However, since the redirected URL is https, the `get_port` call returns 443, which is not equal to 80, and crashes. Assigning to a new variable seems to fix redirects. --- lib/inets/src/http_client/httpc_response.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/inets/src/http_client/httpc_response.erl b/lib/inets/src/http_client/httpc_response.erl index 0f3bd0a06d..0931f0b4d3 100644 --- a/lib/inets/src/http_client/httpc_response.erl +++ b/lib/inets/src/http_client/httpc_response.erl @@ -425,11 +425,11 @@ resolve_uri(Scheme, Host, Port, Path, Query, URI) -> resolve_uri(Scheme, Host, Port, Path, Query, URI, Map0) -> case maps:is_key(scheme, URI) of true -> - Port = get_port(URI), + Port0 = get_port(URI), maybe_add_query( Map0#{scheme => maps:get(scheme, URI), host => maps:get(host, URI), - port => Port, + port => Port0, path => maps:get(path, URI)}, URI); false -> -- cgit v1.2.3 From bd890e1859c97297a06166e5f92e8c33b1db4682 Mon Sep 17 00:00:00 2001 From: Ross Schlaikjer Date: Tue, 3 Jul 2018 17:14:28 +0000 Subject: Add test case on relative redirects with ports --- lib/inets/test/httpc_SUITE.erl | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl index 0440cb302c..6e048a4d56 100644 --- a/lib/inets/test/httpc_SUITE.erl +++ b/lib/inets/test/httpc_SUITE.erl @@ -174,7 +174,8 @@ misc() -> sim_mixed() -> [ - redirect_http_to_https + redirect_http_to_https, + redirect_relative_different_port ]. %%-------------------------------------------------------------------- @@ -767,6 +768,30 @@ redirect_http_to_https(Config) when is_list(Config) -> = httpc:request(post, {URL301, Headers, "text/plain", "foobar"}, [], []). %%------------------------------------------------------------------------- +redirect_relative_different_port() -> + [{doc, "Test that a 30X redirect with a relative target, but different " + "port, is handled correctly."}]. +redirect_relative_different_port(Config) when is_list(Config) -> + URL301 = mixed_url(http, "/301_custom_url.html", Config), + + % We need an extra server of the same protocol here, so spawn a new + % HTTP-protocol one + Port = server_start(sim_http, []), + {ok, Host} = inet:gethostname(), + % Prefix the URI with '/' instead of a scheme + TargetUrl = "//" ++ Host ++ ":" ++ integer_to_list(Port) ++ "/dummy.html", + Headers = [{"x-test-301-url", TargetUrl}], + + {ok, {{_,200,_}, [_ | _], [_|_]}} + = httpc:request(get, {URL301, Headers}, [], []), + + {ok, {{_,200,_}, [_ | _], []}} + = httpc:request(head, {URL301, Headers}, [], []), + + {ok, {{_,200,_}, [_ | _], [_|_]}} + = httpc:request(post, {URL301, Headers, "text/plain", "foobar"}, + [], []). +%%------------------------------------------------------------------------- cookie() -> [{doc, "Test cookies."}]. cookie(Config) when is_list(Config) -> -- cgit v1.2.3 From de29bd57702c41c7d8ee463362dc64bc5bff0b70 Mon Sep 17 00:00:00 2001 From: Ross Schlaikjer Date: Tue, 3 Jul 2018 10:50:17 -0400 Subject: Fix accidental Port assertion in resolve_authority --- lib/inets/src/http_client/httpc_response.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/inets/src/http_client/httpc_response.erl b/lib/inets/src/http_client/httpc_response.erl index 0931f0b4d3..dfb65a4083 100644 --- a/lib/inets/src/http_client/httpc_response.erl +++ b/lib/inets/src/http_client/httpc_response.erl @@ -457,10 +457,10 @@ get_default_port("https") -> resolve_authority(Host, Port, Path, Query, RelURI, Map) -> case maps:is_key(host, RelURI) of true -> - Port = get_port(RelURI), + Port0 = get_port(RelURI), maybe_add_query( Map#{host => maps:get(host, RelURI), - port => Port, + port => Port0, path => maps:get(path, RelURI)}, RelURI); false -> -- cgit v1.2.3 From cf43aba758ceeb1e4f313a2fdc1e901b9c1be06a Mon Sep 17 00:00:00 2001 From: Ross Schlaikjer Date: Tue, 3 Jul 2018 17:33:37 +0000 Subject: Update scheme on redirect URI and accumulator This is necessary to prevent an error when calling get_port inside resolve_authority --- lib/inets/src/http_client/httpc_response.erl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/inets/src/http_client/httpc_response.erl b/lib/inets/src/http_client/httpc_response.erl index dfb65a4083..46073d47ec 100644 --- a/lib/inets/src/http_client/httpc_response.erl +++ b/lib/inets/src/http_client/httpc_response.erl @@ -434,7 +434,8 @@ resolve_uri(Scheme, Host, Port, Path, Query, URI, Map0) -> URI); false -> Map = Map0#{scheme => Scheme}, - resolve_authority(Host, Port, Path, Query, URI, Map) + URI1 = URI#{scheme => Scheme}, + resolve_authority(Host, Port, Path, Query, URI1, Map) end. -- cgit v1.2.3 From 3291b50bb2115008834b8ce0aa2521b1a4a04bc8 Mon Sep 17 00:00:00 2001 From: Ross Schlaikjer Date: Wed, 4 Jul 2018 08:13:20 -0400 Subject: Don't modify URI, explicitly pass scheme to get_port --- lib/inets/src/http_client/httpc_response.erl | 32 +++++++++++++--------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/lib/inets/src/http_client/httpc_response.erl b/lib/inets/src/http_client/httpc_response.erl index 46073d47ec..78d6b4ed24 100644 --- a/lib/inets/src/http_client/httpc_response.erl +++ b/lib/inets/src/http_client/httpc_response.erl @@ -423,24 +423,24 @@ resolve_uri(Scheme, Host, Port, Path, Query, URI) -> resolve_uri(Scheme, Host, Port, Path, Query, URI, #{}). %% resolve_uri(Scheme, Host, Port, Path, Query, URI, Map0) -> - case maps:is_key(scheme, URI) of - true -> - Port0 = get_port(URI), + case maps:get(scheme, URI, undefined) of + undefined -> + Port0 = get_port(Scheme, URI), + Map = Map0#{scheme => Scheme, + port => Port0}, + resolve_authority(Host, Port, Path, Query, URI, Map); + URIScheme -> + Port0 = get_port(URIScheme, URI), maybe_add_query( - Map0#{scheme => maps:get(scheme, URI), - host => maps:get(host, URI), - port => Port0, - path => maps:get(path, URI)}, - URI); - false -> - Map = Map0#{scheme => Scheme}, - URI1 = URI#{scheme => Scheme}, - resolve_authority(Host, Port, Path, Query, URI1, Map) + Map0#{scheme => URIScheme, + host => maps:get(host, URI), + port => Port0, + path => maps:get(path, URI)}, + URI) end. -get_port(URI) -> - Scheme = maps:get(scheme, URI), +get_port(Scheme, URI) -> case maps:get(port, URI, undefined) of undefined -> get_default_port(Scheme); @@ -458,15 +458,13 @@ get_default_port("https") -> resolve_authority(Host, Port, Path, Query, RelURI, Map) -> case maps:is_key(host, RelURI) of true -> - Port0 = get_port(RelURI), maybe_add_query( Map#{host => maps:get(host, RelURI), - port => Port0, path => maps:get(path, RelURI)}, RelURI); false -> Map1 = Map#{host => Host, - port => Port}, + port => Port}, resolve_path(Path, Query, RelURI, Map1) end. -- cgit v1.2.3