From ce9aff19f036310dd33d00d14cff35a559b8ccb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Sat, 29 Sep 2012 13:03:45 +0200 Subject: Remove the urldecode cowboy_protocol option This allows inconsistent behavior and is not used enough to be supported. --- src/cowboy_dispatcher.erl | 64 +++++++++++++++++++++-------------------------- src/cowboy_http.erl | 17 ++++++------- src/cowboy_protocol.erl | 21 +++++----------- src/cowboy_req.erl | 30 +++++++++------------- 4 files changed, 54 insertions(+), 78 deletions(-) (limited to 'src') diff --git a/src/cowboy_dispatcher.erl b/src/cowboy_dispatcher.erl index f5dfcd9..cfb8fb6 100644 --- a/src/cowboy_dispatcher.erl +++ b/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 '...' 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 / if, and only if, a / 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. diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl index 6ba4834..e0b1632 100644 --- a/src/cowboy_http.erl +++ b/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, diff --git a/src/cowboy_protocol.erl b/src/cowboy_protocol.erl index 77ddc2e..a9ce69c 100644 --- a/src/cowboy_protocol.erl +++ b/src/cowboy_protocol.erl @@ -22,9 +22,6 @@ %% Defaults to 5. %%
timeout
Time in milliseconds before an idle %% connection is closed. Defaults to 5000 milliseconds.
-%%
urldecode
Function and options argument to use when decoding -%% URL encoded strings. Defaults to `{fun cowboy_http:urldecode/2, crash}'. -%%
%% %% %% 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). diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl index 3bb5d9a..05ea9a1 100644 --- a/src/cowboy_req.erl +++ b/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. -- cgit v1.2.3