Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Web Socket HyBi 10 support #26

Merged
merged 13 commits into from

4 participants

@veebs
Collaborator

Submitted for your consideration and review.

  1. I put the new code in org.jboss.netty.handler.codec.http.websocketx so that it will not break existing Netty 3 users of web sockets. There are quite a few frameworks using the old web socket package. Maybe you can depreciate the old org.jboss.netty.handler.codec.http.websocket for Netty 4?

  2. Examples are contained in org.jboss.netty.example.http.websocketx.server and org.jboss.netty.example.http.websocketx.client.

There are quite a few files because I have tried to put together a framework where new versions of the specification can be supported. Version 13 of the specification is out already.

I have also added examples on how to use netty to implement web socket server and clients. I hope you find this useful.

Keen to hear your feedback.

@aslakhellesoy

I'm the author of the Webbit code this was based on. You forked it at a point in time when it was quite buggy. It's a lot better on Webbit master, but still has some issues. I'm working to make it pass the Autobahn test suite and there is still a little left to do.

Aslak

@veebs
Collaborator

Hi Aslak,

Thank you for contacting me.

I've been meaning to contact you but I must admit work and family has gotten the better of me at the moment and it totally slipped my mind.

I've been meaning to let you know that I while testing, I found a funny scenario that required changes in the code that I took from your HybiWebSocketFrameDecoder class (and in this netty pull request class WebSocket08FrameDecoder).

I'm not sure if it is to do with my Netty setup or not but I found that sometimes, the payload is not delivered in 1 nice packet in the channel buffer. Hence, I had to put in extra code to:

  • only read to the expected frame length just in case the bytes in the channel buffer is longer than the frame length, or
  • if the bytes in the channel buffer is less than the expected frame length, wait for more bytes to arrive.

From memory, I got this when testing with Chrome.

           case PAYLOAD:
                // Some times, the payload may not be delivered in 1 nice packet
                // We need to accumulate the data until we have it all
                int rbytes = actualReadableBytes();
                ChannelBuffer payload = null;

                int willHaveReadByteCount = currentPayloadBytesRead + rbytes;
                if (willHaveReadByteCount == currentFrameLength) {
                    // We have all our content so proceed to process
                    payload = buffer.readBytes(rbytes);               
                } else if (willHaveReadByteCount < currentFrameLength) {
                    // We don't have all our content so accumulate payload. Returning null means we will get called back
                    payload = buffer.readBytes(rbytes);
                    if (currentPayload == null) {
                        currentPayload = channel.getConfig().getBufferFactory().getBuffer(currentFrameLength);
                    }
                    currentPayload.writeBytes(payload);
                    currentPayloadBytesRead = currentPayloadBytesRead + rbytes;

                    // Return null to wait for more bytes to arrive
                    return null;
                } else if (willHaveReadByteCount > currentFrameLength) {
                    // We have more than what we need so read up to the end of frame
                    // Leave the remainder in the buffer for next frame
                    payload = buffer.readBytes(currentFrameLength - currentPayloadBytesRead);
                }

                // Now we have all the data, the next checkpoint must be the next frame
                checkpoint(State.FRAME_START);

                // Take the data that we have in this packet
                if (currentPayload == null) {
                    currentPayload = payload;
                } else {
                    currentPayload.writeBytes(payload);
                }

                // Unmask ...

Thanks for letting me know about the Autobahn. I'll try to give it a go over the next week as well.

Really appreciate all your efforts on WebSockets. Without you, the Java WebSocket effort will be really behind everyone else!

Thanks again,
Veebs

@veebs
Collaborator

Aslak,

There are some AutoBahn tests failing that I hope you can help. Test 1.1.1 (and others for zero length payload) fails with a ClosedSocketException. I think you've documented it here: https://github.com/joewalnes/webbit/issues/38.

Can I ask what was the fix for it? Was it a fix in AutoBahn? I would have thought that the socket will only be closed after the zero payload message has been returned.

Thanks
Vibul

@veebs
Collaborator

Passed all AutoBahn V 0.4.2 tests. Instructions on running the tests can be found in org.jboss.netty.example.http.websocketx.autobahn.package-info.java.

@bobmcwhirter
Collaborator

Stilts also has up to Hybi-13 in its codebase, I'm happy to donate anything you want to steal.

@veebs
Collaborator

Thanks Bob.

I'm afraid I'm a little tied up at the moment.

However, as I understand, major browsers are only supporting up to HyBi-10 so I don't think there's any rush for the time being.

Also, I think it will be great if we can get what we've done so far into a netty release first. I think it will be a good base line. [Trustin, when is the next release? Can we get this in?]

Thanks again. I'll take a look at your code in a month or so when I have some more time.

Vibul.

@trustin
Owner

Hi Vibul,

I love what you wrote. Did you sign up for the Red Hat CLA? Once you sign up, let me just add you to the committer list so that you can contribute directly:

https://cla.jboss.org/login.seam?cid=361

Bob, I'll add ya too!

@aslakhellesoy

If this lands in Netty I would appreciate if I was credited for the Hybi code you forked from Webbit. (Joe Walnes started the Webbit project, but I wrote all the Hybi stuff).

Cheers,
Aslak

@trustin
Owner
@aslakhellesoy

Sure - that would work :-)

I'm really looking forward to this landing in Netty - it would allow us to slim down Webbit quite a bit :-)

@veebs
Collaborator

Hi,

I've added the webbit license file and credits to Aslak in the license file as well as in the header of the hybi encoder/decoder source files. Thanks Aslak for all your hard work.

Trustin, I've just signed the Red Hat CLA. Thanks for adding me to the team :-) Happy to contribute. I'll have some more free time over Dec/Jan so I can help with updating hybi version support. I read somewhere that Netty 4 is in the works so let me know if you would like some help there.

If you are happy with the changes I've made to the license files and the credits, please merge this pull request when you are ready.

Thanks again.

Regards
Vibul

@trustin
Owner
@veebs veebs merged commit c7886a5 into from
@trustin
Owner

Wouldn't it be better put some notice to NOTICE.txt?

@veebs veebs referenced this pull request
Merged

Added webbit to NOTICE file #36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.