Remove the urldecode cowboy_protocol option

This allows inconsistent behavior and is not used enough to be supported.

src/cowboy_dispatcher.erl

@@ -17,7 +17,7 @@
 -module(cowboy_dispatcher).
 
 %% API.
--export([match/4]).
+-export([match/3]).
 
 -type bindings() :: [{atom(), binary()}].
 -type tokens() :: [binary()].
@@ -63,53 +63,51 @@
 %% options found in the dispatch list, a key-value list of bindings and
 %% the tokens that were matched by the <em>'...'</em> atom for both the
 %% hostname and path.
--spec match(dispatch_rules(), fun((binary()) -> binary()),
-		Host::binary() | tokens(), Path::binary())
+-spec match(dispatch_rules(), Host::binary() | tokens(), Path::binary())
 	-> {ok, module(), any(), bindings(),
 		HostInfo::undefined | tokens(),
 		PathInfo::undefined | tokens()}
 	| {error, notfound, host} | {error, notfound, path}.
-match([], _, _, _) ->
+match([], _, _) ->
 	{error, notfound, host};
-match([{'_', PathMatchs}|_Tail], URLDecode, _, Path) ->
-	match_path(PathMatchs, URLDecode, undefined, Path, []);
-match([{HostMatch, PathMatchs}|Tail], URLDecode, Tokens, Path)
+match([{'_', PathMatchs}|_Tail], _, Path) ->
+	match_path(PathMatchs, undefined, Path, []);
+match([{HostMatch, PathMatchs}|Tail], Tokens, Path)
 		when is_list(Tokens) ->
 	case list_match(Tokens, lists:reverse(HostMatch), []) of
 		false ->
-			match(Tail, URLDecode, Tokens, Path);
+			match(Tail, Tokens, Path);
 		{true, Bindings, undefined} ->
-			match_path(PathMatchs, URLDecode, undefined, Path, Bindings);
+			match_path(PathMatchs, undefined, Path, Bindings);
 		{true, Bindings, HostInfo} ->
-			match_path(PathMatchs, URLDecode, lists:reverse(HostInfo),
+			match_path(PathMatchs, lists:reverse(HostInfo),
 				Path, Bindings)
 	end;
-match(Dispatch, URLDecode, Host, Path) ->
-	match(Dispatch, URLDecode, split_host(Host), Path).
+match(Dispatch, Host, Path) ->
+	match(Dispatch, split_host(Host), Path).
 
--spec match_path(dispatch_path(), fun((binary()) -> binary()),
+-spec match_path(dispatch_path(),
 	HostInfo::undefined | tokens(), binary() | tokens(), bindings())
 	-> {ok, module(), any(), bindings(),
 		HostInfo::undefined | tokens(),
 		PathInfo::undefined | tokens()}
 	| {error, notfound, path}.
-match_path([], _, _, _, _) ->
+match_path([], _, _, _) ->
 	{error, notfound, path};
-match_path([{'_', Handler, Opts}|_Tail], _, HostInfo, _, Bindings) ->
+match_path([{'_', Handler, Opts}|_Tail], HostInfo, _, Bindings) ->
 	{ok, Handler, Opts, Bindings, HostInfo, undefined};
-match_path([{'*', Handler, Opts}|_Tail], _, HostInfo, '*', Bindings) ->
+match_path([{'*', Handler, Opts}|_Tail], HostInfo, '*', Bindings) ->
 	{ok, Handler, Opts, Bindings, HostInfo, undefined};
-match_path([{PathMatch, Handler, Opts}|Tail], URLDecode, HostInfo, Tokens,
+match_path([{PathMatch, Handler, Opts}|Tail], HostInfo, Tokens,
 		Bindings) when is_list(Tokens) ->
 	case list_match(Tokens, PathMatch, []) of
 		false ->
-			match_path(Tail, URLDecode, HostInfo, Tokens, Bindings);
+			match_path(Tail, HostInfo, Tokens, Bindings);
 		{true, PathBinds, PathInfo} ->
 			{ok, Handler, Opts, Bindings ++ PathBinds, HostInfo, PathInfo}
 	end;
-match_path(Dispatch, URLDecode, HostInfo, Path, Bindings) ->
-	match_path(Dispatch, URLDecode, HostInfo,
-		split_path(Path, URLDecode), Bindings).
+match_path(Dispatch, HostInfo, Path, Bindings) ->
+	match_path(Dispatch, HostInfo, split_path(Path), Bindings).
 
 %% Internal.
 
@@ -135,19 +133,19 @@ split_host(Host, Acc) ->
 %% Following RFC2396, this function may return path segments containing any
 %% character, including <em>/</em> if, and only if, a <em>/</em> was escaped
 %% and part of a path segment.
--spec split_path(binary(), fun((binary()) -> binary())) -> tokens().
-split_path(<< $/, Path/bits >>, URLDec) ->
-	split_path(Path, URLDec, []).
+-spec split_path(binary()) -> tokens().
+split_path(<< $/, Path/bits >>) ->
+	split_path(Path, []).
 
-split_path(Path, URLDec, Acc) ->
+split_path(Path, Acc) ->
 	case binary:match(Path, <<"/">>) of
 		nomatch when Path =:= <<>> ->
-			lists:reverse([URLDec(S) || S <- Acc]);
+			lists:reverse([cowboy_http:urldecode(S) || S <- Acc]);
 		nomatch ->
-			lists:reverse([URLDec(S) || S <- [Path|Acc]]);
+			lists:reverse([cowboy_http:urldecode(S) || S <- [Path|Acc]]);
 		{Pos, _} ->
 			<< Segment:Pos/binary, _:8, Rest/bits >> = Path,
-			split_path(Rest, URLDec, [Segment|Acc])
+			split_path(Rest, [Segment|Acc])
 	end.
 
 -spec list_match(tokens(), match_rule(), bindings())
@@ -201,9 +199,7 @@ split_path_test_() ->
 		{<<"/users/42/friends">>, [<<"users">>, <<"42">>, <<"friends">>]},
 		{<<"/users/a+b/c%21d">>, [<<"users">>, <<"a b">>, <<"c!d">>]}
 	],
-	URLDecode = fun(Bin) -> cowboy_http:urldecode(Bin, crash) end,
-	[{P, fun() -> R = split_path(P, URLDecode) end}
-		|| {P, R} <- Tests].
+	[{P, fun() -> R = split_path(P) end} || {P, R} <- Tests].
 
 match_test_() ->
 	Dispatch = [
@@ -249,10 +245,9 @@ match_test_() ->
 			{ok, match_duplicate_vars, [we, {expect, two}, var, here],
 				[{var, <<"fr">>}, {var, <<"987">>}]}}
 	],
-	URLDecode = fun(Bin) -> cowboy_http:urldecode(Bin, crash) end,
 	[{lists:flatten(io_lib:format("~p, ~p", [H, P])), fun() ->
 		{ok, Handler, Opts, Binds, undefined, undefined}
-			= match(Dispatch, URLDecode, H, P)
+			= match(Dispatch, H, P)
 	end} || {H, P, {ok, Handler, Opts, Binds}} <- Tests].
 
 match_info_test_() ->
@@ -278,9 +273,8 @@ match_info_test_() ->
 		{<<"www.ninenines.eu">>, <<"/pathinfo/is/next/foo/bar">>,
 			{ok, match_path, [], [], undefined, [<<"foo">>, <<"bar">>]}}
 	],
-	URLDecode = fun(Bin) -> cowboy_http:urldecode(Bin, crash) end,
 	[{lists:flatten(io_lib:format("~p, ~p", [H, P])), fun() ->
-		R = match(Dispatch, URLDecode, H, P)
+		R = match(Dispatch, H, P)
 	end} || {H, P, R} <- Tests].
 
 -endif.

src/cowboy_http.erl

@@ -47,7 +47,7 @@
 -export([urldecode/2]).
 -export([urlencode/1]).
 -export([urlencode/2]).
--export([x_www_form_urlencoded/2]).
+-export([x_www_form_urlencoded/1]).
 
 -type version() :: {Major::non_neg_integer(), Minor::non_neg_integer()}.
 -type headers() :: [{binary(), iodata()}].
@@ -861,15 +861,14 @@ tohexu(C) when C < 17 -> $A + C - 10.
 tohexl(C) when C < 10 -> $0 + C;
 tohexl(C) when C < 17 -> $a + C - 10.
 
--spec x_www_form_urlencoded(binary(), fun((binary()) -> binary())) ->
-		list({binary(), binary() | true}).
-x_www_form_urlencoded(<<>>, _URLDecode) ->
+-spec x_www_form_urlencoded(binary()) -> list({binary(), binary() | true}).
+x_www_form_urlencoded(<<>>) ->
 	[];
-x_www_form_urlencoded(Qs, URLDecode) ->
+x_www_form_urlencoded(Qs) ->
 	Tokens = binary:split(Qs, <<"&">>, [global, trim]),
 	[case binary:split(Token, <<"=">>) of
-		[Token] -> {URLDecode(Token), true};
-		[Name, Value] -> {URLDecode(Name), URLDecode(Value)}
+		[Token] -> {urldecode(Token), true};
+		[Name, Value] -> {urldecode(Name), urldecode(Value)}
 	end || Token <- Tokens].
 
 %% Tests.
@@ -1053,9 +1052,7 @@ x_www_form_urlencoded_test_() ->
 		{<<"a=b=c=d=e&f=g">>, [{<<"a">>, <<"b=c=d=e">>}, {<<"f">>, <<"g">>}]},
 		{<<"a+b=c+d">>, [{<<"a b">>, <<"c d">>}]}
 	],
-	URLDecode = fun urldecode/1,
-	[{Qs, fun() -> R = x_www_form_urlencoded(
-		Qs, URLDecode) end} || {Qs, R} <- Tests].
+	[{Qs, fun() -> R = x_www_form_urlencoded(Qs) end} || {Qs, R} <- Tests].
 
 urldecode_test_() ->
 	U = fun urldecode/2,

src/cowboy_protocol.erl

@@ -22,9 +22,6 @@
 %%   Defaults to 5.</dd>
 %%  <dt>timeout</dt><dd>Time in milliseconds before an idle
 %%   connection is closed. Defaults to 5000 milliseconds.</dd>
-%%  <dt>urldecode</dt><dd>Function and options argument to use when decoding
-%%   URL encoded strings. Defaults to `{fun cowboy_http:urldecode/2, crash}'.
-%%   </dd>
 %% </dl>
 %%
 %% Note that there is no need to monitor these processes when using Cowboy as
@@ -56,7 +53,6 @@
 	dispatch :: cowboy_dispatcher:dispatch_rules(),
 	onrequest :: undefined | onrequest_fun(),
 	onresponse = undefined :: undefined | onresponse_fun(),
-	urldecode :: {fun((binary(), T) -> binary()), T},
 	max_empty_lines :: integer(),
 	req_keepalive = 1 :: integer(),
 	max_keepalive :: integer(),
@@ -99,8 +95,6 @@ init(ListenerPid, Socket, Transport, Opts) ->
 	OnRequest = get_value(onrequest, Opts, undefined),
 	OnResponse = get_value(onresponse, Opts, undefined),
 	Timeout = get_value(timeout, Opts, 5000),
-	URLDecDefault = {fun cowboy_http:urldecode/2, crash},
-	URLDec = get_value(urldecode, Opts, URLDecDefault),
 	ok = ranch:accept_ack(ListenerPid),
 	wait_request(<<>>, #state{listener=ListenerPid, socket=Socket,
 		transport=Transport, dispatch=Dispatch,
@@ -108,8 +102,7 @@ init(ListenerPid, Socket, Transport, Opts) ->
 		max_request_line_length=MaxRequestLineLength,
 		max_header_name_length=MaxHeaderNameLength,
 		max_header_value_length=MaxHeaderValueLength,
-		timeout=Timeout, onrequest=OnRequest, onresponse=OnResponse,
-		urldecode=URLDec}, 0).
+		timeout=Timeout, onrequest=OnRequest, onresponse=OnResponse}, 0).
 
 %% Request parsing.
 %%
@@ -421,11 +414,11 @@ parse_host(<< C, Rest/bits >>, Acc) ->
 
 request(Buffer, State=#state{socket=Socket, transport=Transport,
 		req_keepalive=ReqKeepalive, max_keepalive=MaxKeepalive,
-		onresponse=OnResponse, urldecode=URLDecode},
+		onresponse=OnResponse},
 		Method, Path, Query, Fragment, Version, Headers, Host, Port) ->
 	Req = cowboy_req:new(Socket, Transport, Method, Path, Query, Fragment,
 		Version, Headers, Host, Port, Buffer, ReqKeepalive < MaxKeepalive,
-		OnResponse, URLDecode),
+		OnResponse),
 	onrequest(Req, State, Host, Path).
 
 %% Call the global onrequest callback. The callback can send a reply,
@@ -443,10 +436,8 @@ onrequest(Req, State=#state{onrequest=OnRequest}, Host, Path) ->
 	end.
 
 -spec dispatch(cowboy_req:req(), #state{}, binary(), binary()) -> ok.
-dispatch(Req, State=#state{dispatch=Dispatch, urldecode={URLDecFun, URLDecArg}},
-		Host, Path) ->
-	case cowboy_dispatcher:match(Dispatch,
-			fun(Bin) -> URLDecFun(Bin, URLDecArg) end, Host, Path) of
+dispatch(Req, State=#state{dispatch=Dispatch}, Host, Path) ->
+	case cowboy_dispatcher:match(Dispatch, Host, Path) of
 		{ok, Handler, Opts, Bindings, HostInfo, PathInfo} ->
 			Req2 = cowboy_req:set_bindings(HostInfo, PathInfo, Bindings, Req),
 			handler_init(Req2, State, Handler, Opts);
@@ -620,7 +611,7 @@ error_terminate(Code, State=#state{socket=Socket, transport=Transport,
 	after 0 ->
 		_ = cowboy_req:reply(Code, cowboy_req:new(Socket, Transport,
 			<<"GET">>, <<>>, <<>>, <<>>, {1, 1}, [], <<>>, undefined,
-			<<>>, false, OnResponse, undefined)),
+			<<>>, false, OnResponse)),
 		ok
 	end,
 	terminate(State).

src/cowboy_req.erl

@@ -42,7 +42,7 @@
 -module(cowboy_req).
 
 %% Request API.
--export([new/14]).
+-export([new/13]).
 -export([method/1]).
 -export([version/1]).
 -export([peer/1]).
@@ -156,8 +156,7 @@
 	resp_body = <<>> :: iodata() | {non_neg_integer(), resp_body_fun()},
 
 	%% Functions.
-	onresponse = undefined :: undefined | cowboy_protocol:onresponse_fun(),
-	urldecode :: {fun((binary(), T) -> binary()), T}
+	onresponse = undefined :: undefined | cowboy_protocol:onresponse_fun()
 }).
 
 -opaque req() :: #http_req{}.
@@ -175,16 +174,15 @@
 -spec new(inet:socket(), module(), binary(), binary(), binary(), binary(),
 	cowboy_http:version(), cowboy_http:headers(), binary(),
 	inet:port_number() | undefined, binary(), boolean(),
-	undefined | cowboy_protocol:onresponse_fun(),
-	undefined | {fun(), atom()})
+	undefined | cowboy_protocol:onresponse_fun())
 	-> req().
 new(Socket, Transport, Method, Path, Query, Fragment,
 		Version, Headers, Host, Port, Buffer, CanKeepalive,
-		OnResponse, URLDecode) ->
+		OnResponse) ->
 	Req = #http_req{socket=Socket, transport=Transport, pid=self(),
 		method=Method, path=Path, qs=Query, fragment=Fragment, version=Version,
 		headers=Headers, host=Host, port=Port, buffer=Buffer,
-		onresponse=OnResponse, urldecode=URLDecode},
+		onresponse=OnResponse},
 	case CanKeepalive of
 		false ->
 			Req#http_req{connection=close};
@@ -288,10 +286,9 @@ qs_val(Name, Req) when is_binary(Name) ->
 %% missing.
 -spec qs_val(binary(), Req, Default)
 	-> {binary() | true | Default, Req} when Req::req(), Default::any().
-qs_val(Name, Req=#http_req{qs=RawQs, qs_vals=undefined,
-		urldecode={URLDecFun, URLDecArg}}, Default) when is_binary(Name) ->
-	QsVals = cowboy_http:x_www_form_urlencoded(
-		RawQs, fun(Bin) -> URLDecFun(Bin, URLDecArg) end),
+qs_val(Name, Req=#http_req{qs=RawQs, qs_vals=undefined}, Default)
+		when is_binary(Name) ->
+	QsVals = cowboy_http:x_www_form_urlencoded(RawQs),
 	qs_val(Name, Req#http_req{qs_vals=QsVals}, Default);
 qs_val(Name, Req, Default) ->
 	case lists:keyfind(Name, 1, Req#http_req.qs_vals) of
@@ -301,10 +298,8 @@ qs_val(Name, Req, Default) ->
 
 %% @doc Return the full list of query string values.
 -spec qs_vals(Req) -> {list({binary(), binary() | true}), Req} when Req::req().
-qs_vals(Req=#http_req{qs=RawQs, qs_vals=undefined,
-		urldecode={URLDecFun, URLDecArg}}) ->
-	QsVals = cowboy_http:x_www_form_urlencoded(
-		RawQs, fun(Bin) -> URLDecFun(Bin, URLDecArg) end),
+qs_vals(Req=#http_req{qs=RawQs, qs_vals=undefined}) ->
+	QsVals = cowboy_http:x_www_form_urlencoded(RawQs),
 	qs_vals(Req#http_req{qs_vals=QsVals});
 qs_vals(Req=#http_req{qs_vals=QsVals}) ->
 	{QsVals, Req}.
@@ -744,11 +739,10 @@ skip_body(Req) ->
 -spec body_qs(Req)
 	-> {ok, [{binary(), binary() | true}], Req} | {error, atom()}
 	when Req::req().
-body_qs(Req=#http_req{urldecode={URLDecFun, URLDecArg}}) ->
+body_qs(Req) ->
 	case body(Req) of
 		{ok, Body, Req2} ->
-			{ok, cowboy_http:x_www_form_urlencoded(
-				Body, fun(Bin) -> URLDecFun(Bin, URLDecArg) end), Req2};
+			{ok, cowboy_http:x_www_form_urlencoded(Body), Req2};
 		{error, Reason} ->
 			{error, Reason}
 	end.