sockjs / sockjs-erlang Goto Github PK

Investigate this commit:

Flotype@e51adf5

Guys, are you serious?

Hi Guys,

sockjs is a great piece of work. I am trying to integrate it with one of my application.
sockjs-erlang application runs on a different host/port than the usual web host/port.

As a result, i would like to have some kind of authorization/authentication flow before
a user can open a websocket stream. I think this is best solved by doing checks on cookie
data and validating them in the cache store.

To get cookie info, i have currently this patch working for me:

git diff src/sockjs_handler.erl 
diff --git a/src/sockjs_handler.erl b/src/sockjs_handler.erl
index f0fce8d..e21edfe 100644
--- a/src/sockjs_handler.erl
+++ b/src/sockjs_handler.erl
@@ -222,7 +222,14 @@ extract_info(Req) ->
                                   end, {[], Req2},
                                   ['Referer', 'X-Client-Ip', 'X-Forwarded-F
                                    'X-Cluster-Client-Ip', 'Via', 'X-Real-Ip
+       
+       %% hack to get cookies in the callback too
+       {cowboy, CReq0} = Req3,
+       {Cookies, CReq1} = cowboy_http_req:cookies(CReq0),
+       Req4 = {cowboy, CReq1},
+       
     {[{peername, Peer},
       {sockname, Sock},
       {path, Path},
-      {headers, Headers}], Req3}.
+      {headers, Headers},
+      {cookies, Cookies}], Req4}.

Other thing i found missing inside sockjs-erlang api is how can i shutdown/deny a connection attempt
from within my Conn init callback when i detect a invalid cookie data (ideally this should be
happening at /echo/info call level). One possible solution is to straightaway call Conn:close() ,
is that the best solution possible here?

See https://github.com/extend/cowboy/blob/master/CHANGELOG.md

Taken from the comments on commit 95b8c7a, making into an issue to make things a bit clearer.

essen: Hello! Cowboy author here, got linked to this commit. Did you find out what was the issue and is there anything I can do to help?

simonmacmullen: Hi essen, thanks for commenting! No, I haven't had any further time to investigate this and try to strip down to a more minimal test case or (more likely) figure out what the hell I'm doing wrong.

If you feel really energetic you could try to run it yourself; in the qunit tests there are lots of failures due to Cowboy returning 400 before my code gets a look in. If not I'll try to strip things down and make this more replicable next week.

essen: Sounds like it could be Cowboy indeed. I'll look later today. Can you tell me what browser you used to run the qunit tests though?

simonmacmullen: Chromium 11.0.696.68 (84545) Ubuntu 10.04, but I just tested with FF6 and got the same.

If you do try:

• I apologise for the rough nature of the build system and tests
• There will be bugs in my code that cause some tests to fail. "xhr-streaming: echo2" is an example of a test that fails in the way we're talking about here.
• I stuck some io:formats in cowboy_http_protocol and it seems like the buffer in the state record contains the tail of a request somehow that gets carried over into the next request in next_request/3. But I have no idea how that module is supposed to work.

Several of the simple issues/pr's have been open for months and the cowboy version is a full version behind the latest version.

Is there anything I can do to help?

Probably due to \0 issue with eep0018, introduced by @squaremo in 2423565. That's not good. Any ideas? Should we ship mochjson or fix eep?

On ubuntu precise, with R15B01, attempting to run the examples gives:

./examples/cowboy_echo.erl
escript: exception error: no match of right hand side value {error,{not_started,xmerl}}
  in function  cowboy_echo:main/1 (./examples/cowboy_echo.erl, line 14)
  in call from escript:run/2 (escript.erl, line 727)
  in call from escript:start/1 (escript.erl, line 277)
  in call from init:start_it/1 
  in call from init:start_em/1 

I think cowboy requires xmerl to be started first.

(Yes, I did do the get-deps and compile steps)

sockjs-erlang git:master ❯ ./examples/cowboy_echo.erl                                                                                                                                                     ⏎
escript: exception error: no match of right hand side value
                 {error,{"no such file or directory","sockjs.app"}}
  in function  cowboy_echo:main/1 (./examples/cowboy_echo.erl, line 14)
  in call from escript:run/2 (escript.erl, line 747)
  in call from escript:start/1 (escript.erl, line 277)
  in call from init:start_it/1
  in call from init:start_em/1
sockjs-erlang git:master ❯ ./examples/cowboy_test_server.erl                                                                                                                                              ⏎
escript: exception error: no match of right hand side value
                 {error,{"no such file or directory","sockjs.app"}}
  in function  cowboy_test_server:main/1 (./examples/cowboy_test_server.erl, line 14)
  in call from escript:run/2 (escript.erl, line 747)
  in call from escript:start/1 (escript.erl, line 277)
  in call from init:start_it/1
  in call from init:start_em/1

What's sockjs.app and where am I supposed to get it from?

Sockjs-node allows user to specify option {disabled_transports: ['websocket']}. This functionality is described in sockjs/sockjs-protocol: https://github.com/sockjs/sockjs-protocol/blob/master/sockjs-protocol-0.1.py#L486

We should support it in sockjs-erlang

There is a bad match exceptionin dev branche in sockname/1 function.
Please fix that. May be the result of second case must be {ok,{{0,0,0,0},0}?

Please add support for cowboy 0.8.x which adds many useful features, additional security measurements and speedups.

See https://github.com/extend/cowboy/blob/master/CHANGELOG.md

Guys, we are living in 21 century, why don't you use jiffy instead of mochijson?

As suggested in #2.

Need help.
Is there way to get cookies from request.
I'm using cowboy and want to authorize user before connect him.
Is there any way to solve my problem ?

Max suggests that we could use hibernate to save memory:
http://groups.google.com/group/sockjs/msg/152fc7a4c0718657

On this release sockjs is not compiled. Parametrized modules are no longer supported.

Stacktrace:
"deps/sockjs/src/sockjs_multiplex_channel.erl:1: parameterized modules are no longer supported"

Running out of atoms may kill the VM.

yuchao@bogon: ~/Github/sockjs-erlang master!
$ ./rebar compile [10:29:36]
==> ranch (compile)
==> cowboy (compile)
==> sockjs-erlang (compile)
src/sockjs_session.erl:17: type queue() undefined
FAIL

Here is a list of things you can do in the code base I saw after skimming through it.

• The session gen_server process stores SessionIDs in an ETS table. This mapping of Id to Pid is quite common in Erlang, so there is a project, esl/gproc@master , which does exactly that.
• You can easily avoid the need to trap exits in the session code. gproc will set a monitor on your process and clean up if your process terminates.
• Sending back go to a process when it enqueues is a bit dangerous. Other processes might send go back. Consider some unique reference with the call make_ref/0 and send that along, so it is safe. Perhaps you can write all of it via a gen_server:call/3 and then use gen_server:reply/2 if the reply has to be sent asynchronously.
• There are a number of calls to iolist_to_binary/1 in the code. If possible, consider sending iolist() constructions around instead. Many output functions takes iolist()s. Converting them to binaries means that you traverse the iolist() and copy its data to a new binary you form on the fly. This copying may be unnecessary, takes time, and it increases the pressure on the binary-allocator in the Erlang VM.
• Considered using another web server in the backend? Misultin is rather cool, but there are other projects, like klacke/yaws@master or extend/cowboy@master .. Since you already did a large part of the nasty work, and since the code has neat separation, this should definitly be considered. You may also be interested in talking to @omarkj who has shown some interest in SockJS for Cowboy.
• Considered using rebar to build the source code? To be honest, I've already converted part of the repository to rebar. With rebar, you can easily embed the project in other projects. I imagine this is the main intention of the code.

Finally:

• How do you stress test the code?
• I've tried to read the SockJS spec, but it looks like there are some prerequisites I don't understand, because there are a lot of parts in the spec I can't wrap my head around. There is a framing system, where we prefix messages for instance, but there is no description of why we want exactly these frames, what a frame is, what the grammar for frames is and so on.

SockJS-erlang fails on slow websocket consumer.

For example:

=ERROR REPORT==== 26-Nov-2012::14:20:55 ===
** Handler sockjs_cowboy_handler terminating in websocket_handle/3
   for the reason exit:{noproc,
                           {gen_server,call,
                               [<0.158.0>,{received,[<<"18">>]},infinity]}}
** Message was {text,<<"[\"18\"]">>}
** Options were {service,"/amplify",#Fun<cowboy_test_server.2.61145267>,state,
                         "http://cdn.sockjs.org/sockjs-0.2.js",false,true,
                         5000,25000,131072,5000,
                         #Fun<sockjs_handler.0.34008514>}
** Handler state was {websocket,<0.158.0>,{#Ref<0.0.0.8840>,5000}}
** Request was {http_req,#Port<0.2609>,cowboy_tcp_transport,keepalive,
                         <0.157.0>,'GET',
                         {1,1},
                         {{127,0,0,1},59316},
                         [<<"localhost">>],
                         undefined,<<"localhost">>,8081,
                         [<<"amplify">>,<<"0">>,<<"0">>,<<"websocket">>],
                         [<<"0">>,<<"0">>,<<"websocket">>],
                         <<"/amplify/0/0/websocket">>,undefined,<<>>,[],
                         [{<<"Sec-Websocket-Version">>,<<"13">>},
                          {<<"Sec-Websocket-Key">>,
                           <<"FVKay/apSU2ZytGQBD2aBw==">>},
                          {<<"Origin">>,<<"localhost:8081">>},
                          {'Host',<<"localhost">>},
                          {'Connection',<<"Upgrade">>},
                          {'Upgrade',<<"websocket">>}],
                         [{'Upgrade',[<<"websocket">>]},
                          {'Connection',[<<"upgrade">>]}],
                         undefined,
                         [{websocket_version,13}],
                         waiting,<<>>,done,[],<<>>,
                         {#Fun<cowboy_http.urldecode.2>,crash}}
** Stacktrace: [{gen_server,call,3},
                {sockjs_session,received,2},
                {sockjs_ws_handler,session_received,2},
                {sockjs_cowboy_handler,websocket_handle,3},
                {cowboy_http_websocket,handler_call,7},
                {cowboy_http_protocol,upgrade_protocol,3}]

To reproduce pip install websocket-client and:

from websocket import create_connection
ws = create_connection("ws://localhost:8081/amplify/0/0/websocket")

result =  ws.recv()
print "Received '%s'" % result


for i in range(10000):
    ws.send('["18"]')


#    ws.send('["18"]')
result =  ws.recv()

1. Compile and start test server using make test
2. Make request using http://localhost:8080/broadcast/077/p2fpxgt4d/xhr or run smoketest for xhr-polling
3. The following error message in erl console:

=ERROR REPORT==== 1-Oct-2011::01:40:21 ===
    module: misultin_http
    line: 597
file not found

As discussed here:

https://groups.google.com/d/msg/sockjs/vsFvHqppq5g/XQPgErs9BpUJ

We should introduce three new calls:

sockjs:send_multi([Conn], iolist)
sockjs:prepare(iolist)
Conn:send_prepared(prepared)

Of course send_multi might use prepared underneath.

sockjs-erlang with misultin produces such error during http://localhost:8080/example-cursors.html websockets test.

other transports are ok.

=ERROR REPORT==== 30-Dec-2011::12:39:41 ===
Error in process <0.84.0> with exit value: {badarg,[{jiffy,nif_decode,["\"{\\"x\\":188,\\"y\\":22,\\"t\\":1325266781650,\\"id\\":\\"0649712570358062\\"}\""],[]},{jiffy,decode,1,[{file,"src/jiffy.erl"},{line,11}]},{sockjs_util,jiffy,2,[{file,"src/sockjs_util.erl"},{line,39}]},{sockjs_http... 


=ERROR REPORT==== 30-Dec-2011::12:39:41 ===
        module: misultin_websocket
        line: 238
linked websocket controlling loop crashed with reason: {badarg,
                                                        [{jiffy,nif_decode,
                                                          ["\"{\\\"x\\\":188,\\\"y\\\":22,\\\"t\\\":1325266781650,\\\"id\\\":\\\"0649712570358062\\\"}\""],
                                                          []},
                                                         {jiffy,decode,1,
                                                          [{file,
                                                            "src/jiffy.erl"},
                                                           {line,11}]},
                                                         {sockjs_util,jiffy,
                                                          2,
                                                          [{file,
                                                            "src/sockjs_util.erl"},
                                                           {line,39}]},
                                                         {sockjs_http,
                                                          misultin_ws_loop0,3,
                                                          [{file,
                                                            "src/sockjs_http.erl"},
                                                           {line,129}]}]}

I was doing some internal tests and here's what I found.

1. There's sockjs_session_sup supervisor which is configured to have up to 10 failures in 10 seconds.
2. sockjs_session calls callback functions in context of its own process
3. If there's error in the one of the callback methods, sockjs_session process will die and supervisor will restart it.
4. If socks_session dies too often (basically 10 times in 10 seconds), the supervisor will be killed with all its children. Effectively, if you can find a way to send a message which triggers the exception, you can kill SockJS server.

I think client processes should be isolated. Even if one client misbehaves, it should not kill whole process tree.

Not sure how to fix it properly, but few ideas:

1. Increasing limits won't really help - it is temporary measure
2. Maybe switch to temporary restart strategy? The session will be closed anyway - terminate() is getting called and the session is removed from ETS. Plus, the state is lost, so it can't recover. Or am I missing something?

At least not every aspect.

This should be tested by the -protocol, but it's not yet.

The issue:

$ curl http://localhost:8080/echo/chunking_test -v -X OPTIONS
501 Not Implemented

Confirmed with tracing when using xhr-streaming protocol. I will probably fix it in my branch and prepare a pull request later on (when I'm not as short on time as I am now)

As discussed in #1