Allow setting of handshake attributes directly

[ngauthier]

In order to use the handshake server from a non-string based protocol (e.g. Rack), it is convenient to be able to set the headers, path, and query manually, and then call set_version manually.

This means that this library can be easily used with Rack 1.5's new hijack feature, which gives a user access to the underlying socket.

[imanel]

I'm not sure if this is good approach - this will skip all validations and allow to break something too easily. I was thinking about making method from_hash that would allow passing path, headers and body and method from_rack on top of it. If you want to propose change then my idea about acceptable params would be:

for from_hash:

{
  "path": string,
  "headers": hash, all headers lower case,
  "body": string or IO stream(responding to read)
}

For from_rack that would be Rack env. If you would implement even part of it then my work would be much easier - otherwise I will try do add such function during this weekend.

[ngauthier]

I did have a from_rack, but I thought you might object to a rack specific method.

The trick is with rack when you're in rails, rails already parses the headers and stuff so there isn't a body object to read from. But I guess a body of "" is fine as long as all the headers are there.

Are you going to separate the path from the query as well?

[ngauthier]

Oh, and for some context, I'm using the library here:

https://github.com/ngauthier/tubesock/blob/master/lib/tubesock.rb#L14

[imanel]

I'm think that calling rewind on rails request body will give you valid result(this would be obvious step for any IO stream for from_hash) And I was thinking about implementing something like code you mentioned, but directly as from_rack wrapper around from_hash.

[ngauthier]

oh duh, I didn't think of that! I will try a few things tomorrow. If I can get it to work with your current version that would be fine with me, and we can document the approach.

Thanks!

[ngauthier]

I updated the PR with just a from_rack method. It turns out you can't rewind a TCP socket so I couldn't get the original request back out of rack.

In rack, the http headers are all caps and are preceded by HTTP_. We could add a from_hash that expected lowercased headers w/o HTTP_, then have from_rack delegate to that after processing.

Let me know what you think.

[imanel]

That's nearly complete. Things I will need in order to accept this pull request:

• Set state to finished
• Support for draft-76 keys 'sec-websocket-key1' and 'sec-websocket-key2'. You might also consider storing all headers like in current code instead of only selected ones.
• Support for 'body' in request(also used in draft-76)
• Tests(easiest way would be to modify common file and add from_rack versions of hashes. This would also require adding hash version of requests, but that's easy task)

[ngauthier]

Thanks for the rundown, I will work on it!

On Thu, Mar 21, 2013 at 11:55 AM, Bernard Potocki notifications@github.comwrote:

That's nearly complete. Things I will need in order to accept this pull request:

• Set state to finishedhttps://github.com/imanel/websocket-ruby/blob/master/lib/websocket/handshake/base.rb#L104
• Support for draft-76 keyshttps://github.com/imanel/websocket-ruby/blob/master/lib/websocket/handshake/handler/server76.rb#L48'sec-websocket-key1' and 'sec-websocket-key2'. You might also consider storing all headers like in current codehttps://github.com/imanel/websocket-ruby/blob/master/lib/websocket/handshake/base.rb#L99instead of only selected ones.
• Support for 'body' in request(also used in draft-76https://github.com/imanel/websocket-ruby/blob/master/lib/websocket/handshake/handler/server76.rb#L50 )
• Tests(easiest way would be to modify commonhttps://github.com/imanel/websocket-ruby/blob/master/spec/support/all_server_drafts.rbfile and add from_rack versions of hashes. This would also require adding hash version of requests, but that's easy task)

— Reply to this email directly or view it on GitHubhttps://github.com/imanel/websocket-ruby/pull/15#issuecomment-15247058 .

[imanel]

My small suggestion would be:

• Change lines 76-80 something like this(warning: ugly code - you can improve it as I'm currently on the road):

@headers = Hash[env.select {|key| key =~ /\AHTTP\_/}.map{|key, value| [key.gsub(/\AHTTP\_/, '').gsub('_', '-').downcase, value]}]

• Add body:

@lefovers = env['rack.input'].read # this might require 'rewind' before

• Set finished

@state = :finished

This code should fix first three points of list. If you want to add from_hash in the same way you can do it like that:

def from_hash(hash)
  @headers = hash['headers']
  @path = hash['path']
  @query = hash['query']
  @leftovers = hash['body']
  @state = :finished
  set_version
end

If you could then lease add 2-3 lines of documentation :)

[ngauthier]

Hello again!

I've added a from_rack test. What I did was to use WEBrick to parse your client_request into a Rack env, then feed that to my from_rack. I was able to pass all the version tests after parsing all HTTP headers just like you recommended.

Take a look, I think it's ready :-)

[imanel]

It looks like tests are not passing because of line 88 of test - you used Ruby 1.9 hash syntax. Please change it to 1.8 version and I will merge it.

[ngauthier]

Green https://travis-ci.org/imanel/websocket-ruby/builds/6081565

[imanel]

Merged - thanks for hard work! I will release it together with v1.1.0 in next week.

[ngauthier]

Awesome! And thanks for all the help.

[ngauthier]

How is version 1.1.0 coming?

[imanel]

Yesterday I pushed part one of big changes(out of two) - if I will be able to spend some time today then it should be released tomorrow(hope so). Sorry for the delay - I'm a little overworked here ;)

[ngauthier]

Hey no problem, just checking in because I was working on Tubesock today.

On Wed, Apr 24, 2013 at 3:43 AM, Bernard Potocki notifications@github.comwrote:

Yesterday I pushed part onehttps://github.com/imanel/websocket-ruby/commit/9194711ec2d287b8154d123acc7bc2f5eb786694of big changes(out of two) - if I will be able to spend some time today then it should be released tomorrow(hope so). Sorry for the delay - I'm a little overworked here ;)

— Reply to this email directly or view it on GitHubhttps://github.com/imanel/websocket-ruby/pull/15#issuecomment-16912281 .

[ngauthier]

Hate to be a bug, but is there a version release coming soon? Is there anything I can help with? If you make some issues and point me to them I'll give them a shot.

Thanks again for a great gem!

[imanel]

Sorry for keeping you waiting - I needed to make sure that new update will not break anything before releasing. It's now tagged as v1.1.0 - enjoy!

[ngauthier]

Thanks! I'm glad you take the time to verify releases :-)

On Mon, Jun 17, 2013 at 7:58 AM, Bernard Potocki notifications@github.comwrote:

Sorry for keeping you waiting - I needed to make sure that new update will not break anything before releasing. It's not tagged as v1.1.0 - enjoy!

— Reply to this email directly or view it on GitHubhttps://github.com/imanel/websocket-ruby/pull/15#issuecomment-19540454 .