ninenines / gun Goto Github PK

Options are needed, code also, and guide of course.

See inaka/shotgun#49

In times of protocol negotiation at the TLS level, we need to be aware of what protocol was negotiated before we attempt to do certain things like open a Websocket connection, as SPDY does not support it.

Currently Gun does not detect when the owner dies, leaving the Gun connection alive potentially a little longer. Fix that.

We have one for the HTTP/SPDY part, but not for Websocket.

Should probably be different options.

And this should probably be improved.

For example when a connection failure happen and Gun gives up reconnecting it is fairly hard to debug.

The guide lacks information about defaults when using open. Some defaults must be dependent on the given port, for example 80 should default to tcp/http, 6121 should default to tcp/spdy, and 443 should default to tls/(tls negotiated protocol either through ALPN or NPN with ALPN taking priority, and HTTP/1.1 being used if nothing gets negotiated).

The code must also be updated to use those defaults.

Title.

Per section 14.23 of the HTTP spec, gun should include the port along with the hostname in the host header.

the request function in gun_http.erl should have line 196 and line 210 read something like:

false ->
    HostWithPort =
        case Port of
            undefined -> Host;
            80 -> Host;
            _ -> lists:concat(Host, ":", Port)
        end,
    [{<<"host">>, HostWithPort}|Headers2]`

Then you'd have to pass in the port from the main gun loop and change the other protocol request callback methods to match.

I didn't do this because the workaround is just to specify the host header yourself, there isn't a test suite, and I wasn't sure if someone had other opinions. I'm willing to submit some actual code if that's desired, though.

FYI, this is causing issues when we use gun to test a cowboy server, and cowboy uses the host header (apparently) to determine the port, thus passing an incorrect port number (80 instead of 8080).

We want two options:

• transport: either tcp or tls
• protocol: a list of protocols we may use, the default being different depending on the transport and port used

6121/tcp defaults to ['spdy/3.1']
*/tcp defaults to ['http/1.1']
*/tls defaults to ['spdy/3.1', 'spdy/3', 'http/1.1'] with protocol negotiation (ALPN then NPN and if nothing choose 'http/1.1'

Hello!

I decide to write some common tests and got an error with upgrading socket on web server Cowboy.

test_gun(_Config) ->
    {ok, Pid} = gun:open(?HOST, ?PORT, []),
    Res = gun:ws_upgrade(Pid, "/websocket"),
    ct:print("Res - ~p~n", [Res]),
    receive
        {gun_ws_upgrade, Pid, ok} ->
            gun:ws_send(Pid, {text, "Hello!"});
        {gun_ws_upgrade, Pid, error, IsFin, Status, Headers} ->
            exit({ws_upgrade_failed, Status, Headers})
    after 10000 ->
        exit(timeout)
    end,
    _ = gun:close(Pid),
    ok.

Test fails by timeout and on the server side I get the next error:

17:16:44.177 [error] gen_fsm <0.418.0> in state hello terminated with reason: no function clause matching tls_v1:enum_to_oid(28) line 404
17:16:44.178 [error] CRASH REPORT Process <0.418.0> with 0 neighbours exited with reason: no
function clause matching tls_v1:enum_to_oid(28) line 404 in gen_fsm:terminate/7 line 622
17:16:44.178 [error] Supervisor tls_connection_sup had child undefined started with
{tls_connection,start_link,undefined} at <0.418.0> exit with reason no function clause matching
tls_v1:enum_to_oid(28) line 404 in context child_terminated
17:16:44.179 [error] Ranch listener https had connection process started with
cowboy_protocol:start_link/4 at <0.419.0> exit with reason: {{function_clause,[{tls_v1,enum_to_oid,[28]
[{file,"tls_v1.erl"},{line,404}]},{ssl_handshake,'-dec_hello_extensions/2-blc$^1/1-0-',1
[{file,"ssl_handshake.erl"},{line,1657}]},{ssl_handshake,'-dec_hello_extensions/2-blc$^1/1-0-',1
[{file,"ssl_handshake.erl"},{line,1657}]},{ssl_handshake,dec_hello_extensions,2
[{file,"ssl_handshake.erl"},{line,1657}]},{tls_handshake,decode_handshake,3,[{file,"tls_handshake.erl"}
{line,182}]},{tls_handshake,get_tls_handshake_aux,3,[{file,"tls_handshake.erl"},{line,153}]}
{tls_connection,next_state,4,[{file,"tls_connection.erl"},{line,454}]},{gen_fsm,handle_msg,7
[{file,"gen_fsm.erl"},{line,505}]}]},{gen_fsm,sync_send_all_state_event,[<0.418.0>,{start,5000},infinity]}}

Could you please give me some suggestion what I do wrong? Thanks.

When calling gun:open/2,3 with a binary for the hostname (instead of a string), gun silently fails to connect.

This behavior makes it harder to use Gun in the shell as when the shell crashes it restarts a process but keeps the pid around.

Looks like some code in gun_http:data/4 is using IsFin as a flag but IsFin is fin/nofin:

data(State, StreamRef, IsFin, Data) ->
...
if
    Length2 =:= 0, IsFin ->
        State#http_state{out=head};
    Length2 > 0, not IsFin ->
        State#http_state{out={body, Length2}}
end

Was not working for me so I changed the corresponding tests to 'IsFin =:= fin' and 'IsFin =:= nofin' and it now works.

Probably better to call them 'messages' though as we can easily confuse with Erlang messages I think keeping the slightly incorrect "frames" is better.

Currently when gun gets a 200 response that:

• does not contain content-length
• does not contain transfer-encoding

it cuts the server response off after head. See gun_http:io_from_headers/2.

Expected behavior would be that it returns received body message until server closes connection.

Here's why:
Per http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4

1. Response that may include body, e.g. 200
2. transfer-encoding is not present
3. content-length is not present
4. message doesn't use multipart/byteranges media type
   then
5. client must determine message length by server closing connection

So the proposed fix, since gun doesn't yet track (1) above, maybe it should assume body may be present and change head to body_close when lists:keyfind(<<"transfer-encoding">>, 1, Headers) is false.

Alternatively, I'm way off and @essen will set me straight.

To my comprehension in its current state gun sends all the data messages to the controlling process. Is (or will be) there a something like {active, N} in gen_tcp or any other overload protection mechanism?

Should be a short task.

Currently we receive one message at a time, it should be faster in some cases to use recv as it can wait for the entire body and give that at once, giving a much smaller number of messages to be sent/received.

Please consider adding HTTP proxy and Socks5 proxy (with and without remote DNS resolution). Many applications require at least Socks Proxy support. If choosing is necessary Socks5 would be good enough for most use cases.

Sometimes gun client gets a message {Closed, Socket} for a previously open socket and Socket doesn't match the one in the state. In this case gun crashes with unexpected message.
This seems like a timing issue when both client and server close the connection and server message gets delivered after client has moved on. Could we add

{Closed, _PreviousSocket}
    loop(State);

to the receive section below the {Closed, Socket} in main gun:loop/1 to account for this scenario?

I've compiled the master branch and when I run tests with make tests I get the following error report:

TEST INFO: 1 test(s), 1 case(s) in 1 suite(s)

Testing Desktop.gun.twitter_SUITE: Starting test, 1 test cases

=ERROR REPORT==== 16-Feb-2014::20:06:52 ===
Ignored GOAWAY control frame 0 protocol_error

- - - - - - - - - - - - - - - - - - - - - - - - - -
twitter_SUITE:spdy failed on line 58
Reason: timeout
- - - - - - - - - - - - - - - - - - - - - - - - - -

Testing Desktop.gun.twitter_SUITE: *** FAILED test case 1 of 1 ***
Testing Desktop.gun.twitter_SUITE: TEST COMPLETE, 0 ok, 1 failed of 1 test cases

The user of the Gun client should never have to set this header manually. We want to use only content-length and content-type to detect whether a header is present. A transfer-encoding header in a request should always be removed, even for HTTP/1.1.

There's four kinds of errors:

• stream error
• general connection error
• DOWN error
• timeout error

We should probably have a specific return value type for each of them.

The upgrade and connection headers need to be verified too.

The Gun user guide implies that the library supports the HTTP protocol but as far as I can tell (though my Erlang experience is limited) there's only support for SPDY so far?

Currently only a brutal close is possible. Shutdown should be the graceful equivalent.

When using SPDY, there is no way to connect to a certain IP and specify a different :host header.

Title.

Should probably make it clearer.

We need to detail in the guide how to recover from connection failures when the Websocket protocol is used. Do we need to upgrade again? Does Gun upgrade automatically? Should that be an option?

The method is not needed as it is only used for GET.

The host and path are lacking a scheme and should probably be sent as a complete URI, since push is meant for caching purposes (the client will query immediately after).

Documentation is needed to explain what headers are sent in a push.

When trying to connect to a resource without ssl, using tcp option

{gun_error,<0.344.0>,
         {{error,undef,
                 [{gun_http,init,[<0.279.0>,#Port<0.6545>,ranch_tcp],[]},
                  {gun,connect,2,[{file,"src/gun.erl"},{line,285}]},
                  {gun,init,5,[{file,"src/gun.erl"},{line,250}]},
                  {proc_lib,init_p_do_apply,3,
                            [{file,"proc_lib.erl"},{line,239}]}]},
          "An unexpected error occurred."}}

The culprit: https://github.com/extend/gun/blob/master/src/gun.erl#L283 and gun_http doesn't exist :)

It could be interesting to support cookies. Thinking it should be per connection though, with cookies being forwarded to the owner process so that it can keep them elsewhere if needed.

When server sends HTTP response while gun client is sending keepalive requests, gun client stops with function_clause.

The reason for this is that during this time there are no open streams. When gun client gets a response, its loop matches {OK, Socket, Data} and in turn calls gun_http:handle. Given proper HTTP response from server this produces binary:match on <<"\r\n\r\n">> and calls handle_head, which expects streams to have a tuple in list in its state. Not matching this condition, it gets function_clause.

Option 1: add this to gun_http

handle_head(Data, State=#http_state{streams=[]}) ->
    handle_head(Data, State#http_state{streams=[{undefined, false}]});

Option 2: only process server response when there was a request - i.e. ignore server responses during keepalives
Option 3: ?

Title.

It'd be nice to be able to pass SSL parameters (https://github.com/ninenines/ranch/blob/master/src/ranch_ssl.erl#L38-L65) to the underlying SSL socket.

When using HTTP/1.1, we can't cancel streams. If the server is sending us a very large body, we should probably close the socket and reconnect so we can perform new requests without having to wait for everything to be downloaded.

The await function can receive data (one message at a time), while the await_body function will accumulate all data until the entire body is received.

mkdir test
touch test/index.html
echo '<html></html>' >> test/index.html
cd test
python -m SimpleHTTPServer 8282

Client code

    {ok, Pid} = gun:open("localhost", 8282),
    Path = "/",
    ct:print("requesting path ~p", [Path]),
    Stream = gun:get(Pid, Path),
    Res = gun:await(Pid, Stream),

Output

Client {error, timeout}

Server console

127.0.0.1 - - [10/Apr/2014 16:41:38] code 400, message Bad request syntax ("\x16\x03\x03\x00\xf7\x01\x00\x00\xf3\x03\x03SG\x02\x02*\x0e\xfd\x1f\xaf(\x8bs\x84>\xb6x\xc1\xe0h:/\x93I\x06\x15c\x97\xe4\xc3\xbf&\xd3\x00\x00X\x00\xff\xc0$\xc0(\xc0&\xc0*\x00k\x00j\x00=\xc0#\xc0'\xc0%\xc0)\x00g\x00@\x00<\xc0")
127.0.0.1 - - [10/Apr/2014 16:41:38] "˜SG*˝Ø(ãsÑ>∂x¡:/ìIcóø&Xˇ¿$¿(¿&¿*kj=¿#¿'¿%¿)g@<¿" 400 -
127.0.0.1 - - [10/Apr/2014 16:41:43] code 400, message Bad HTTP/0.9 request type ('\x16\x03\x03\x00\xf7\x01\x00\x00\xf3\x03\x03SG\x02\x07\xd7^=\x0eT\

When using the gun:data/4 functions, the user may send empty chunks. These must be ignored except when the IsFin value is set to 'fin'. For HTTP/1.1 that means sending the 0 sized chunk, for SPDY that means sending an empty chunk with the fin flag set.

Title.

Most of everything has to be implemented.

When gun client starts with init it calls connect which calls before_loop which schedules a keepalive with erlang:send_after. Subsequently, before_loop is invoked when main loop gets a keepalive message.

Now, when server sends connection close or closes socket, gun client tries to reconnect by calling retry_loop from main loop function. retry_loop calls connect and that in turn calls before_loop which schedules another keepalive with erlang:send_after. However, at this time there already was a keepalive scheduled prior to socket close, so now we have two sets of keepalive schedules. Another socket close will add another keepalive schedule on top and so on.

One solution seems to be to not schedule a keepalive during reconnects.

Calling gun:ws_upgrade/2 results in an exception:

undefined function gun_http:ws_upgrade/3
      in function  gun:loop/1 (src/gun.erl, line 492)

Looks like websocket support is not implemented yet :( Any ETA on that?

When cowlib not started any warnings or errors not viewed. After cowboy:await/2 just {error, undef} or {error, timeout}.
It's realy difficult to find why your simple app doesn't work correctly, please add error message!

Is there a post example?

And how can i get the data?

wget is not necessarily installed by default on all systems (Mac OS X being one and Ubuntu being the other, if I'm not mistaken about Ubuntu).

I think curl is available on more systems out of the box.

This is not a terrible problem per se, but could be an annoyance.

When I am connected to an SSE handler and I receive events, for some reason cowboy sends me in the first two cases a no fin message with the complete event, but in the third case I get a no fin message with many events. If you check the third message you will see that it has many \n\ndata. I do know if this is ok or not. I thought that gun would split each sse event into a different message. If this is not the case I should buffer all the events and then split it by myself.

It would be great if you could let me know if this is a bug or if I need to detect each sse event by myself.

Regards.

IsFin: nofin
StreamRef: #Ref<0.0.0.260017>
Message: <<"data: {\"tigertext_api\":{\"version\":\"1.0\",\"timestamp\":\"2014-7-24T15:27:56Z\",\"app\":\"http_sse\",\"app_version\":\"1.0\",\"event_id\":\"466f6e1d-a614-429d-a3e3-c9ab5455a81c\",\"event\":{\"tigertext:iq:client_advisory\":{\"xmlns\":\"tigertext:iq:client_advisory\",\"advisory\":{\"name\":\"replay_start\",\"replay_message_count\":19}}}}}\n\n">>

IsFin: nofin
StreamRef: #Ref<0.0.0.185341>
Event: <<"data: {\"tigertext_api\":{\"version\":\"1.0\",\"timestamp\":\"2014-7-24T15:19:43Z\",\"app\":\"http_sse\",\"app_version\":\"1.0\",\"event_id\":\"aca5df83-4697-46c5-822a-43f9be32872e\",\"event\":{\"tigertext:iq:update\":{\"xmlns\":\"tigertext:iq:update\",\"entities\":[{\"xmlns\":\"tigertext:entity:group\",\"token\":\"6eGmPWCpTHOTnfH4lx2omA\",\"name\":[],\"members\":[\"test\",\"test\"]}]}}}}\n\n">>

IsFin: nofin
StreamRef: #Ref<0.0.0.185341>
Event: <<"data: {\"tigertext_api\":{\"version\":\"1.0\",\"timestamp\":\"2014-7-24T15:19:43Z\",\"app\":\"http_sse\",\"app_version\":\"1.0\",\"event_id\":\"2aa5f747-c27b-472b-a055-6b4e785243f2\",\"event\":{\"tigertext:iq:update\":{\"xmlns\":\"tigertext:iq:update\",\"entities\":[{\"xmlns\":\"tigertext:entity:group\",\"token\":\"X2hSUFfESp6xje54_TnUYA\",\"name\":[],\"members\":[\"test\",\"test\"]}]}}}}\n\ndata: {\"tigertext_api\":{\"version\":\"1.0\",\"timestamp\":\"2014-7-24T15:19:43Z\",\"app\":\"http_sse\",\"app_version\":\"1.0\",\"event_id\":\"e3d511e8-3c2a-40ef-b91c-24ef737d2349\",\"event\":{\"tigertext:iq:update\":{\"xmlns\":\"tigertext:iq:update\",\"entities\":[{\"xmlns\":\"tigertext:entity:group\",\"token\":\"0aFq0GzAQ6a3YII0HLRR3w\",\"name\":[],\"members\":[\"test\",\"test\"]}]}}}}\n\ndata: {\"tigertext_api\":{\"version\":\"1.0\",\"timestamp\":\"2014-7-24T15:19:43Z\",\"app\":\"http_sse\",\"app_version\":\"1.0\",\"event_id\":\"a96622df-4c4a-47c1-93f7-a9f53c120449\",\"event\":{\"tigertext:iq:update\":{\"xmlns\":\"tigertext:iq:update\",\"entities\":[{\"xmlns\":\"tigertext:entity:group\",\"token\":\"sIKFXV_fSzW5rtRkro22wQ\",\"name\":[],\"members\":[\"test\",\"test\"]}]}}}}\n\ndata: {\"tigertext_api\":{\"version\":\"1.0\",\"timestamp\":\"2014-7-24T15:19:43Z\",\"app\":\"http_sse\",\"app_version\":\"1.0\",\"event_id\":\"a0bd94c2-b851-40ed-a1b1-cf4014eaa7eb\",\"event\":{\"tigertext:iq:update\":{\"xmlns\":\"tigertext:iq:update\",\"entities\":[{\"xmlns\":\"tigertext:entity:group\",\"token\":\"8iQz-VlGR7KC22mCv87BAg\",\"name\":[],\"members\":[\"test\",\"test\"]}]}}}}\n\ndata: {\"tigertext_api\":{\"version\":\"1.0\",\"timest">>

Options for open/3 and options for protocols should use a map. Websocket already does.