elli-lib / elli Goto Github PK

Fix sendfile/0 and sendfile_range/0 in elli_tests.

https://github.com/elli-lib/elli/blob/develop/src/elli_http.erl#L583 here.
timestamp is highly unreliable for measuring duration (i.e. time warps).

with timing fix

As in elli-lib/elli_fileserve#6, there should be a generic, configurable facility for this in elli.

I'm having issues keeping state in the lifecycle of a request. More precisely, I'd like to be able to pass data between handlers, and between init, preprocess, handle and postprocess in each handler.

It seems to be a common pattern in Erlang to explicitely propagate state, for example in gen_server, but it is not the case in elli.

#35 suggests that the only way is the process dictionary, but the Erlang website has good arguments not to overuse it (https://www.erlang.org/course/advanced#dict).

Is there anyone currently working on a patch to add an explicit state ? Would this kind of patch be accepted ?

See discussion on #30.

It appears that when attempting to generate an HTTP status code binary description, Elli is unable to handle any codes that are not included in the list of handled function heads in elli_http.erl or a binary.

When receiving and attempting to handle non-standard or uncommon HTTP status codes, the handler crashes.

Since elli.erl declares a response type to be any integer between 100..999 inclusive, it should have a valid fall-through behavior for handling integer status codes that are not expected by that prepared list of status/1 function heads here

because elli_tcp:accept/3 sends the 'accepted' cast after successful ssl:transport_accept/2 but before ssl:ssl_accept/2 succeeds, any failed TLS handshake will result in an additional process. The process that called elli_tcp:accept/3 will continue to loop (in elli_http:accept/4 ).

This is a problem because anything that can reach the tcp listening port can create an arbitrary number of processes in the erlang vm:

[sg@ptha /tmp/elli]$ rebar3 shell
===> Verifying dependencies...
===> Compiling elli
Erlang/OTP 23 [erts-11.0] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [hipe] [dtrace]

Eshell V11.0  (abort with ^G)
1> Opts=[{callback,elli_example_callback}, ssl,
1>       {keyfile,"test/server_key.pem"}, {certfile,"test/server_cert.pem"}].
[{callback,elli_example_callback},
 ssl,
 {keyfile,"test/server_key.pem"},
 {certfile,"test/server_cert.pem"}]
2> {ok, Elli} = elli:start_link(Opts).
{ok,<0.194.0>}
3> logger:set_primary_config(#{filter_default => log,filters => [],level => error}).
ok
4> elli:get_open_reqs(Elli).
{ok,0}
5> elli:get_acceptors(Elli).
{ok,[<0.212.0>,<0.206.0>,<0.210.0>,<0.198.0>,<0.208.0>,
     <0.204.0>,<0.203.0>,<0.202.0>,<0.197.0>,<0.207.0>,<0.199.0>,
     <0.201.0>,<0.209.0>,<0.211.0>,<0.200.0>,<0.213.0>,<0.215.0>,
     <0.214.0>,<0.216.0>,<0.205.0>]}
6> ClientAbort="openssl s_client -connect localhost:8080 -verify_return_error".
"openssl s_client -connect localhost:8080 -verify_return_error"
7> lists:foreach(fun(_) -> os:cmd(ClientAbort) end, lists:seq(1, 200)).
ok
8> {ok, L} = elli:get_acceptors(Elli).
{ok,[<0.541.0>,<0.937.0>,<0.713.0>,<0.305.0>,<0.485.0>,
     <0.933.0>,<0.765.0>,<0.753.0>,<0.481.0>,<0.833.0>,<0.237.0>,
     <0.629.0>,<0.949.0>,<0.529.0>,<0.273.0>,<0.393.0>,<0.601.0>,
     <0.229.0>,<0.789.0>,<0.397.0>,<0.505.0>,<0.865.0>,<0.969.0>,
     <0.561.0>,<0.981.0>,<0.293.0>,<0.705.0>|...]}
9> length(L).
220
10> elli:get_open_reqs(Elli).
{ok,200}
11> 

Since OTP 21,
ssl:ssl_accept/2: deprecated; use ssl:handshake/2 instead

a little bit of explanation about this change can be found here

it's only a warning during compilation, but it's you use warning_as_error for production mode, it might be a problem;

handshake method not exist before OTP 21, it's not very easy to switch to this version.
do you have roadmap to take all compatibility with OTP 21 ?

As in elli-lib/elli_fileserve#5, I think we should have generic support for this in elli itself. elli_fileserve et al. could then hook into it.

Is there any objection to this? Maps have been available for the last 3 major releases, going on 4 major releases soon enough. Proplists don't make much sense for things that don't need the true/false functionality.

We now only test on 17+, but it should work on 15 and 16.

I believe that the documentation is missing the changelog entry for version 3.2.0.

Add API so plugins can instrument themselves, just like main Elli does. Something generic, or just use Prometheus/Prometheus data model?

See #17 for discussion and bad solution.

We're using Elli in a project and finding a couple of functions currently private to the elli_http module that would be useful for us to have access to and hoping you agree there's no reason not to expose them for external use.

elli_http:send_file/5 - this one looks like it could be exposed already to mirror the exported elli_http:send_response/4 and is already fully spec'd and documented.

elli_http:close_or_keepalive/2 - this one is useful for handlers operating in handover mode where the implementation will be handling the response but wants to properly honor the session keepalive configuration based on the header with a sane default based on HTTP version.

I've prepared a PR that should take care of this or collect comments/feedback if this isn't desirable as-is.

Thanks!

Elli middlewares export any of init/2, preprocess/2 and postprocess/3, and many elli_handler modules are also elli_middleware modules. Right now it's sort of casual. The elli_middleware module checks whether these functions are exported and acts accordingly. I think it'd be cool to spec out a proper behaviour so we can validate code at compile time and such.

• 2.0.0: v1.0.5...2.0.0
• 2.0.1: 2.0.0...2.0.1
• Add CHANGELOG link to Hex metadata

Currently there appears to be a timing issue in executing the elli_ssl_tests. This was observed during trivial changes refactoring function internals without changing their end result and exposing private functions as public. It was also observed on different OTP versions within the matrix execution running in GitHub actions (the first time on OTP 22.3.4.2, the second time on OTP 21.3.8.16) and was not reproducible locally (running on 2017 Macbook Pro with i5, 8 gb RAM). At the time of the failures, GitHub actions is set to execute the build matrix on "ubuntu-latest" which at time of writing is still 18.04, version 20210111.1 in GH actions with a pending update to 20.04. (https://github.com/actions/virtual-environments/blob/ubuntu18/20210111.1/images/linux/Ubuntu1804-README.md)

Re-running the tests with subsequent builds successfully resolved the issue temporarily.

I've wanted to bring this up for a while and just realized I have a perfect PR on another project that shows what I'd want to do, so I'm opening this with https://github.com/adoptingerlang/service_discovery/pull/9/files

The reason I've been hesitant to bring it up is Elli currently has no dependencies and I'd expect adding a dependency would be controversial because of that.

The dependency would be https://github.com/fishcakez/acceptor_pool/

What it brings to Elli you can see if you look at the PR and how the pool and listen socket are started separately (in that order) allowing for shutdown to first close the listen socket, preventing new requests, and then (with an optional grace period) shutdown the other connections.

I'm seeing request durations (request_end - request_start) of multiple seconds for sub-second page refreshes in browsers, but not curl.

That metric seemed like best measure of true response time (as noted in original elli sources).

What is going on?

Here's the prometheus.erl code:

handle_event(request_complete,
             [Req, ResponseCode, _ResponseHeaders, _ResponseBody,
              {Timings, _Sizes}],
             _Config) ->
    prometheus_counter:inc(http_requests_total,
                           [ResponseCode, rawpath(Req)]),
    Start = proplists:get_value(request_start, Timings),
    End = proplists:get_value(request_end, Timings),
    NativeDelta = End - Start,
    io:format("handle_event: ~p  ~p  ~p~n",
              [ResponseCode,
               erlang:convert_time_unit(NativeDelta, native, micro_seconds),
               rawpath(Req)]),
    prometheus_histogram:observe(response_time_in_microseconds,
                                 [rawpath(Req)], NativeDelta),
    ok;

the Erlang interpreter output:

handle_event: 200  1990847  <<"/hello/world">>
handle_event: 200  2740893  <<"/metrics">>
handle_event: 200  1239785  <<"/metrics">>
handle_event: 200  2138739  <<"/hello/world">>

and if I use curl instead, I get output like this:

handle_event: 200  1409  <<"/hello/world">>
handle_event: 200  112  <<"/hello/world">>
handle_event: 404  100  <<"/hmetrics">>
handle_event: 200  6334  <<"/metrics">>
handle_event: 200  6614  <<"/metrics">>

Chrome also is high:

handle_event: 200  183414  <<"/hello/world">>
handle_event: 404  350253  <<"/favicon.ico">>
handle_event: 200  7395448  <<"/metrics">>
handle_event: 200  3918845  <<"/metrics">>
handle_event: 200  1744633  <<"/metrics">>

Hi there!

I'm enjoying Elli after switching a couple projects to it from Cowboy.

My main problem has been lack of documentation, for example I needed to read the source code to determine how to use middleware (and I feel I'm still missing many of the details).

I feel this results in Elli being less accessible and less likely to be used, especially to people who are more used to Elixir rather than Erlang. Could we improve the docs? Perhaps include a series of examples in the README?

I'd be happy to help here.

Cheers,
Louis

Hi guys,

I'm trying to upgrade from 3.0 to 3.1, but I'm getting the following error on both my dev machine and in CI:

===> Fetching rebar_erl_vsn ({pkg,<<"rebar_erl_vsn">>,<<"0.1.7">>})
===> Downloaded package, caching at /home/john/.cache/rebar3/hex/default/packages/rebar_erl_vsn-0.1.7.tar
===> Compiling rebar_erl_vsn
===> Compiling elli
===> Unable to run pre hooks for 'compile', command 'erl_vsn' not found.
** (Mix) Could not compile dependency :elli, "/home/john/.asdf/installs/elixir/1.6.3/.mix/rebar3 bare compile --paths "/home/john/code/hc/dylan/_build/test/lib/*/ebin"" command failed. You can recompile this dependency with "mix deps.compile elli", update it with "mix deps.update elli" or clean it with "mix deps.clean elli"

It seems that since 3.0, rebar_erl_vsn was added to rebar.config, could this be the source of the issue?

I'm running Erlang 20.3.6 and Elixir 1.6.3.

Hi, i've been getting these errors intermittently but there is not crash detail.

Please how can I enable verbose logging by default on Elli?

20:34:10.457 [error] Elli request (pid #PID<0.262.0>) unexpectedly crashed:
{:badmatch,
 "POST /1111111111:AAF32ORayQp1hhh543uu1wD7AlSP9mJ333 HTTP/1.1\r\nHost: 75.128.232.93\r\nContent-Type: application/json\r\nContent-Length: 297\r\nConnection: keep-alive\r\nAccept-Encoding: gzip, deflate\r\n\r\n"}

I have installed 3.1.0 from hex but cannot see a changelog entry?

Right now it's possible to "load" the socket with large headers, so I don't think there are any checks for this scenario (other than headers count < 100).

It's be great to have HTTP2 support in Elli!

This seems like quite an ambitious addition but I thought I'd open an issue anyway :)

If there's a need, it shouldn't be too difficult to add support for OTP <18 via preprocessor directives or similar. I have no need for it personally, but I'll leave this issue open in case someone does.

See @tsloughter's comment re: naming collisions.

Hi. Do you suppose we'll have a release soon? master seems to have fallen a bit behind develop, but in the meanwhile 3.1.0 and 3.2.0 have been released and post-3.2.0 code has been added to develop. Thanks.

While our fork is the most active one I'm sure we can learn people's needs by looking at the other forks:
https://github.com/knutin/elli/network.

I think the ultimate goal may be to incorporate all features and fixes accumulated so far.

elli_request:peer/1 uses X-Forwarded-For as the primary source of the request origin.

This is concerning because the header can not be trusted so applications using elli are not able to rely on elli_request:peer/1 when IP addresses are used in scoring systems and blacklists.

At the very least, it would be helplful to have a separate function that uses elli_tcp:peername(Socket) to exclude the peer address.

Ideally though, X-Forwarded-For should not be used in peer/1.

Thank you!

Hello!

Is there a recommended way of handling errors so that an application using Elli serves 500 with a suitable body whenever the handler crashes?

Thanks,
Louis

Don’t let #33 happen again.

The question/confusion is coming up again.

@knutin would you be willing to have knutin/elli redirect to elli-lib/elli? Or put in the knutin/elli title or readme to go to elli-lib/elli?

We "officially" support 18+, so let's drop any lingering obviated conditional logic,
e.g. the -ifdef('16.0') in #55.

Looks like iolist() doesn't mean the same thing anymore.

-type body() :: binary() | iolist().

@anha0825 is merging PRs to knutin/elli. We need to bring them over and figure out if this is going to be the canonical or if we need to be sure to track upstream so we don't miss any changes that get merged.

Per comment on #61.

Depends on #93, at least. This is just a reminder for now.

👋

Something like

-spec port(Server :: atom | pid) -> {ok, inet:port_number()} | {error, any}.
port(Server) ->
  gen_server:call(Server, port).
  
%% @hidden
handle_call(port, _From, State) ->
  {reply, elli_tcp:port(State#state.socket), State}.

and in elli_tcp:

port({plain, Socket}) -> inet:port(Socket).
port({ssl, Socket}) -> {error, not_supported}. % don't care about ssl

I mostly would need it to run tests concurrently in elixir:

  setup_all do
    {:ok, elli_pid} = Plug.Adapters.Elli.http(__MODULE__, [], port: 0)
    {:ok, port} = :elli.port(elli_pid)
    on_exit(fn ->
      :ok = :elli.shutdown(elli_pid)
    end)

    {:ok, port: port}
  end

  # ...

  test "does that thing", %{port: port} do
    resp = TestClient.get("http://localhost:#{port}/hello")
    assert resp.status_code == 200
    assert resp.body == "hello"
  end

Or maybe there are other ways for elli to bind to a random port such that I'd know about the port elli did bind to?

The elli-lib version has several improvements over the currently published version (1.0.5), namely wrt type naming collisions. I've merged develop into master and think it's good to go. The open issues either pertain to documentation or can wait.

@knutin, would you prefer I open a PR to knutin/elli, or should we just publish from here? Also, by we, I mean you, since you're the owner of the Hex package. 😄

Response size is trivial - Headers iolist + Body iolist, but for requests Elli uses this built-in 'event-based' parser. Currently I have no idea how to calculate request size.

That way code search and such will work.

Please can we add scheme, host, port metadata to elli request record type (:req)?

Why is it being discarded here? https://github.com/elli-lib/elli/blob/develop/src/elli_http.erl#L688

parse_path({abs_path, FullPath}) ->
    case binary:split(FullPath, [<<"?">>]) of
        [URL]       -> {ok, {FullPath, split_path(URL), []}};
        [URL, Args] -> {ok, {FullPath, split_path(URL), split_args(Args)}}
    end;
parse_path({absoluteURI, _Scheme, _Host, _Port, Path}) -> # why  discard scheme, host and port ??
    parse_path({abs_path, Path});
parse_path(_) ->
    {error, unsupported_uri}.

Consider pulling out the About and Usage sections into Wiki pages or just standalone Markdown files.

Hello!

I'd like to create an Elli middleware for reporting exceptions that we can use in all our services. When added to the middleware stack it would catch any errors thrown by later middleware (or receive a report of these errors) and send them to our error aggregation service.

Is it possible to do this with the current Elli middleware system? I've been unable to find a way to do this and have instead wrapped the middleware module to add this functionality.

Thanks,
Louis

Currently there is only the pdict as a way to track the context of a request (requestid, trace and span ids, etc) through middlewares to handlers.

I see two possibilities for expanding this, one which would be a major change to the elli interface and require a major version bump and the other being a simpler extension but feels sort of hacky.

The major change option would be to pass a context that is modifiable, like a map, by the middlewares. I've been starting on a lib for this: https://github.com/tsloughter/ctx

Alternatively the req could have a map or ctx added to it.

The advantage of using a context map as the request instead of req is it can be used and extended by other Erlang apps and code without depending on the Elli req record.

when implementing my handle function on localhost, I was hoping to be able to utilize scheme, host, and port. I keep getting undefined. I admittedly am not super slick with erlang, but the rest of the application works correctly (when getting raw_path, for example, everything is as expected).

All are undefined when this runs inside my server (though method properly returns).

 handle(Req, [Secrets, VendorConfigs, AvroEncoders]) ->
+    io:fwrite("~1p is the method~n",  [Req#req.method]),
+    io:fwrite("~1p is the host~n",    [Req#req.host]),
+    io:fwrite("~1p is the scheme~n",  [Req#req.scheme]),
+    io:fwrite("~1p is the port~n",    [Req#req.port]),

result:

'POST' is the method
undefined is the host
undefined is the scheme
undefined is the port

calling using curl:

curl -H -v http://localhost:3000/v2/my/path -d '{"data":"1113"}'

System specs:

MacOS Catalina, 10.15.4

Erlang installed using https://github.com/asdf-vm/asdf :

$ cat plugins/erlang/kerl-home/.kerlrc
KERL_CONFIGURE_OPTIONS="--without-javac --with-ssl=/usr/local/opt/[email protected] --enable-wx --with-wx-config=/usr/local/bin/wx-config"

$ erl --version
Erlang/OTP 22 [erts-10.4.4] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [hipe]

$ uname -a
Darwin alanvoss 19.4.0 Darwin Kernel Version 19.4.0: Wed Mar  4 22:28:40 PST 2020; root:xnu-6153.101.6~15/RELEASE_X86_64 x86_64