Merge branch 'peterdmv/inets/http_content_injection/ERL-456/OTP-14726'

* peterdmv/inets/http_content_injection/ERL-456/OTP-14726:
  inets: Fix http content injection bug in httpc

5 files changed, 112 insertions, 64 deletions

diff --git a/lib/inets/doc/src/httpc.xml b/lib/inets/doc/src/httpc.xml
index 29e4b22632..58714328c5 100644
--- a/lib/inets/doc/src/httpc.xml
+++ b/lib/inets/doc/src/httpc.xml
@@ -287,8 +287,7 @@
                            {autoredirect,    boolean()} | 
                            {proxy_auth, {userstring(), passwordstring()}} | 
                            {version,         http_version()} | 
-                           {relaxed,         boolean()} |
-	                   {url_encode,      boolean()}</v>
+                           {relaxed,         boolean()}</v>
         <v>timeout() = integer() >= 0 | infinity</v>
         <v>Options = options()</v>
         <v>options() = [option()]</v>
@@ -379,13 +378,6 @@
 	    from the HTTP-standard are enabled.</p>
             <p>Default is <c>false</c>.</p>
 	  </item>
-
-	  <tag><c><![CDATA[url_encode]]></c></tag>
-          <item>
-            <p>Applies Percent-encoding, also known as URL encoding on the 
-	    URL.</p>
-	    <p>Default is <c>false</c>.</p>
-	  </item>
         </taglist>
 
         <p>Option (<c>option()</c>) details:</p>
diff --git a/lib/inets/src/http_client/httpc.erl b/lib/inets/src/http_client/httpc.erl
index bf2da82603..2efe2c2858 100644
--- a/lib/inets/src/http_client/httpc.erl
+++ b/lib/inets/src/http_client/httpc.erl
@@ -175,10 +175,10 @@ request(Method,
        (Method =:= delete) orelse 
        (Method =:= trace) andalso 
        (is_atom(Profile) orelse is_pid(Profile)) ->
-    case uri_parse(Url, Options) of
-	{error, Reason} ->
+    case uri_string:parse(uri_string:normalize(Url)) of
+	{error, Reason, _} ->
 	    {error, Reason};
-	{ok, ParsedUrl} ->
+	ParsedUrl ->
 	    case header_parse(Headers) of
 		{error, Reason} ->
 		    {error, Reason};
@@ -189,10 +189,10 @@ request(Method,
     end.
 
 do_request(Method, {Url, Headers, ContentType, Body}, HTTPOptions, Options, Profile) ->
-    case uri_parse(Url, Options) of
-	{error, Reason} ->
+    case uri_string:parse(uri_string:normalize(Url)) of
+	{error, Reason, _} ->
 	    {error, Reason};
-	{ok, ParsedUrl} ->
+	ParsedUrl ->
 	    handle_request(Method, Url, 
 			   ParsedUrl, Headers, ContentType, Body, 
 			   HTTPOptions, Options, Profile)
@@ -312,23 +312,28 @@ store_cookies(SetCookieHeaders, Url) ->
 
 store_cookies(SetCookieHeaders, Url, Profile) 
   when is_atom(Profile) orelse is_pid(Profile) ->
-    try 
-	begin
+    case uri_string:parse(uri_string:normalize(Url)) of
+        {error, Bad, _} ->
+            {error, {parse_failed, Bad}};
+        URI ->
+            Scheme = scheme_to_atom(maps:get(scheme, URI, '')),
+            Host = maps:get(host, URI, ""),
+            Port = maps:get(port, URI, default_port(Scheme)),
+            Path = maps:get(path, URI, ""),
 	    %% Since the Address part is not actually used
 	    %% by the manager when storing cookies, we dont
 	    %% care about ipv6-host-with-brackets.
-	    {ok, {_, _, Host, Port, Path, _}} = uri_parse(Url),
 	    Address     = {Host, Port}, 
 	    ProfileName = profile_name(Profile),
 	    Cookies     = httpc_cookie:cookies(SetCookieHeaders, Path, Host),
 	    httpc_manager:store_cookies(Cookies, Address, ProfileName), 
 	    ok
-	end
-    catch 
-	error:{badmatch, Bad} ->
-	    {error, {parse_failed, Bad}}
     end.
 
+default_port(http) ->
+    80;
+default_port(https) ->
+    443.
 
 %%--------------------------------------------------------------------------
 %% cookie_header(Url) -> Header | {error, Reason}
@@ -495,7 +500,7 @@ service_info(Pid) ->
 %%% Internal functions
 %%%========================================================================
 handle_request(Method, Url, 
-	       {Scheme, UserInfo, Host, Port, Path, Query}, 
+               URI,
 	       Headers0, ContentType, Body0,
 	       HTTPOptions0, Options0, Profile) ->
 
@@ -520,37 +525,40 @@ handle_request(Method, Url,
 			throw({error, {bad_body, Body0}})
 		end,
 
-	    HTTPOptions   = http_options(HTTPOptions0),
-	    Options       = request_options(Options0), 
-	    Sync          = proplists:get_value(sync,   Options),
-	    Stream        = proplists:get_value(stream, Options),
-	    Host2         = http_request:normalize_host(Scheme, Host, Port),
-	    HeadersRecord = header_record(NewHeaders, Host2, HTTPOptions),
-	    Receiver      = proplists:get_value(receiver, Options),
-	    SocketOpts    = proplists:get_value(socket_opts, Options),
-	    BracketedHost = proplists:get_value(ipv6_host_with_brackets, 
-						Options),
-	    MaybeEscPath  = maybe_encode_uri(HTTPOptions, Path),
-	    MaybeEscQuery = maybe_encode_uri(HTTPOptions, Query),
-	    AbsUri        = maybe_encode_uri(HTTPOptions, Url),
+            HTTPOptions   = http_options(HTTPOptions0),
+            Options       = request_options(Options0),
+            Sync          = proplists:get_value(sync,   Options),
+            Stream        = proplists:get_value(stream, Options),
+            Receiver      = proplists:get_value(receiver, Options),
+            SocketOpts    = proplists:get_value(socket_opts, Options),
+            BracketedHost = proplists:get_value(ipv6_host_with_brackets,
+                                                Options),
+
+            Scheme        = scheme_to_atom(maps:get(scheme, URI, '')),
+            Userinfo      = maps:get(userinfo, URI, ""),
+            Host          = http_util:maybe_add_brackets(maps:get(host, URI, ""), BracketedHost),
+            Port          = maps:get(port, URI, default_port(Scheme)),
+            Host2         = http_request:normalize_host(Scheme, Host, Port),
+            Path          = maps:get(path, URI, ""),
+            Query         = add_question_mark(maps:get(query, URI, "")),
+            HeadersRecord = header_record(NewHeaders, Host2, HTTPOptions),
 
 	    Request = #request{from          = Receiver,
-			       scheme        = Scheme, 
-			       address       = {host_address(Host, BracketedHost), Port},
-			       path          = MaybeEscPath,
-			       pquery        = MaybeEscQuery,
+			       scheme        = Scheme,
+			       address       = {Host, Port},
+			       path          = Path,
+			       pquery        = Query,
 			       method        = Method,
 			       headers       = HeadersRecord, 
 			       content       = {ContentType, Body},
 			       settings      = HTTPOptions, 
-			       abs_uri       = AbsUri,
-			       userinfo      = UserInfo, 
+			       abs_uri       = Url,
+			       userinfo      = Userinfo,
 			       stream        = Stream, 
 			       headers_as_is = headers_as_is(Headers0, Options),
 			       socket_opts   = SocketOpts, 
 			       started       = Started,
 			       ipv6_host_with_brackets = BracketedHost},
-
 	    case httpc_manager:request(Request, profile_name(Profile)) of
 		{ok, RequestId} ->
 		    handle_answer(RequestId, Sync, Options);
@@ -565,14 +573,31 @@ handle_request(Method, Url,
 	    Error
     end.
 
+
+add_question_mark(<<>>) ->
+    <<>>;
+add_question_mark([]) ->
+    [];
+add_question_mark(Comp) when is_binary(Comp) ->
+    <<$?, Comp/binary>>;
+add_question_mark(Comp) when is_list(Comp) ->
+    [$?|Comp].
+
+
+scheme_to_atom("http") ->
+    http;
+scheme_to_atom("https") ->
+    https;
+scheme_to_atom('') ->
+    '';
+scheme_to_atom(Scheme) ->
+    throw({error, {bad_scheme, Scheme}}).
+
+
 ensure_chunked_encoding(Hdrs) ->
     Key = "transfer-encoding",
     lists:keystore(Key, 1, Hdrs, {Key, "chunked"}).
 
-maybe_encode_uri(#http_options{url_encode = true}, URI) ->
-    http_uri:encode(URI);
-maybe_encode_uri(_, URI) ->
-    URI.
 
 mk_chunkify_fun(ProcessBody) ->
     fun(eof_body) ->
@@ -1190,17 +1215,6 @@ validate_headers(RequestHeaders, _, _) ->
 %% These functions is just simple wrappers to parse specifically HTTP URIs
 %%--------------------------------------------------------------------------
 
-scheme_defaults() ->
-    [{http, 80}, {https, 443}].
-
-uri_parse(URI) ->
-    http_uri:parse(URI, [{scheme_defaults, scheme_defaults()}]).
-
-uri_parse(URI, Opts) ->
-    http_uri:parse(URI, [{scheme_defaults, scheme_defaults()} | Opts]).
-
-
-%%--------------------------------------------------------------------------
 header_parse([]) ->
     ok;
 header_parse([{Field, Value}|T]) when is_list(Field), is_list(Value) ->    
@@ -1221,10 +1235,6 @@ child_name(Pid, [{Name, Pid} | _]) ->
 child_name(Pid, [_ | Children]) ->
     child_name(Pid, Children).
 
-host_address(Host, false) ->
-    Host;
-host_address(Host, true) ->
-    string:strip(string:strip(Host, right, $]), left, $[).
 
 check_body_gen({Fun, _}) when is_function(Fun) -> 
     ok;
diff --git a/lib/inets/src/http_lib/http_util.erl b/lib/inets/src/http_lib/http_util.erl
index 487d04f7aa..5577b00cc8 100644
--- a/lib/inets/src/http_lib/http_util.erl
+++ b/lib/inets/src/http_lib/http_util.erl
@@ -27,7 +27,8 @@
 	 convert_month/1, 
 	 is_hostname/1,
 	 timestamp/0, timeout/2,
-	 html_encode/1
+	 html_encode/1,
+	 maybe_add_brackets/2
 	]).
 
 
@@ -194,6 +195,24 @@ html_encode(Chars) ->
     lists:append([char_to_html_entity(Char, Reserved) || Char <- Chars]).
 
 
+maybe_add_brackets(Addr, false) ->
+    Addr;
+maybe_add_brackets(Addr, true) when is_list(Addr) ->
+    case is_ipv6_address(Addr) of
+        true ->
+            [$[|Addr] ++ "]";
+        false ->
+            Addr
+    end;
+maybe_add_brackets(Addr, true) when is_binary(Addr) ->
+    case is_ipv6_address(Addr) of
+        true ->
+            <<$[,Addr/binary,$]>>;
+        false ->
+            Addr
+    end.
+
+
 %%%========================================================================
 %%% Internal functions
 %%%========================================================================
@@ -205,3 +224,14 @@ char_to_html_entity(Char, Reserved) ->
 	false ->
 	    [Char]
     end.
+
+is_ipv6_address(Addr) when is_binary(Addr) ->
+    B = binary_to_list(Addr),
+    is_ipv6_address(B);
+is_ipv6_address(Addr) when is_list(Addr) ->
+    case inet:parse_ipv6strict_address(Addr) of
+        {ok, _ } ->
+            true;
+        {error, _} ->
+            false
+    end.
diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl
index 75b50f3420..954787d7ee 100644
--- a/lib/inets/test/httpc_SUITE.erl
+++ b/lib/inets/test/httpc_SUITE.erl
@@ -68,6 +68,7 @@ real_requests()->
     [
      head,
      get,
+     get_query_string,
      post,
      delete,
      post_stream,
@@ -243,6 +244,15 @@ get(Config) when is_list(Config) ->
 
     {ok, {{_,200,_}, [_ | _], BinBody}} =  httpc:request(get, Request, [], [{body_format, binary}]),
     true = is_binary(BinBody).
+
+
+get_query_string() ->
+    [{doc, "Test http get request with query string against local server"}].
+get_query_string(Config) when is_list(Config) ->
+    Request  = {url(group_name(Config), "/dummy.html?foo=bar", Config), []},
+    {ok, {{_,200,_}, [_ | _], Body = [_ | _]}} = httpc:request(get, Request, [], []),
+
+    inets_test_lib:check_body(Body).
 %%--------------------------------------------------------------------
 post() ->
     [{"Test http post request against local server. We do in this case "
@@ -1712,6 +1722,9 @@ content_length(["content-length:" ++ Value | _]) ->
 content_length([_Head | Tail]) ->
    content_length(Tail).
 
+handle_uri("GET","/dummy.html?foo=bar",_,_,_,_) ->
+    "HTTP/1.0 200 OK\r\n\r\nTEST";
+
 handle_uri(_,"/just_close.html",_,_,_,_) ->
 		close;
 handle_uri(_,"/no_content.html",_,_,_,_) ->
diff --git a/lib/inets/test/httpd_basic_SUITE.erl b/lib/inets/test/httpd_basic_SUITE.erl
index 931cd076cc..94d22ea76c 100644
--- a/lib/inets/test/httpd_basic_SUITE.erl
+++ b/lib/inets/test/httpd_basic_SUITE.erl
@@ -303,7 +303,10 @@ escaped_url_in_error_body(Config) when is_list(Config) ->
     %% Ask for a non-existing page(1)
     Path            = "/<b>this_is_bold<b>",
     HTMLEncodedPath = http_util:html_encode(Path),
-    URL2            = URL1 ++ Path,
+    URL2 = uri_string:recompose(#{scheme => "http",
+                                  host => "localhost",
+                                  port => Port,
+                                  path => Path}),
     {ok, {404, Body3}} = httpc:request(get, {URL2, []},
 				       [{url_encode,  true}, 
 					{version,     "HTTP/1.0"}],