Browse files

Remove the urldecode cowboy_protocol option

This allows inconsistent behavior and is not used enough to be supported.
  • Loading branch information...
1 parent c326a19 commit ce9aff19f036310dd33d00d14cff35a559b8ccb1 @essen essen committed Sep 29, 2012
Showing with 54 additions and 78 deletions.
  1. +29 −35 src/cowboy_dispatcher.erl
  2. +7 −10 src/cowboy_http.erl
  3. +6 −15 src/cowboy_protocol.erl
  4. +12 −18 src/cowboy_req.erl
View
64 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.
View
17 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,
View
21 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,17 +95,14 @@ 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,
max_empty_lines=MaxEmptyLines, max_keepalive=MaxKeepalive,
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).
View
30 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.

0 comments on commit ce9aff1

Please sign in to comment.