From 3c10d5488396aaeeae7666aa25361c227db039f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Mon, 24 Sep 2012 02:58:33 +0200 Subject: Removal of binary:split from cowboy_dispatcher and small optimizations The internal host_tokens value now has host tokens in reverse order compared to before. This allows us to remove one lists:reverse call. --- src/cowboy_dispatcher.erl | 94 +++++++++++++++++++++++++---------------------- test/dispatcher_prop.erl | 8 ++-- 2 files changed, 56 insertions(+), 46 deletions(-) diff --git a/src/cowboy_dispatcher.erl b/src/cowboy_dispatcher.erl index 38648ca..c52aba9 100644 --- a/src/cowboy_dispatcher.erl +++ b/src/cowboy_dispatcher.erl @@ -44,13 +44,25 @@ split_host(Host) -> case binary:match(Host, <<":">>) of nomatch -> - {binary:split(Host, <<".">>, [global, trim]), Host, undefined}; + {do_split_host(Host, []), Host, undefined}; {Pos, _} -> << Host2:Pos/binary, _:8, Port/bits >> = Host, - {binary:split(Host2, <<".">>, [global, trim]), Host2, + {do_split_host(Host2, []), Host2, list_to_integer(binary_to_list(Port))} end. +do_split_host(Host, Acc) -> + case binary:match(Host, <<".">>) of + nomatch when Host =:= <<>> -> + Acc; + nomatch -> + [Host|Acc]; + {Pos, _} -> + << Segment:Pos/binary, _:8, Rest/bits >> = Host, + false = byte_size(Segment) == 0, + do_split_host(Rest, [Segment|Acc]) + end. + %% @doc Split a path into a list of path segments. %% %% Following RFC2396, this function may return path segments containing any @@ -67,13 +79,18 @@ split_path(Path, URLDec) -> {do_split_path(Path2, URLDec), Path2, Qs} end. --spec do_split_path(binary(), fun((binary()) -> binary())) -> tokens(). -do_split_path(RawPath, URLDec) -> - EncodedPath = case binary:split(RawPath, <<"/">>, [global, trim]) of - [<<>>|Path] -> Path; - Path -> Path - end, - [URLDec(Token) || Token <- EncodedPath]. +do_split_path(<< "/", Path/bits >>, URLDec) -> + do_split_path(Path, URLDec, []). +do_split_path(Path, URLDec, Acc) -> + case binary:match(Path, <<"/">>) of + nomatch when Path =:= <<>> -> + lists:reverse([URLDec(S) || S <- Acc]); + nomatch -> + lists:reverse([URLDec(S) || S <- [Path|Acc]]); + {Pos, _} -> + << Segment:Pos/binary, _:8, Rest/bits >> = Path, + do_split_path(Rest, URLDec, [Segment|Acc]) + end. %% @doc Match hostname tokens and path tokens against dispatch rules. %% @@ -112,7 +129,7 @@ match(_Host, _Path, []) -> match(_Host, Path, [{'_', PathMatchs}|_Tail]) -> match_path(Path, PathMatchs, [], undefined); match(Host, Path, [{HostMatch, PathMatchs}|Tail]) -> - case try_match(host, Host, HostMatch) of + case list_match(Host, lists:reverse(HostMatch), []) of false -> match(Host, Path, Tail); {true, HostBinds, undefined} -> @@ -134,7 +151,7 @@ match_path(_Path, [{'_', Handler, Opts}|_Tail], HostBinds, HostInfo) -> match_path('*', [{'*', Handler, Opts}|_Tail], HostBinds, HostInfo) -> {ok, Handler, Opts, HostBinds, HostInfo, undefined}; match_path(Path, [{PathMatch, Handler, Opts}|Tail], HostBinds, HostInfo) -> - case try_match(path, Path, PathMatch) of + case list_match(Path, PathMatch, []) of false -> match_path(Path, Tail, HostBinds, HostInfo); {true, PathBinds, PathInfo} -> @@ -143,13 +160,6 @@ match_path(Path, [{PathMatch, Handler, Opts}|Tail], HostBinds, HostInfo) -> %% Internal. --spec try_match(host | path, tokens(), match_rule()) - -> {true, bindings(), undefined | tokens()} | false. -try_match(host, List, Match) -> - list_match(lists:reverse(List), lists:reverse(Match), []); -try_match(path, List, Match) -> - list_match(List, Match, []). - -spec list_match(tokens(), match_rule(), bindings()) -> {true, bindings(), undefined | tokens()} | false. %% Atom '...' matches any trailing path, stop right now. @@ -179,23 +189,19 @@ split_host_test_() -> %% {Host, Result} Tests = [ {<<"">>, {[], <<"">>, undefined}}, - {<<".........">>, {[], <<".........">>, undefined}}, {<<"*">>, {[<<"*">>], <<"*">>, undefined}}, {<<"cowboy.ninenines.eu">>, - {[<<"cowboy">>, <<"ninenines">>, <<"eu">>], + {[<<"eu">>, <<"ninenines">>, <<"cowboy">>], <<"cowboy.ninenines.eu">>, undefined}}, - {<<"ninenines..eu">>, - {[<<"ninenines">>, <<>>, <<"eu">>], - <<"ninenines..eu">>, undefined}}, {<<"ninenines.eu">>, - {[<<"ninenines">>, <<"eu">>], <<"ninenines.eu">>, undefined}}, + {[<<"eu">>, <<"ninenines">>], <<"ninenines.eu">>, undefined}}, {<<"ninenines.eu:8080">>, - {[<<"ninenines">>, <<"eu">>], <<"ninenines.eu">>, 8080}}, + {[<<"eu">>, <<"ninenines">>], <<"ninenines.eu">>, 8080}}, {<<"a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z">>, - {[<<"a">>, <<"b">>, <<"c">>, <<"d">>, <<"e">>, <<"f">>, <<"g">>, - <<"h">>, <<"i">>, <<"j">>, <<"k">>, <<"l">>, <<"m">>, <<"n">>, - <<"o">>, <<"p">>, <<"q">>, <<"r">>, <<"s">>, <<"t">>, <<"u">>, - <<"v">>, <<"w">>, <<"x">>, <<"y">>, <<"z">>], + {[<<"z">>, <<"y">>, <<"x">>, <<"w">>, <<"v">>, <<"u">>, <<"t">>, + <<"s">>, <<"r">>, <<"q">>, <<"p">>, <<"o">>, <<"n">>, <<"m">>, + <<"l">>, <<"k">>, <<"j">>, <<"i">>, <<"h">>, <<"g">>, <<"f">>, + <<"e">>, <<"d">>, <<"c">>, <<"b">>, <<"a">>], <<"a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z">>, undefined}} ], @@ -203,6 +209,8 @@ split_host_test_() -> split_host_fail_test_() -> Tests = [ + <<".........">>, + <<"ninenines..eu">>, <<"ninenines.eu:owns">>, <<"ninenines.eu: owns">>, <<"ninenines.eu:42fun">>, @@ -263,22 +271,22 @@ match_test_() -> %% {Host, Path, Result} Tests = [ {[<<"any">>], [], {ok, match_any, [], []}}, - {[<<"www">>, <<"any">>, <<"ninenines">>, <<"eu">>], + {[<<"eu">>, <<"ninenines">>, <<"any">>, <<"www">>], [<<"users">>, <<"42">>, <<"mails">>], {ok, match_any_subdomain_users, [], []}}, - {[<<"www">>, <<"ninenines">>, <<"eu">>], + {[<<"eu">>, <<"ninenines">>, <<"www">>], [<<"users">>, <<"42">>, <<"mails">>], {ok, match_any, [], []}}, - {[<<"www">>, <<"ninenines">>, <<"eu">>], [], {ok, match_any, [], []}}, - {[<<"www">>, <<"any">>, <<"ninenines">>, <<"eu">>], + {[<<"eu">>, <<"ninenines">>, <<"www">>], [], {ok, match_any, [], []}}, + {[<<"eu">>, <<"ninenines">>, <<"any">>, <<"www">>], [<<"not_users">>, <<"42">>, <<"mails">>], {error, notfound, path}}, - {[<<"ninenines">>, <<"eu">>], [], {ok, match_extend, [], []}}, - {[<<"ninenines">>, <<"eu">>], [<<"users">>, <<"42">>, <<"friends">>], + {[<<"eu">>, <<"ninenines">>], [], {ok, match_extend, [], []}}, + {[<<"eu">>, <<"ninenines">>], [<<"users">>, <<"42">>, <<"friends">>], {ok, match_extend_users_friends, [], [{id, <<"42">>}]}}, - {[<<"erlang">>, <<"fr">>], '_', + {[<<"fr">>, <<"erlang">>], '_', {ok, match_erlang_ext, [], [{ext, <<"fr">>}]}}, {[<<"any">>], [<<"users">>, <<"444">>, <<"friends">>], {ok, match_users_friends, [], [{id, <<"444">>}]}}, - {[<<"ninenines">>, <<"fr">>], [<<"threads">>, <<"987">>], + {[<<"fr">>, <<"ninenines">>], [<<"threads">>, <<"987">>], {ok, match_duplicate_vars, [we, {expect, two}, var, here], [{var, <<"fr">>}, {var, <<"987">>}]}} ], @@ -296,19 +304,19 @@ match_info_test_() -> ]} ], Tests = [ - {[<<"ninenines">>, <<"eu">>], [], + {[<<"eu">>, <<"ninenines">>], [], {ok, match_any, [], [], [], undefined}}, - {[<<"bugs">>, <<"ninenines">>, <<"eu">>], [], + {[<<"eu">>, <<"ninenines">>, <<"bugs">>], [], {ok, match_any, [], [], [<<"bugs">>], undefined}}, - {[<<"cowboy">>, <<"bugs">>, <<"ninenines">>, <<"eu">>], [], + {[<<"eu">>, <<"ninenines">>, <<"bugs">>, <<"cowboy">>], [], {ok, match_any, [], [], [<<"cowboy">>, <<"bugs">>], undefined}}, - {[<<"www">>, <<"ninenines">>, <<"eu">>], + {[<<"eu">>, <<"ninenines">>, <<"www">>], [<<"pathinfo">>, <<"is">>, <<"next">>], {ok, match_path, [], [], undefined, []}}, - {[<<"www">>, <<"ninenines">>, <<"eu">>], + {[<<"eu">>, <<"ninenines">>, <<"www">>], [<<"pathinfo">>, <<"is">>, <<"next">>, <<"path_info">>], {ok, match_path, [], [], undefined, [<<"path_info">>]}}, - {[<<"www">>, <<"ninenines">>, <<"eu">>], + {[<<"eu">>, <<"ninenines">>, <<"www">>], [<<"pathinfo">>, <<"is">>, <<"next">>, <<"foo">>, <<"bar">>], {ok, match_path, [], [], undefined, [<<"foo">>, <<"bar">>]}} ], diff --git a/test/dispatcher_prop.erl b/test/dispatcher_prop.erl index 163fcd8..26c4f87 100644 --- a/test/dispatcher_prop.erl +++ b/test/dispatcher_prop.erl @@ -49,12 +49,14 @@ prop_split_host_symmetric() -> ?FORALL(Server, server(), begin case cowboy_dispatcher:split_host(Server) of {Tokens, RawHost, undefined} -> - (Server == RawHost) and (Server == binary_join(Tokens, ".")); + (Server == RawHost) + and (Server == binary_join(lists:reverse(Tokens), ".")); {Tokens, RawHost, Port} -> PortBin = (list_to_binary(":" ++ integer_to_list(Port))), (Server == << RawHost/binary, PortBin/binary >>) - and (Server == << (binary_join(Tokens, "."))/binary, - PortBin/binary >>) + and (Server == + << (binary_join(lists:reverse(Tokens), "."))/binary, + PortBin/binary >>) end end). %% Internal. -- cgit v1.2.3