Issue with test_synchronous when interacting with cowboy master port of sockjs-erlang #57

Closed
yrashk opened this Issue Oct 25, 2012 · 6 comments

Projects

None yet

3 participants

@yrashk
yrashk commented Oct 25, 2012

Hi,

I've ported sockjs-erlang to cowboy master (https://github.com/spawngrid/sockjs-erlang/tree/cowboy-master). Everything seems to be fine, but I comparing to the master sockjs (that uses much older cowboy) there is one failure:

$ ./venv/bin/python sockjs-protocol-0.3.3.py
............E.......................................................
======================================================================
ERROR: test_synchronous (__main__.Http10)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "sockjs-protocol-0.3.3.py", line 1449, in test_synchronous
    print c.closed()
  File "/Users/yrashk/tmp/sockjs-protocol/utils_03.py", line 245, in closed
    r = self.s.recv(1) == ''
timeout: timed out

I investigated this issue and I can't think of anything but to blame the test suite :). Here's why:

It fails here https://github.com/sockjs/sockjs-protocol/blob/master/sockjs-protocol-0.3.3.py#L1449, I figured out that what happens here is that connection variable is an empty string (I was printing out values)

However, when I run curl I get this:

* Connected to localhost (127.0.0.1) port 8081 (#0)
> GET /echo HTTP/1.1
> User-Agent: curl/7.21.4 (universal-apple-darwin11.0) libcurl/7.21.4 OpenSSL/0.9.8r zlib/1.2.5
> Host: localhost:8081
> Accept: */*
>
< HTTP/1.1 200 OK
< connection: keep-alive
< server: Cowboy
< date: Thu, 25 Oct 2012 21:10:09 GMT
< content-length: 19
< Content-Type: text/plain; charset=UTF-8
<
Welcome to SockJS!

As you can see, it has connection: keep-alive header. When I modify the test-suite to force it in believing it got connection: keep-alive the test passes without a problem.

Any ideas why this might be happening?

@majek
Member
majek commented Oct 26, 2012

Correct me if I'm wrong, but you're saying that: When tests send a HTTP/1.0 request, with 'Connection: keep-alive' header set, cowboy responds with no 'Connection' header and 'Content-Length' set correctly. Furthermore, when test suite receives that (ie: lack of Connection header) it assumes that the server speaks correct HTTP/1.0 and expects the tcp/ip connection to be closed by the server. But that never happens.

Curl uses HTTP/1.1.

@essen What do you think? Is that possible? I believe that if server doesn't respond with 'Connection: keep-alive', it's correct to assume that it will use HTTP/1.0 semantics - ie: close connection after response.

@yrashk
yrashk commented Oct 26, 2012

Good point about http version, that somehow escaped my mind. Anyhow...
Would that mean cowboys behaviour got changed since?
On Oct 25, 2012 7:48 PM, "Marek Majkowski" notifications@github.com wrote:

Correct me if I'm wrong, but you're saying that: When tests send a
HTTP/1.0 request, with 'Connection: keep-alive' header set, cowboy responds
with no 'Connection' header and 'Content-Length' set correctly.
Furthermore, when test suite receives that (ie: lack of Connection header)
it assumes that the server speaks correct HTTP/1.0 and expects the tcp/ip
connection to be closed by the server. But that never happens.

Curl uses HTTP/1.1.

@essen https://github.com/essen What do you think? Is that possible? I
believe that if server doesn't respond with 'Connection: keep-alive', it's
correct to assume that it will use HTTP/1.0 semantics - ie: close
connection after response.


Reply to this email directly or view it on GitHubhttps://github.com/sockjs/sockjs-protocol/issues/57#issuecomment-9801501.

@majek
Member
majek commented Oct 26, 2012

Not sure, I'm not able to take a closer look now. Let's wait for what @essen says!

@essen
essen commented Oct 26, 2012

HTTP/1.0 doesn't have keep-alive, it will always close the connection.
HTTP/1.1 will use keep-alive if there is no connection header or if the value is unspecified.

It makes no sense to check for a connection header when using HTTP/1.0. Even if you do reply a connection header, if it's HTTP/1.0 the header should be discarded and the connection should be closed.

I'm not sure this is what Cowboy is doing, but at least this is what it should do. :)

@majek
Member
majek commented Oct 30, 2012

Right, looks like a minor bug on cowboy side.

@majek
Member
majek commented Nov 9, 2012

@yrashk Feel free to reopen this issue if you think there is something more to it then the cowboy http 1.0 thing.

@majek majek closed this Nov 9, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment