Properly parse URI fragments

This fixes a bug in httpc where redirection URIs could lead to bad requests if they
contained fragments.

3 files changed, 61 insertions, 22 deletions

diff --git a/lib/inets/doc/src/http_uri.xml b/lib/inets/doc/src/http_uri.xml
index e64c375bba..acbd79b201 100644
--- a/lib/inets/doc/src/http_uri.xml
+++ b/lib/inets/doc/src/http_uri.xml
@@ -63,6 +63,7 @@ host()      = string()
 port()      = pos_integer()
 path()      = string() - Representing a file path or directory path 
 query()     = string()
+fragment()  = string()
     ]]></code>
 
     <marker id="scheme_defaults"></marker>
@@ -92,13 +93,16 @@ query()     = string()
         <v>URI = uri() </v> 
         <v>Options = [Option] </v> 
         <v>Option = {ipv6_host_with_brackets, boolean()} | 
-                    {scheme_defaults, scheme_defaults()}]</v> 
-	<v>Result = {Scheme, UserInfo, Host, Port, Path, Query}</v>
+                    {scheme_defaults, scheme_defaults()} |
+                    {fragment, boolean()}]</v>
+        <v>Result = {Scheme, UserInfo, Host, Port, Path, Query} |
+                    {Scheme, UserInfo, Host, Port, Path, Query, Fragment}</v>
 	<v>UserInfo = user_info()</v>
 	<v>Host = host()</v>
 	<v>Port = pos_integer()</v>
 	<v>Path = path()</v>
 	<v>Query = query()</v>
+        <v>Fragment = fragment()</v>
 	<v>Reason = term() </v>
       </type>
       <desc>
@@ -111,6 +115,9 @@ query()     = string()
 	a scheme not found in the scheme defaults) a port number must be 
 	provided or else the parsing will fail. </p>
 
+        <p>If the fragment option is true, the URI fragment will be returned as
+          part of the parsing result, otherwise it is completely ignored.</p>
+
         <marker id="encode"></marker>
       </desc>
     </func>
diff --git a/lib/inets/src/http_lib/http_uri.erl b/lib/inets/src/http_lib/http_uri.erl
index 5962001c3a..350a4bc169 100644
--- a/lib/inets/src/http_lib/http_uri.erl
+++ b/lib/inets/src/http_lib/http_uri.erl
@@ -90,8 +90,8 @@ parse(AbsURI, Opts) ->
 	    {error, Reason};
 	{Scheme, DefaultPort, Rest} ->
 	    case (catch parse_uri_rest(Scheme, DefaultPort, Rest, Opts)) of
-		{ok, {UserInfo, Host, Port, Path, Query}} ->
-		    {ok, {Scheme, UserInfo, Host, Port, Path, Query}};
+                {ok, Result} ->
+                    {ok, Result};
 		{error, Reason} ->
 		    {error, {Reason, Scheme, AbsURI}};
 		_  ->
@@ -148,27 +148,22 @@ parse_scheme(AbsURI, Opts) ->
     end.
 
 parse_uri_rest(Scheme, DefaultPort, "//" ++ URIPart, Opts) ->
-    {Authority, PathQuery} =
-	case split_uri(URIPart, "/", URIPart, 1, 0) of
-	    Split = {_, _} ->
-		Split;
-	    URIPart ->
-		case split_uri(URIPart, "\\?", URIPart, 1, 0) of
-		    Split = {_, _} ->
-			Split;
-		    URIPart ->
-			{URIPart,""}
-		end
-	end,
+    {Authority, PathQueryFragment} =
+        split_uri(URIPart, "[/?#]", {URIPart, ""}, 1, 0),
+    {RawPath, QueryFragment} =
+        split_uri(PathQueryFragment, "[?#]", {PathQueryFragment, ""}, 1, 0),
+    {Query, Fragment} =
+        split_uri(QueryFragment, "#", {QueryFragment, ""}, 1, 0),
     {UserInfo, HostPort} = split_uri(Authority, "@", {"", Authority}, 1, 1),
     {Host, Port}         = parse_host_port(Scheme, DefaultPort, HostPort, Opts),
-    {Path, Query}        = parse_path_query(PathQuery),
-    {ok, {UserInfo, Host, Port, Path, Query}}.
-
+    Path                 = path(RawPath),
+    case lists:keyfind(fragment, 1, Opts) of
+        {fragment, true} ->
+            {ok, {Scheme, UserInfo, Host, Port, Path, Query, Fragment}};
+        _ ->
+            {ok, {Scheme, UserInfo, Host, Port, Path, Query}}
+    end.
 
-parse_path_query(PathQuery) ->
-    {Path, Query} =  split_uri(PathQuery, "\\?", {PathQuery, ""}, 1, 0),
-    {path(Path), Query}.
 
 %% In this version of the function, we no longer need 
 %% the Scheme argument, but just in case...
diff --git a/lib/inets/test/uri_SUITE.erl b/lib/inets/test/uri_SUITE.erl
index 9ba09e1474..f75e347d0c 100644
--- a/lib/inets/test/uri_SUITE.erl
+++ b/lib/inets/test/uri_SUITE.erl
@@ -46,6 +46,7 @@ all() ->
      userinfo,
      scheme,
      queries,
+     fragments,
      escaped,
      hexed_query
     ].
@@ -105,6 +106,42 @@ queries(Config) when is_list(Config) ->
     {ok, {http,[],"localhost",8888,"/foobar.html","?foo=bar&foobar=42"}} =
 	http_uri:parse("http://localhost:8888/foobar.html?foo=bar&foobar=42").
 
+fragments(Config) when is_list(Config) ->
+    {ok, {http,[],"localhost",80,"/",""}} =
+        http_uri:parse("http://localhost#fragment"),
+    {ok, {http,[],"localhost",80,"/path",""}} =
+        http_uri:parse("http://localhost/path#fragment"),
+    {ok, {http,[],"localhost",80,"/","?query"}} =
+        http_uri:parse("http://localhost?query#fragment"),
+    {ok, {http,[],"localhost",80,"/path","?query"}} =
+        http_uri:parse("http://localhost/path?query#fragment"),
+    {ok, {http,[],"localhost",80,"/","","#fragment"}} =
+        http_uri:parse("http://localhost#fragment", [{fragment,true}]),
+    {ok, {http,[],"localhost",80,"/path","","#fragment"}} =
+        http_uri:parse("http://localhost/path#fragment", [{fragment,true}]),
+    {ok, {http,[],"localhost",80,"/","?query","#fragment"}} =
+        http_uri:parse("http://localhost?query#fragment", [{fragment,true}]),
+    {ok, {http,[],"localhost",80,"/path","?query","#fragment"}} =
+        http_uri:parse("http://localhost/path?query#fragment",
+                       [{fragment,true}]),
+    {ok, {http,[],"localhost",80,"/","",""}} =
+        http_uri:parse("http://localhost", [{fragment,true}]),
+    {ok, {http,[],"localhost",80,"/path","",""}} =
+        http_uri:parse("http://localhost/path", [{fragment,true}]),
+    {ok, {http,[],"localhost",80,"/","?query",""}} =
+        http_uri:parse("http://localhost?query", [{fragment,true}]),
+    {ok, {http,[],"localhost",80,"/path","?query",""}} =
+        http_uri:parse("http://localhost/path?query", [{fragment,true}]),
+    {ok, {http,[],"localhost",80,"/","","#"}} =
+        http_uri:parse("http://localhost#", [{fragment,true}]),
+    {ok, {http,[],"localhost",80,"/path","","#"}} =
+        http_uri:parse("http://localhost/path#", [{fragment,true}]),
+    {ok, {http,[],"localhost",80,"/","?query","#"}} =
+        http_uri:parse("http://localhost?query#", [{fragment,true}]),
+    {ok, {http,[],"localhost",80,"/path","?query","#"}} =
+        http_uri:parse("http://localhost/path?query#", [{fragment,true}]),
+    ok.
+
 escaped(Config) when is_list(Config) ->
        {ok, {http,[],"www.somedomain.com",80,"/%2Eabc",[]}} =
 	http_uri:parse("http://www.somedomain.com/%2Eabc"),