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

Add. Decode reserved bits in header. #55

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

moteus
Copy link
Contributor

@moteus moteus commented Feb 27, 2015

No description provided.

@lipp
Copy link
Owner

lipp commented Feb 27, 2015

why should we read out the reserved if they carry no info at all?

@moteus
Copy link
Contributor Author

moteus commented Feb 27, 2015

SV1, RSV2, RSV3: 1 bit each

  MUST be 0 unless an extension is negotiated that defines meanings
  for non-zero values.  If a nonzero value is received and none of
  the negotiated extensions defines the meaning of such a nonzero
  value, the receiving endpoint MUST _Fail the WebSocket
  Connection_.
  1. Implementation must check this to complain rfc
  2. It ma be set(I do not kno how yet)
  3. It needed to pass autobahn test(test case 3)

@lipp
Copy link
Owner

lipp commented Feb 27, 2015

ok. but this implementation does not support extensions yet. further, we are not processing the rsv values... how can the autobahn test suite check this?

@moteus
Copy link
Contributor Author

moteus commented Feb 27, 2015

Autobahn server send messages with setted rsv and client must close connection with 1002 status

@lipp
Copy link
Owner

lipp commented Feb 27, 2015

Ok. I don't see that the connection is closed when rsv != 0. Can we add this test to the Travis file?

@moteus
Copy link
Contributor Author

moteus commented Feb 27, 2015

May be we can use autobahn server. The only problem is I do not know how detect test fail.
To run testcase for RSV you can use wstest -m fuzzingserver -s rsv-config.json with config like

{
   "url": "ws://127.0.0.1:9001",
   "outdir": "./reports/clients",
   "cases": ["3.*"],
   "exclude-cases": [],
   "exclude-agent-cases": {}
}

My test runner is https://github.com/moteus/lua-lluv-websocket/blob/master/test/autobahn_client_test.lua

@lipp
Copy link
Owner

lipp commented Feb 27, 2015

interesting. i'd like to integrate that somehow.

anyways, shouldn't there be a check for rsv bytes and eventually a connection close?

@moteus
Copy link
Contributor Author

moteus commented Feb 27, 2015

shouldn't there be a check for rsv bytes and eventually a connection close?

websocket implementation should check RSV and if it not 0 then it should start close handshake with status 1002.
I do not know how extensions works but it is valid for client without extenstions.
So my lluv websocket in this case send protocol error and return WEBSOCKET.EOF error to client code. So this logic need to be inplemented in client/server code.
Current implementation do not do this and it could work in real world but to provide ability to fix this websockets-core part should returns this bits.

@moteus
Copy link
Contributor Author

moteus commented Feb 28, 2015

I just make commit to run autobahn tests on Travis for lluv-websocket. Without this PR tests 3.* are failed.

@lipp
Copy link
Owner

lipp commented Mar 5, 2015

👍

but still: we should integrate that test using the "official" api and auto-close the connection as you do in your lluv autobahn test client.

@moteus
Copy link
Contributor Author

moteus commented Mar 6, 2015

Please take a look to autobahn_wclient_test. It uses pablic client API.
You just need remove SSL parts and replace uv.run() call.
To run test you need first run run wstest
wstest -m fuzzingserver -s rsv-config.json with config like

{
   "url": "ws://127.0.0.1:9001",
   "outdir": "./reports/clients",
   "cases": ["3.*"],
   "exclude-cases": [],
   "exclude-agent-cases": {}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants