Add cowboy_req:uri/1,2

Along with more cowboy_req tests.

This commit also removes cowboy_req:url/1 and cowboy_req:host_url/1
in favor of the much more powerful new set of functions.

3 files changed, 249 insertions, 60 deletions

diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl
index 121f747..68d96e7 100644
--- a/src/cowboy_req.erl
+++ b/src/cowboy_req.erl
@@ -28,8 +28,8 @@
 -export([qs/1]).
 -export([parse_qs/1]).
 -export([match_qs/2]).
--export([host_url/1]).
--export([url/1]).
+-export([uri/1]).
+-export([uri/2]).
 -export([binding/2]).
 -export([binding/3]).
 -export([bindings/1]).
@@ -204,31 +204,135 @@ parse_qs(#{qs := Qs}) ->
 match_qs(Fields, Req) ->
 	filter(Fields, kvlist_to_map(Fields, parse_qs(Req))).
 
-%% The URL includes the scheme, host and port only.
--spec host_url(req()) -> undefined | binary().
-host_url(#{port := undefined}) ->
-	undefined;
-host_url(#{scheme := Scheme, host := Host, port := Port}) ->
-	PortBin = case {Scheme, Port} of
-		{<<"https">>, 443} -> <<>>;
-		{<<"http">>, 80} -> <<>>;
-		_ -> << ":", (integer_to_binary(Port))/binary >>
-	end,
-	<< Scheme/binary, "://", Host/binary, PortBin/binary >>.
-
-%% The URL includes the scheme, host, port, path and query string.
--spec url(req()) -> undefined | binary().
-url(Req) ->
-	url(Req, host_url(Req)).
-
-url(_, undefined) ->
-	undefined;
-url(#{path := Path, qs := QS}, HostURL) ->
-	QS2 = case QS of
-		<<>> -> <<>>;
-		_ -> << "?", QS/binary >>
+-spec uri(req()) -> iodata().
+uri(Req) ->
+	uri(Req, #{}).
+
+-spec uri(req(), map()) -> iodata().
+uri(#{scheme := Scheme0, host := Host0, port := Port0,
+		path := Path0, qs := Qs0}, Opts) ->
+	Scheme = case maps:get(scheme, Opts, Scheme0) of
+		S = undefined -> S;
+		S -> iolist_to_binary(S)
 	end,
-	<< HostURL/binary, Path/binary, QS2/binary >>.
+	Host = maps:get(host, Opts, Host0),
+	Port = maps:get(port, Opts, Port0),
+	Path = maps:get(path, Opts, Path0),
+	Qs = maps:get(qs, Opts, Qs0),
+	Fragment = maps:get(fragment, Opts, undefined),
+	[uri_host(Scheme, Scheme0, Port, Host), uri_path(Path), uri_qs(Qs), uri_fragment(Fragment)].
+
+uri_host(_, _, _, undefined) -> <<>>;
+uri_host(Scheme, Scheme0, Port, Host) ->
+	case iolist_size(Host) of
+		0 -> <<>>;
+		_ -> [uri_scheme(Scheme), <<"//">>, Host, uri_port(Scheme, Scheme0, Port)]
+	end.
+
+uri_scheme(undefined) -> <<>>;
+uri_scheme(Scheme) ->
+	case iolist_size(Scheme) of
+		0 -> Scheme;
+		_ -> [Scheme, $:]
+	end.
+
+uri_port(_, _, undefined) -> <<>>;
+uri_port(undefined, <<"http">>, 80) -> <<>>;
+uri_port(undefined, <<"https">>, 443) -> <<>>;
+uri_port(<<"http">>, _, 80) -> <<>>;
+uri_port(<<"https">>, _, 443) -> <<>>;
+uri_port(_, _, Port) ->
+	[$:, integer_to_binary(Port)].
+
+uri_path(undefined) -> <<>>;
+uri_path(Path) -> Path.
+
+uri_qs(undefined) -> <<>>;
+uri_qs(Qs) ->
+	case iolist_size(Qs) of
+		0 -> Qs;
+		_ -> [$?, Qs]
+	end.
+
+uri_fragment(undefined) -> <<>>;
+uri_fragment(Fragment) ->
+	case iolist_size(Fragment) of
+		0 -> Fragment;
+		_ -> [$#, Fragment]
+	end.
+
+-ifdef(TEST).
+uri1_test() ->
+	<<"http://localhost/path">> = iolist_to_binary(uri(#{
+		scheme => <<"http">>, host => <<"localhost">>, port => 80,
+		path => <<"/path">>, qs => <<>>})),
+	<<"http://localhost:443/path">> = iolist_to_binary(uri(#{
+		scheme => <<"http">>, host => <<"localhost">>, port => 443,
+		path => <<"/path">>, qs => <<>>})),
+	<<"http://localhost:8080/path">> = iolist_to_binary(uri(#{
+		scheme => <<"http">>, host => <<"localhost">>, port => 8080,
+		path => <<"/path">>, qs => <<>>})),
+	<<"http://localhost:8080/path?dummy=2785">> = iolist_to_binary(uri(#{
+		scheme => <<"http">>, host => <<"localhost">>, port => 8080,
+		path => <<"/path">>, qs => <<"dummy=2785">>})),
+	<<"https://localhost/path">> = iolist_to_binary(uri(#{
+		scheme => <<"https">>, host => <<"localhost">>, port => 443,
+		path => <<"/path">>, qs => <<>>})),
+	<<"https://localhost:8443/path">> = iolist_to_binary(uri(#{
+		scheme => <<"https">>, host => <<"localhost">>, port => 8443,
+		path => <<"/path">>, qs => <<>>})),
+	<<"https://localhost:8443/path?dummy=2785">> = iolist_to_binary(uri(#{
+		scheme => <<"https">>, host => <<"localhost">>, port => 8443,
+		path => <<"/path">>, qs => <<"dummy=2785">>})),
+	ok.
+
+uri2_test() ->
+	Req = #{
+		scheme => <<"http">>, host => <<"localhost">>, port => 8080,
+		path => <<"/path">>, qs => <<"dummy=2785">>
+	},
+	<<"http://localhost:8080/path?dummy=2785">> = iolist_to_binary(uri(Req, #{})),
+	%% Disable individual components.
+	<<"//localhost:8080/path?dummy=2785">> = iolist_to_binary(uri(Req, #{scheme => undefined})),
+	<<"/path?dummy=2785">> = iolist_to_binary(uri(Req, #{host => undefined})),
+	<<"http://localhost/path?dummy=2785">> = iolist_to_binary(uri(Req, #{port => undefined})),
+	<<"http://localhost:8080?dummy=2785">> = iolist_to_binary(uri(Req, #{path => undefined})),
+	<<"http://localhost:8080/path">> = iolist_to_binary(uri(Req, #{qs => undefined})),
+	<<"http://localhost:8080/path?dummy=2785">> = iolist_to_binary(uri(Req, #{fragment => undefined})),
+	<<"http://localhost:8080">> = iolist_to_binary(uri(Req, #{path => undefined, qs => undefined})),
+	<<>> = iolist_to_binary(uri(Req, #{host => undefined, path => undefined, qs => undefined})),
+	%% Empty values.
+	<<"//localhost:8080/path?dummy=2785">> = iolist_to_binary(uri(Req, #{scheme => <<>>})),
+	<<"//localhost:8080/path?dummy=2785">> = iolist_to_binary(uri(Req, #{scheme => ""})),
+	<<"//localhost:8080/path?dummy=2785">> = iolist_to_binary(uri(Req, #{scheme => [<<>>]})),
+	<<"/path?dummy=2785">> = iolist_to_binary(uri(Req, #{host => <<>>})),
+	<<"/path?dummy=2785">> = iolist_to_binary(uri(Req, #{host => ""})),
+	<<"/path?dummy=2785">> = iolist_to_binary(uri(Req, #{host => [<<>>]})),
+	<<"http://localhost:8080?dummy=2785">> = iolist_to_binary(uri(Req, #{path => <<>>})),
+	<<"http://localhost:8080?dummy=2785">> = iolist_to_binary(uri(Req, #{path => ""})),
+	<<"http://localhost:8080?dummy=2785">> = iolist_to_binary(uri(Req, #{path => [<<>>]})),
+	<<"http://localhost:8080/path">> = iolist_to_binary(uri(Req, #{qs => <<>>})),
+	<<"http://localhost:8080/path">> = iolist_to_binary(uri(Req, #{qs => ""})),
+	<<"http://localhost:8080/path">> = iolist_to_binary(uri(Req, #{qs => [<<>>]})),
+	<<"http://localhost:8080/path?dummy=2785">> = iolist_to_binary(uri(Req, #{fragment => <<>>})),
+	<<"http://localhost:8080/path?dummy=2785">> = iolist_to_binary(uri(Req, #{fragment => ""})),
+	<<"http://localhost:8080/path?dummy=2785">> = iolist_to_binary(uri(Req, #{fragment => [<<>>]})),
+	%% Port is integer() | undefined.
+	{'EXIT', _} = (catch iolist_to_binary(uri(Req, #{port => <<>>}))),
+	{'EXIT', _} = (catch iolist_to_binary(uri(Req, #{port => ""}))),
+	{'EXIT', _} = (catch iolist_to_binary(uri(Req, #{port => [<<>>]}))),
+	%% Update components.
+	<<"https://localhost:8080/path?dummy=2785">> = iolist_to_binary(uri(Req, #{scheme => "https"})),
+	<<"http://example.org:8080/path?dummy=2785">> = iolist_to_binary(uri(Req, #{host => "example.org"})),
+	<<"http://localhost:123/path?dummy=2785">> = iolist_to_binary(uri(Req, #{port => 123})),
+	<<"http://localhost:8080/custom?dummy=2785">> = iolist_to_binary(uri(Req, #{path => "/custom"})),
+	<<"http://localhost:8080/path?smart=42">> = iolist_to_binary(uri(Req, #{qs => "smart=42"})),
+	<<"http://localhost:8080/path?dummy=2785#intro">> = iolist_to_binary(uri(Req, #{fragment => "intro"})),
+	%% Interesting combinations.
+	<<"http://localhost/path?dummy=2785">> = iolist_to_binary(uri(Req, #{port => 80})),
+	<<"https://localhost/path?dummy=2785">> = iolist_to_binary(uri(Req, #{scheme => "https", port => 443})),
+	ok.
+-endif.
 
 -spec binding(atom(), req()) -> any() | undefined.
 binding(Name, Req) ->
@@ -1125,33 +1229,6 @@ filter_constraints(Tail, Map, Key, Value, Constraints) ->
 %% Tests.
 
 -ifdef(TEST).
-url_test() ->
-	undefined =
-		url(#http_req{transport=ranch_tcp, host= <<>>, port= undefined,
-			path= <<>>, qs= <<>>, pid=self()}),
-	<<"http://localhost/path">> =
-		url(#http_req{transport=ranch_tcp, host= <<"localhost">>, port=80,
-			path= <<"/path">>, qs= <<>>, pid=self()}),
-	<<"http://localhost:443/path">> =
-		url(#http_req{transport=ranch_tcp, host= <<"localhost">>, port=443,
-			path= <<"/path">>, qs= <<>>, pid=self()}),
-	<<"http://localhost:8080/path">> =
-		url(#http_req{transport=ranch_tcp, host= <<"localhost">>, port=8080,
-			path= <<"/path">>, qs= <<>>, pid=self()}),
-	<<"http://localhost:8080/path?dummy=2785">> =
-		url(#http_req{transport=ranch_tcp, host= <<"localhost">>, port=8080,
-			path= <<"/path">>, qs= <<"dummy=2785">>, pid=self()}),
-	<<"https://localhost/path">> =
-		url(#http_req{transport=ranch_ssl, host= <<"localhost">>, port=443,
-			path= <<"/path">>, qs= <<>>, pid=self()}),
-	<<"https://localhost:8443/path">> =
-		url(#http_req{transport=ranch_ssl, host= <<"localhost">>, port=8443,
-			path= <<"/path">>, qs= <<>>, pid=self()}),
-	<<"https://localhost:8443/path?dummy=2785">> =
-		url(#http_req{transport=ranch_ssl, host= <<"localhost">>, port=8443,
-			path= <<"/path">>, qs= <<"dummy=2785">>, pid=self()}),
-	ok.
-
 connection_to_atom_test_() ->
 	Tests = [
 		{[<<"close">>], close},
diff --git a/test/handlers/echo_h.erl b/test/handlers/echo_h.erl
index cdb9be4..802d537 100644
--- a/test/handlers/echo_h.erl
+++ b/test/handlers/echo_h.erl
@@ -5,15 +5,48 @@
 -export([init/2]).
 
 init(Req, Opts) ->
-	echo(cowboy_req:binding(key, Req), Req, Opts).
+	case cowboy_req:binding(arg, Req) of
+		undefined ->
+			echo(cowboy_req:binding(key, Req), Req, Opts);
+		Arg ->
+			echo_arg(Arg, Req, Opts)
+	end.
 
+echo(<<"body">>, Req0, Opts) ->
+	{ok, Body, Req} = cowboy_req:read_body(Req0),
+	cowboy_req:reply(200, #{}, Body, Req),
+	{ok, Req, Opts};
+echo(<<"uri">>, Req, Opts) ->
+	Value = case cowboy_req:path_info(Req) of
+		[<<"origin">>] -> cowboy_req:uri(Req, #{host => undefined});
+		[<<"protocol-relative">>] -> cowboy_req:uri(Req, #{scheme => undefined});
+		[<<"no-qs">>] -> cowboy_req:uri(Req, #{qs => undefined});
+		[<<"no-path">>] -> cowboy_req:uri(Req, #{path => undefined, qs => undefined});
+		[<<"set-port">>] -> cowboy_req:uri(Req, #{port => 123});
+		[] -> cowboy_req:uri(Req)
+	end,
+	cowboy_req:reply(200, #{}, Value, Req),
+	{ok, Req, Opts};
 echo(What, Req, Opts) ->
 	F = binary_to_atom(What, latin1),
-	Value = case cowboy_req:F(Req) of
-		V when is_integer(V) -> integer_to_binary(V);
-		V when is_atom(V) -> atom_to_binary(V, latin1);
-		V when is_list(V); is_tuple(V) -> io_lib:format("~p", [V]);
-		V -> V
+	Value = cowboy_req:F(Req),
+	cowboy_req:reply(200, #{}, value_to_iodata(Value), Req),
+	{ok, Req, Opts}.
+
+echo_arg(Arg0, Req, Opts) ->
+	F = binary_to_atom(cowboy_req:binding(key, Req), latin1),
+	Arg = case F of
+		binding -> binary_to_atom(Arg0, latin1);
+		_ -> Arg0
 	end,
-	cowboy_req:reply(200, #{}, Value, Req),
+	Value = case cowboy_req:binding(default, Req) of
+		undefined -> cowboy_req:F(Arg, Req);
+		Default -> cowboy_req:F(Arg, Req, Default)
+	end,
+	cowboy_req:reply(200, #{}, value_to_iodata(Value), Req),
 	{ok, Req, Opts}.
+
+value_to_iodata(V) when is_integer(V) -> integer_to_binary(V);
+value_to_iodata(V) when is_atom(V) -> atom_to_binary(V, latin1);
+value_to_iodata(V) when is_list(V); is_tuple(V); is_map(V) -> io_lib:format("~p", [V]);
+value_to_iodata(V) -> V.
diff --git a/test/req_SUITE.erl b/test/req_SUITE.erl
index 39c975c..4d9502f 100644
--- a/test/req_SUITE.erl
+++ b/test/req_SUITE.erl
@@ -73,6 +73,32 @@ do_get_body(Path, Headers, Config) ->
 
 %% Tests.
 
+binding(Config) ->
+	doc("Value bound from request URI path with/without default."),
+	<<"binding">> = do_get_body("/args/binding/key", Config),
+	<<"binding">> = do_get_body("/args/binding/key/default", Config),
+	<<"default">> = do_get_body("/args/binding/undefined/default", Config),
+	ok.
+
+%% @todo Do we really want a key/value list here instead of a map?
+bindings(Config) ->
+	doc("Values bound from request URI path."),
+	<<"[{key,<<\"bindings\">>}]">> = do_get_body("/bindings", Config),
+	ok.
+
+header(Config) ->
+	doc("Request header with/without default."),
+	<<"value">> = do_get_body("/args/header/defined", [{<<"defined">>, "value"}], Config),
+	<<"value">> = do_get_body("/args/header/defined/default", [{<<"defined">>, "value"}], Config),
+	<<"default">> = do_get_body("/args/header/undefined/default", [{<<"defined">>, "value"}], Config),
+	ok.
+
+headers(Config) ->
+	doc("Request headers."),
+	<< "#{<<\"header\">> => <<\"value\">>", _/bits >>
+		= do_get_body("/headers", [{<<"header">>, "value"}], Config),
+	ok.
+
 host(Config) ->
 	doc("Request URI host."),
 	<<"localhost">> = do_get_body("/host", Config),
@@ -95,6 +121,30 @@ method(Config) ->
 	ok.
 
 %% @todo Do we really want a key/value list here instead of a map?
+parse_cookies(Config) ->
+	doc("Request cookies."),
+	<<"[]">> = do_get_body("/parse_cookies", Config),
+	<<"[{<<\"cake\">>,<<\"strawberry\">>}]">>
+		= do_get_body("/parse_cookies", [{<<"cookie">>, "cake=strawberry"}], Config),
+	<<"[{<<\"cake\">>,<<\"strawberry\">>},{<<\"color\">>,<<\"blue\">>}]">>
+		= do_get_body("/parse_cookies", [{<<"cookie">>, "cake=strawberry; color=blue"}], Config),
+	ok.
+
+parse_header(Config) ->
+	doc("Parsed request header with/without default."),
+	<<"[{{<<\"text\">>,<<\"html\">>,[]},1000,[]}]">>
+		= do_get_body("/args/parse_header/accept", [{<<"accept">>, "text/html"}], Config),
+	<<"[{{<<\"text\">>,<<\"html\">>,[]},1000,[]}]">>
+		= do_get_body("/args/parse_header/accept/default", [{<<"accept">>, "text/html"}], Config),
+	%% Header not in request but with default defined by Cowboy.
+	<<"0">> = do_get_body("/args/parse_header/content-length", Config),
+	%% Header not in request and no default from Cowboy.
+	<<"undefined">> = do_get_body("/args/parse_header/upgrade", Config),
+	%% Header in request and with default provided.
+	<<"100-continue">> = do_get_body("/args/parse_header/expect/100-continue", Config),
+	ok.
+
+%% @todo Do we really want a key/value list here instead of a map?
 parse_qs(Config) ->
 	doc("Parsed request URI query string."),
 	<<"[]">> = do_get_body("/parse_qs", Config),
@@ -147,6 +197,35 @@ scheme(Config) ->
 		<<"https">> when Transport =:= ssl -> ok
 	end.
 
+uri(Config) ->
+	doc("Request URI building/modification."),
+	Scheme = case config(type, Config) of
+		tcp -> <<"http">>;
+		ssl -> <<"https">>
+	end,
+	SLen = byte_size(Scheme),
+	Port = integer_to_binary(config(port, Config)),
+	PLen = byte_size(Port),
+	%% Absolute form.
+	<< Scheme:SLen/binary, "://localhost:", Port:PLen/binary, "/uri?qs" >>
+		= do_get_body("/uri?qs", Config),
+	%% Origin form.
+	<< "/uri/origin?qs" >> = do_get_body("/uri/origin?qs", Config),
+	%% Protocol relative.
+	<< "//localhost:", Port:PLen/binary, "/uri/protocol-relative?qs" >>
+		= do_get_body("/uri/protocol-relative?qs", Config),
+	%% No query string.
+	<< Scheme:SLen/binary, "://localhost:", Port:PLen/binary, "/uri/no-qs" >>
+		= do_get_body("/uri/no-qs?qs", Config),
+	%% No path or query string.
+	<< Scheme:SLen/binary, "://localhost:", Port:PLen/binary >>
+		= do_get_body("/uri/no-path?qs", Config),
+	%% Changed port.
+	<< Scheme:SLen/binary, "://localhost:123/uri/set-port?qs" >>
+		= do_get_body("/uri/set-port?qs", Config),
+	%% This function is tested more extensively through unit tests.
+	ok.
+
 version(Config) ->
 	doc("Request HTTP version."),
 	Protocol = config(protocol, Config),