Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix request scheme, host and port in req record #110

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:

strategy:
matrix:
otp_version: ['24', '23', '22', '21']
otp_version: ['24', '23', '22']
os: [ubuntu-latest]

steps:
Expand Down
13 changes: 7 additions & 6 deletions elvis.config
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,18 @@
]}},
{elvis_style, used_ignored_variable},
{elvis_style, no_behavior_info},
{elvis_style, state_record_and_type},
{elvis_style, atom_naming_convention,
#{ignore => [elli_http]}},
{elvis_style, state_record_and_type,
#{ignore => [elli]}},
{elvis_style, no_spec_with_records},
{elvis_style, dont_repeat_yourself},
{elvis_style, no_debug_call}
{elvis_style, no_debug_call},
{elvis_style, god_modules,
#{ignore => [elli_request]}}
],
ruleset => erl_files
},
#{dirs => ["."],
filter => "Makefile",
ruleset => makefiles
},
#{dirs => ["."],
filter => "rebar.config",
ruleset => rebar_config
Expand Down
2 changes: 1 addition & 1 deletion include/elli.hrl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
original_headers :: elli:headers(),
body :: elli:body(),
pid :: pid(),
socket :: undefined | elli_tcp:socket(),
socket :: undefined | elli_tcp:socket() | {plain, undefined},
callback :: elli_handler:callback()
}).

Expand Down
2 changes: 1 addition & 1 deletion rebar.config
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

{project_plugins, [
{covertool, "2.0.3"},
{rebar3_lint, "v0.1.10"}
{rebar3_lint, "1.0.1"}
]}.

{provider_hooks, [{pre, [{eunit, lint}]}]}.
Expand Down
1 change: 1 addition & 0 deletions src/elli.erl
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
-behaviour(gen_server).
-include("elli.hrl").
-include("elli_util.hrl").
-include_lib("kernel/include/logger.hrl").

%% API
-export([start_link/0,
Expand Down
7 changes: 7 additions & 0 deletions src/elli_example_callback.erl
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
-behaviour(elli_handler).

-include_lib("kernel/include/file.hrl").
-include_lib("kernel/include/logger.hrl").

%%
%% ELLI REQUEST CALLBACK
Expand Down Expand Up @@ -188,6 +189,12 @@ handle('GET', [<<"shorthand">>], _Req) ->
handle('GET', [<<"ip">>], Req) ->
{<<"200 OK">>, elli_request:peer(Req)};

handle('GET', [<<"scheme">>], Req) ->
{<<"200 OK">>, elli_request:scheme(Req)};

handle('GET', [<<"host">>], Req) ->
{<<"200 OK">>, elli_request:host(Req)};

handle('GET', [<<"304">>], _Req) ->
%% A "Not Modified" response is exactly like a normal response (so
%% Content-Length is included), but the body will not be sent.
Expand Down
116 changes: 49 additions & 67 deletions src/elli_http.erl
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
-module(elli_http).
-include("elli.hrl").
-include("elli_util.hrl").

-include_lib("kernel/include/logger.hrl").

%% API
-export([start_link/4]).
Expand All @@ -19,7 +19,7 @@

%% Exported for looping with a fully-qualified module name
-export([accept/4, handle_request/4, chunk_loop/1, split_args/1,
parse_path/1, keepalive_loop/3, keepalive_loop/5]).
parse_path/3, keepalive_loop/3, keepalive_loop/5]).

%% Exported for correctly handling session keep-alive for handlers
%% operating in handler mode.
Expand All @@ -31,6 +31,7 @@
-type version() :: {0, 9} | {1, 0} | {1, 1}.


-define(HOST_HEADER, <<"host">>).
-define(CONTENT_LENGTH_HEADER, <<"content-length">>).
-define(EXPECT_HEADER, <<"expect">>).
-define(CONNECTION_HEADER, <<"connection">>).
Expand Down Expand Up @@ -323,17 +324,17 @@ execute_callback(#req{callback = {Mod, Args}} = Req) ->
catch
throw:{ResponseCode, Headers, Body} when is_integer(ResponseCode) ->
{response, ResponseCode, Headers, Body};
?WITH_STACKTRACE(throw, Exc, Stacktrace)
throw:Exc:Stacktrace ->
handle_event(Mod, request_throw,
[Req, Exc, Stacktrace],
Args),
{response, 500, [], <<"Internal server error">>};
?WITH_STACKTRACE(error, Error, Stacktrace)
error:Error:Stacktrace ->
handle_event(Mod, request_error,
[Req, Error, Stacktrace],
Args),
{response, 500, [], <<"Internal server error">>};
?WITH_STACKTRACE(exit, Exit, Stacktrace)
exit:Exit:Stacktrace ->
handle_event(Mod, request_exit,
[Req, Exit, Stacktrace],
Args),
Expand Down Expand Up @@ -604,14 +605,16 @@ do_check_max_size_x2(_, _, _, _) -> ok.
Headers :: elli:headers(),
Body :: elli:body(),
V :: version(),
Socket :: elli_tcp:socket() | undefined,
Socket :: elli_tcp:socket() | {plain, undefined} | undefined,
Callback :: elli_handler:callback(),
Req :: elli:req().
mk_req(Method, PathTuple, Headers, ParsedHeaders, Body, V, Socket, {Mod, Args} = Callback) ->
case parse_path(PathTuple) of
HostHeader = get_header(?HOST_HEADER, ParsedHeaders, undefined),
SocketScheme = scheme(Socket),
case parse_path(SocketScheme, HostHeader, PathTuple) of
{ok, {Scheme, Host, Port}, {Path, URL, URLArgs}} ->
#req{method = Method, scheme = Scheme, host = Host,
port = Port, path = URL, args = URLArgs,
#req{method = Method, scheme = Scheme,
host = Host, port = Port, path = URL, args = URLArgs,
version = V, raw_path = Path, original_headers = Headers,
body = Body, pid = self(), socket = Socket,
callback = Callback, headers = ParsedHeaders};
Expand All @@ -627,6 +630,11 @@ mk_req(Method, Scheme, Host, Port, PathTuple, Headers, ParsedHeaders, Body, V, S
Req = mk_req(Method, PathTuple, Headers, ParsedHeaders, Body, V, Socket, Callback),
Req#req{scheme = Scheme, host = Host, port = Port}.

scheme({plain, _}) ->
<<"http">>;
scheme({ssl, _}) ->
<<"https">>.

%%
%% HEADERS
%%
Expand Down Expand Up @@ -708,7 +716,6 @@ is_header_defined(Key, Headers) ->
get_header(Key, Headers) ->
get_header(Key, Headers, undefined).

-ifdef(OTP_RELEASE).
get_header(Key, Headers, Default) ->
CaseFoldedKey = string:casefold(Key),
case lists:search(fun({N, _}) -> string:equal(CaseFoldedKey, N, true) end, Headers) of
Expand All @@ -717,68 +724,43 @@ get_header(Key, Headers, Default) ->
false ->
Default
end.
-else.
get_header(Key, Headers, Default) ->
CaseFoldedKey = string:casefold(Key),
case search(fun({N, _}) -> string:equal(CaseFoldedKey, N, true) end, Headers) of
{value, {_, Value}} ->
Value;
false ->
Default
end.

search(Pred, [Hd|Tail]) ->
case Pred(Hd) of
true -> {value, Hd};
false -> search(Pred, Tail)
end;
search(Pred, []) when is_function(Pred, 1) ->
false.
-endif.
default_scheme_port(<<"http">>) ->
80;
default_scheme_port(<<"https">>) ->
443;
default_scheme_port(_) ->
undefined.

%%
%% PATH HELPERS
%%

-ifdef(OTP_RELEASE).
-if(?OTP_RELEASE >= 22).
parse_path({abs_path, FullPath}) ->
URIMap = uri_string:parse(FullPath),
Host = maps:get(host, URIMap, undefined),
Scheme = maps:get(scheme, URIMap, undefined),
Path = maps:get(path, URIMap, <<>>),
Query = maps:get(query, URIMap, <<>>),
Port = maps:get(port, URIMap, case Scheme of http -> 80; https -> 443; _ -> undefined end),
{ok, {Scheme, Host, Port}, {Path, split_path(Path), uri_string:dissect_query(Query)}};
parse_path({absoluteURI, Scheme, Host, Port, Path}) ->
setelement(2, parse_path({abs_path, Path}), {Scheme, Host, Port});
parse_path(_) ->
{error, unsupported_uri}.
-else.
parse_path({abs_path, FullPath}) ->
Parsed = case binary:split(FullPath, [<<"?">>]) of
[URL] -> {FullPath, split_path(URL), []};
[URL, Args] -> {FullPath, split_path(URL), split_args(Args)}
end,
{ok, {undefined, undefined, undefined}, Parsed};
parse_path({absoluteURI, Scheme, Host, Port, Path}) ->
setelement(2, parse_path({abs_path, Path}), {Scheme, Host, Port});
parse_path(_) ->
{error, unsupported_uri}.
-endif.
-else.
%% same as else branch above. can drop this when only OTP 21+ is supported
parse_path({abs_path, FullPath}) ->
Parsed = case binary:split(FullPath, [<<"?">>]) of
[URL] -> {FullPath, split_path(URL), []};
[URL, Args] -> {FullPath, split_path(URL), split_args(Args)}
end,
{ok, {undefined, undefined, undefined}, Parsed};
parse_path({absoluteURI, Scheme, Host, Port, Path}) ->
setelement(2, parse_path({abs_path, Path}), {Scheme, Host, Port});
parse_path(_) ->
{error, unsupported_uri}.
-endif.
parse_path(SocketScheme, HostHeader, {abs_path, FullPath}) ->
case uri_string:parse(FullPath) of
URIMap when not is_map_key(host, URIMap) ,
SocketScheme =/= undefined ,
HostHeader =/= undefined ->
HostMap = uri_string:parse(
unicode:characters_to_binary([SocketScheme, "://", HostHeader])),
Host = maps:get(host, HostMap, HostHeader),
Scheme = maps:get(scheme, HostMap, SocketScheme),
Port = maps:get(port, HostMap, default_scheme_port(Scheme));
URIMap ->
Host = maps:get(host, URIMap, HostHeader),
Scheme = maps:get(scheme, URIMap, SocketScheme),
Port = maps:get(port, URIMap, default_scheme_port(Scheme))
end,

Path = maps:get(path, URIMap, <<>>),
Query = maps:get(query, URIMap, <<>>),

{ok, {Scheme, Host, Port},
{Path, split_path(Path), uri_string:dissect_query(Query)}};
parse_path(_Scheme, _HostHeader, {absoluteURI, Scheme, Host, Port, Path}) ->
setelement(2, parse_path(undefined, undefined, {abs_path, Path}), {Scheme, Host, Port});
parse_path(_, _, _) ->
{error, unsupported_uri}.

split_path(Path) ->
[P || P <- binary:split(Path, [<<"/">>], [global]),
Expand Down Expand Up @@ -813,7 +795,7 @@ handle_event(Mod, Name, EventArgs, ElliArgs) ->
try
Mod:handle_event(Name, EventArgs, ElliArgs)
catch
?WITH_STACKTRACE(EvClass, EvError, Stacktrace)
EvClass:EvError:Stacktrace ->
?LOG_ERROR("~p:handle_event/3 crashed ~p:~p~n~p",
[Mod, EvClass, EvError, Stacktrace])
end.
Expand Down
2 changes: 1 addition & 1 deletion src/elli_test.erl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ call(Method, Path, Headers, Body, Opts) ->
Callback = proplists:get_value(callback, Opts),
CallbackArgs = proplists:get_value(callback_args, Opts),
Req = elli_http:mk_req(Method, {abs_path, Path}, Headers, Headers,
Body, {1, 1}, undefined, {Callback, CallbackArgs}),
Body, {1, 1}, {plain, undefined}, {Callback, CallbackArgs}),
ok = Callback:handle_event(elli_startup, [], CallbackArgs),
Callback:handle(Req, CallbackArgs).

Expand Down
15 changes: 0 additions & 15 deletions src/elli_util.hrl
Original file line number Diff line number Diff line change
@@ -1,17 +1,2 @@

-ifdef(OTP_RELEASE).
-include_lib("kernel/include/logger.hrl").
-else.
-define(LOG_ERROR(Str), error_logger:error_msg(Str)).
-define(LOG_ERROR(Format,Data), error_logger:error_msg(Format, Data)).
-define(LOG_INFO(Format,Data), error_logger:info_msg(Format, Data)).
-endif.

-ifdef(OTP_RELEASE).
-define(WITH_STACKTRACE(T, R, S), T:R:S ->).
-else.
-define(WITH_STACKTRACE(T, R, S), T:R -> S = erlang:get_stacktrace(),).
-endif.

%% Bloody useful
-define(IF(Test,True,False), case Test of true -> True; false -> False end).
9 changes: 8 additions & 1 deletion test/elli_ssl_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ elli_ssl_test_() ->
?_test(hello_world()),
?_test(chunked()),
?_test(sendfile()),
?_test(acceptor_leak_regression())
?_test(acceptor_leak_regression()),
?_test(check_scheme_parsing())
]}
]}.

Expand All @@ -33,6 +34,12 @@ hello_world() ->
?assertMatch(200, status(Response)),
?assertMatch({ok, 200, _, _}, Response).

check_scheme_parsing() ->
Response = hackney:get("https://localhost:3443/scheme",
[], <<>>, [insecure]),
?assertMatch(200, status(Response)),
?assertMatch(<<"https">>, body(Response)).

chunked() ->
Expected = <<"chunk10chunk9chunk8chunk7chunk6chunk5chunk4chunk3chunk2chunk1">>,

Expand Down
24 changes: 17 additions & 7 deletions test/elli_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,8 @@
-define(VTB(T1, T2, LB, UB),
time_diff_to_micro_seconds(T1, T2) >= LB andalso
time_diff_to_micro_seconds(T1, T2) =< UB).
-ifdef(OTP_RELEASE).

-include_lib("kernel/include/logger.hrl").
-else.
-define(LOG_ERROR(Str), error_logger:error_msg(Str)).
-define(LOG_ERROR(Format,Data), error_logger:error_msg(Format, Data)).
-define(LOG_INFO(Format,Data), error_logger:info_msg(Format, Data)).
-endif.

time_diff_to_micro_seconds(T1, T2) ->
erlang:convert_time_unit(
Expand Down Expand Up @@ -63,7 +58,9 @@ elli_test_() ->
?_test(get_pipeline()),
?_test(head()),
?_test(no_body()),
?_test(sends_continue())
?_test(sends_continue()),
?_test(check_scheme_parsing()),
?_test(check_host_parsing())
]}
]}.

Expand Down Expand Up @@ -108,12 +105,14 @@ accessors_test_() ->
RawPath = <<"/foo/bar">>,
Headers = [{<<"content-type">>, <<"application/x-www-form-urlencoded">>}],
Method = 'POST',
Scheme = <<"http">>,
Body = <<"name=knut%3D">>,
Name = <<"knut=">>,
Req1 = #req{raw_path = RawPath,
original_headers = Headers,
headers = Headers,
method = Method,
scheme = Scheme,
body = Body},
Args = [{<<"name">>, Name}],
Req2 = #req{original_headers = Headers, headers = Headers, args = Args, body = <<>>},
Expand All @@ -123,6 +122,7 @@ accessors_test_() ->
?_assertMatch(RawPath, elli_request:raw_path(Req1)),
?_assertMatch(Headers, elli_request:headers(Req1)),
?_assertMatch(Method, elli_request:method(Req1)),
?_assertMatch(Scheme, elli_request:scheme(Req1)),
?_assertMatch(Body, elli_request:body(Req1)),
?_assertMatch(Args, elli_request:post_args_decoded(Req1)),
?_assertMatch(undefined, elli_request:post_arg(<<"foo">>, Req1)),
Expand Down Expand Up @@ -595,6 +595,16 @@ sends_continue() ->
?assertMatch({ok, ExpectedResponse},
gen_tcp:recv(Socket, size(ExpectedResponse))).

check_scheme_parsing() ->
Response = hackney:get("http://localhost:3001/scheme"),
?assertMatch(200, status(Response)),
?assertMatch(<<"http">>, body(Response)).

check_host_parsing() ->
Response = hackney:get("http://localhost:3001/host"),
?assertMatch(200, status(Response)),
?assertMatch(<<"localhost">>, body(Response)).

%%% Slow client, sending only the specified byte size every millisecond

start_slow_client(Port, Url) ->
Expand Down