Merge pull request #1862 from rschlaikjer/rschlaikjer-httpc-ssl-redirect-bug

httpc 301 redirect: Do not assert scheme ports are equal

2 files changed, 110 insertions, 19 deletions

diff --git a/lib/inets/src/http_client/httpc_response.erl b/lib/inets/src/http_client/httpc_response.erl
index 0f3bd0a06d..78d6b4ed24 100644
--- a/lib/inets/src/http_client/httpc_response.erl
+++ b/lib/inets/src/http_client/httpc_response.erl
@@ -423,23 +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 ->
-            Port = 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 => Port,
-                   path => maps:get(path, URI)},
-              URI);
-        false ->
-            Map = Map0#{scheme => Scheme},
-            resolve_authority(Host, Port, Path, Query, URI, 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);
@@ -457,15 +458,13 @@ get_default_port("https") ->
 resolve_authority(Host, Port, Path, Query, RelURI, Map) ->
     case maps:is_key(host, RelURI) of
         true ->
-            Port = get_port(RelURI),
             maybe_add_query(
               Map#{host => maps:get(host, RelURI),
-                   port => Port,
                    path => maps:get(path, RelURI)},
               RelURI);
         false ->
             Map1 = Map#{host => Host,
-                    port => Port},
+                        port => Port},
             resolve_path(Path, Query, RelURI, Map1)
     end.
 
diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl
index d43e2cc179..6e048a4d56 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,12 @@ misc() ->
      wait_for_whole_response
     ].
 
+sim_mixed() ->
+    [
+     redirect_http_to_https,
+     redirect_relative_different_port
+    ].
+
 %%--------------------------------------------------------------------
 
 init_per_suite(Config) ->
@@ -195,7 +203,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 +247,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 +750,48 @@ 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"},
+			[], []).
+%%-------------------------------------------------------------------------
+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) ->
@@ -1559,6 +1617,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 +1660,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 +1721,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 +2167,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 = "<HTML><BODY><a href=" ++ NewUri ++
+	">New place</a></BODY></HTML>",
+    "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) ++