Re: WebSocket progress report
Hello, we're group of three CS grad students taking a course in advanced networking. As a course project, each of us has implemented a basic websocket server in Java with functionality roughly equal to the current state of Mark's patch. The next stage of the project is to continue in a group and get the implementation as far as possible. Since building our own standalone websocket server (that does just websocket) doesn't really make sense, we'd like to join in the effort to make websocket available in Tomcat. We've a few questions: 1) Is there an interest in our help? 2) What are the priority TODOs we should start considering? 3) Some examples (e.g. echo server) of how to use the current implementation to write a servlet would be nice. It'd help us to get up to speed more quickly. Also, please note that although this is a course project sanctioned by the professor, we're not really doing this as a course requirement but rather out of our personal interest in contributing to open source. The scope of the class project is 2-3 weeks but that of course doesn't mean we'll abandon the code after that :) Thanks, Petr Praus
Re: WebSocket progress report
On Tue, Feb 7, 2012 at 03:47, Mark Thomas wrote: > On 07/02/2012 02:23, Jonathan Drake wrote: > > I'm one the three CS grad students working on WebSocket (along with Petr > > Praus). > > > > Just wanted to give an update on our progress, to let you know what we're > > working on: > > > > Adding support for fragmented payloads: > > Excellent. That and handling control frames are the biggest gaps right > now in my view. > > > Right now, after receiving a frame, StreamInbound unmasks the payload in > a > > WsInputStream and passes it up to the servlet via > > onBinaryData()/onTextData(). > > To support fragmented frames, we add an intermediate step: after > unmasking > > the payload, write it to a PipedOutputStream connected to a > > PipedInputStream that we pass upward via onBinaryData()/onTextData(). > When > > the next fragment arrives, keep streaming data through the pipe. > > Piped[Input|Output]Stream are intended to be used with a separate thread > at each end. There is currently only a single thread processing the > incoming data. How do you propose to provide the additional thread? > Ah, then it's different. I thought the client handler is running in different thread. Then we'll probably need something different. > > This has the advantage of also allowing us to stream huge payloads (RFC > > 6455 allows for a 64-bit extended length field---way too much data to > > buffer in memory all at once). > > That is certainly a requirement. However, there is more than one way to > meet that requirement. > Our initial idea was some form of a streaming API where the reader code receiving the message fills a stream that's somehow connected to a client code that would be consuming it. Hence the Piped[Input|Output]Stream stuff. I also think the individual frames of a fragmented messages should not be exposed to the client code. If I remember correctly, RFC specifies that its up to the implementation to decide. What do you think? And, what are your other ideas for supporting huge payloads? > > > It has the minor disadvantage of breaking the ByteBuffer wrappers from > > MessageInbound (we can still use them for small payloads if we buffer > > fragments in memory) > > Could you clarify what is broken in what circumstances please. > MessageInbound is expected to work providing that the message > (regardless of how many fragments it is spread across) is smaller than > the available buffer size. > > My expectation is that the current echo example would continue to work > regardless of whether or not the messages were in a single fragment or > multiple fragments. > > I'm working on a patch that implements this...maybe a day or two. > > > > I'd appreciate any early criticism you may have---otherwise I mainly just > > want to prevent duplicate work by explaining what we're up to. > > Thanks for the heads up. > > The approach you describe isn't the one I had in mind, but that is a > good thing. It provides an opportunity to compare and to take the best > from both. > > It would be nice to have a test case or an example client for this. > Unless there is an easy way to force a browser to fragment packets, I > suspect a test case will be required. > I'm not sure about other browsers but Chrome 16 fragments payloads so that WS frames fit into MSS of the underlying protocol (TCP). So if I sent string larger than roughly 1450 bytes from the client javascript, I got fragments packets. Beware that "lo" device on Linux has by default MTU 16436 so on localhost, the above value of 1450 bytes is much larger. > Given that the implementation currently uses blocking IO Just to make sure, processor.read() calls are always blocking, right? I'm somewhat confused by Nio/Bio/Apr in combination with this. As I understand it, any of the three can be used at the user's will and so the processor can be of type UpgradeNioProcessor. In that case the read on the underlying socket is not blocking. So how come read on the UpgradeProcessor will always be blocking? Can you shed some light on this? Thanks > , my approach was going to be something along the lines of: > a) read the headers > b) call onBinaryData() / onTextData() > c) make the payload available via WsInputStream until the end of the > fragment > d) read the headers for the next fragment > e) make the payload available via WsInputStream until the end of the > fragment > f) repeat d) & e) until the final fragment is processed > g) return EOF on the next read > This would mean moving most of the code currently in > StreamInBound.onData() to WsInputStream > > I haven't tried coding this up or anything so there is no guarantee it > would actually work. > This seems like a sensible approach, I'll look into it. Petr
Re: WebSocket progress report
Hi, sorry for the delay, we got stalled a little bit, I'll post the patch today (US central time) after I manage to merge it (oh the cursed newlines). Thanks, Petr On Mon, Feb 13, 2012 at 14:18, Christopher Schultz < ch...@christopherschultz.net> wrote: > Jeremy, > > On 2/10/12 12:08 PM, Jeremy Brown wrote: > >> I suspect it will need more than that. The XLST will almost certainly > >> need some tweaks too. > > > > How timely, I'm doing xml transformations in my SOA class right now. > > If you have any questions about XSLT, I'd be happy to answer them. It's > definitely something that you have to have a Zenlike relationship with > in order to do properly. > > Just like Lisp, it's possible to write really awful > procedurally-oriented code with it, but then it just sucks horribly. > > We've been using XSLT for a long time with Apache Cocoon (such a great > product) to transform XML into XHTML. I'd be happy to help. > > -chris > >
Re: WebSocket progress report
Hello, attached is our patch. It applies cleanly on top of current trunk rev. 1244719. It has rudimentary support for fragmentation (callback after last frame), supports close messages and ping/pong. Sorry for not sending a patchset but I thought it wouldn't really make sense, since there were quite a lot of back and forth changes. Let me know if I should send a patchset instead. (Jonathan's summary) "Echoing fragments. Close messages. Pings/pongs. Just barely works. :) Fragmentation support is very limited: a mere callback when last frame received. Sends normal closing reply messages and some protocol error closes. Answers pings with pongs. Moving towards a better application API. The servlet container is doing something I don't understand with rapid connection attempts…not sure what's up. I renamed StreamInbound to WebSocketConnection since it's bidirectional. Also I gave the upgrade processors close() methods. Fixed a number of bugs, including a switch statement with accidentally cascading cases, and a problem with in Conversions.byteArrayToLong()." Thanks, Petr On Wed, Feb 15, 2012 at 17:17, Petr Praus wrote: > Hi, sorry for the delay, we got stalled a little bit, I'll post the patch > today (US central time) after I manage to merge it (oh the cursed newlines). > Thanks, > Petr > > > On Mon, Feb 13, 2012 at 14:18, Christopher Schultz < > ch...@christopherschultz.net> wrote: > >> Jeremy, >> >> On 2/10/12 12:08 PM, Jeremy Brown wrote: >> >> I suspect it will need more than that. The XLST will almost certainly >> >> need some tweaks too. >> > >> > How timely, I'm doing xml transformations in my SOA class right now. >> >> If you have any questions about XSLT, I'd be happy to answer them. It's >> definitely something that you have to have a Zenlike relationship with >> in order to do properly. >> >> Just like Lisp, it's possible to write really awful >> procedurally-oriented code with it, but then it just sucks horribly. >> >> We've been using XSLT for a long time with Apache Cocoon (such a great >> product) to transform XML into XHTML. I'd be happy to help. >> >> -chris >> >> > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: WebSocket progress report
Thanks Johno Crawford for pointing out that attachments are stripped, I uploaded the patch here: https://gist.github.com/1844837 On Wed, Feb 15, 2012 at 22:01, Petr Praus wrote: > Hello, attached is our patch. It applies cleanly on top of current trunk > rev. 1244719. It has rudimentary support for fragmentation (callback after > last frame), supports close messages and ping/pong. Sorry for not sending a > patchset but I thought it wouldn't really make sense, since there were > quite a lot of back and forth changes. Let me know if I should send a > patchset instead. > > (Jonathan's summary) > "Echoing fragments. Close messages. Pings/pongs. > > Just barely works. :) Fragmentation support is very limited: a mere > callback when last frame received. Sends normal closing reply messages > and some protocol error closes. Answers pings with pongs. Moving > towards a better application API. The servlet container is doing > something I don't understand with rapid connection attempts…not sure > what's up. I renamed StreamInbound to WebSocketConnection since it's > bidirectional. Also I gave the upgrade processors close() methods. > Fixed a number of bugs, including a switch statement with accidentally > cascading cases, and a problem with in Conversions.byteArrayToLong()." > > Thanks, > Petr > > > On Wed, Feb 15, 2012 at 17:17, Petr Praus wrote: > >> Hi, sorry for the delay, we got stalled a little bit, I'll post the patch >> today (US central time) after I manage to merge it (oh the cursed newlines). >> Thanks, >> Petr >> >> >> On Mon, Feb 13, 2012 at 14:18, Christopher Schultz < >> ch...@christopherschultz.net> wrote: >> >>> Jeremy, >>> >>> On 2/10/12 12:08 PM, Jeremy Brown wrote: >>> >> I suspect it will need more than that. The XLST will almost certainly >>> >> need some tweaks too. >>> > >>> > How timely, I'm doing xml transformations in my SOA class right now. >>> >>> If you have any questions about XSLT, I'd be happy to answer them. It's >>> definitely something that you have to have a Zenlike relationship with >>> in order to do properly. >>> >>> Just like Lisp, it's possible to write really awful >>> procedurally-oriented code with it, but then it just sucks horribly. >>> >>> We've been using XSLT for a long time with Apache Cocoon (such a great >>> product) to transform XML into XHTML. I'd be happy to help. >>> >>> -chris >>> >>> >> >
Re: WebSocket progress report
Hi Mark, I noticed you wrote a websocket test client, I haven't looked at it extensively but I wanted to ask - have you considered using Autobahn for testing? It's rather extensive opensource websocket testing suite. It already contains a lot of stuff we need to test including fragmentation testing. On Fri, Feb 17, 2012 at 14:50, Mark Thomas wrote: > On 16/02/2012 04:01, Petr Praus wrote: > > Hello, attached is our patch. It applies cleanly on top of current trunk > > rev. 1244719. It has rudimentary support for fragmentation (callback > > after last frame), supports close messages and ping/pong. Sorry for not > > sending a patchset but I thought it wouldn't really make sense, since > > there were quite a lot of back and forth changes. Let me know if I > > should send a patchset instead. > > The final diff is fine to work with from my point of view. > > > > > (Jonathan's summary) > > "Echoing fragments. Close messages. Pings/pongs. > > > > Just barely works. :) Fragmentation support is very limited: a mere > > callback when last frame received. > > Repeating what I already wrote in BZ on fragmentation so there is a > complete set of review comments in one place > > I am extremely reluctant to apply the current fragmentation patch. It > relies on buffering individual fragments and - given the maximum > fragment size - that is simply not going to scale. This is why the > current API is built on streams and does not buffer unless the message > based API is used. > > I'd like to fully explore the possibility of supporting fragments > without using buffering before starting down that path. > I understand your concern, the main idea was that the logic that pertains only to a single frame and the information about the frame itself should be encapsulated there. What about letting the client code know there's a new frame before we begin reading the data but still keep the frames encapsulated in WebSocketFrame? > > > Sends normal closing reply messages and some protocol error closes. > Answers pings with pongs. > > The fragmentation implementation is likely to impact how this is done, > so I'd like to resolve that issue first and then come back to this. > > > Moving towards a better application API. > > I think I know what you mean by this but please could you explain in > more detail. > > > The servlet container is doing something I don't understand with rapid > connection attempts…not sure what's up. > > Tell us what you observe and we might be able to explain it. > > > I renamed StreamInbound to WebSocketConnection since it's bidirectional. > > This looks to be related to the refactoring. I'm more worried about > design and features than I am naming right now. > > > Also I gave the upgrade processors close() methods. > > That isn't how socket closure needs to be managed. You'll get away with > it with BIO but things are very likely blow up (with a JVM core) when > you do that with APR. You need to let the surrounding Protocol handle > that by returning the correct socket state. > > > Fixed a number of bugs, including > > Please enumerate all the bugs you fixed so I can ensure that part of the > patch is pulled forward ASAP. > > > a switch statement with accidentally cascading cases, > > That looks to be deliberate to me, although a bunch of TODOs wouldn't > have hurt. > This was actually a bug in my code in WebSocketFrame, it wasn't deliberate :) > > and a problem with in Conversions.byteArrayToLong()." > > Thanks. Fixed. > > > General comment: > The code style of a patch needs to be consistent with the Tomcat code > style. Braces on a new line is the obvious thing that jumped out at me > as I read the patch. Actually, this looks to be confined to a single class. > Thanks for the comment, we'll try to keep it consistent. > Yes, I know the Tomcat code base isn't internally consistent style wise. > The code is 10+ years old and it shows. Use the code in the current > websocket package as a basis. > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
Re: WebSocket progress report
I'm glad to hear that. BTW, Jonathan quite significantly overhauled our implementation over the weekend. The WebSocketFrame no longer buffers data but just stores a reference to inputstream from which the data can be read by the client code receiving the frame. This is not exactly what you originally implemented but I think it's conceptually the same. In the future it would desirable to shield client code from the WebSocketFrame itself and just provide streams. I personally think of WebSocketFrame as being more useful to internal protocol logic than client code which very often does not care about most of the stuff captured in the frame, just the data. I added some fixes yesterday and the code now passes all Autobahn tests including proper closes and close status codes. The only exception are UTF-8-related tests which require reading whole payload and checking that its valid UTF-8. Unfortunately it's no longer easily mergeable with current trunk because of your fragmentation changes on Friday, but if you would be willing to look at it anyway, it's available in our GitHub repository: https://github.com/praus/tomcat (the repository is a fork of the official github apache/tomcat mirror). Petr On Mon, Feb 20, 2012 at 14:35, Mark Thomas wrote: > On 20/02/2012 10:04, Mark Thomas wrote: > > On 20/02/2012 02:55, Petr Praus wrote: > >> but I wanted to ask - have you considered using Autobahn for > >> testing? It's rather extensive opensource websocket testing suite. > > > > I haven't. I just took a quick look. Looks like a nice tool. My test > > client is intended to be something we can run in the unit tests to look > > at very specific issues. It isn't intended to be a general WebSocket > > client, nor to test the full range of functionality. I think Autobhan is > > something we would run separately - a bit like we do with the WebDAV > > test suite and the TCKs. > > I've been looking at this today and so far, my experience has been very > good. Thanks for the tip. It has been really useful. > > I now have all the framing tests passing but the message close is > currently unclean. Next step will be to fix that by adding support for > close. > > Mark > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
Re: WebSocket progress report
On Mon, Feb 20, 2012 at 16:19, Mark Thomas wrote: > On 20/02/2012 21:28, Petr Praus wrote: > > I'm glad to hear that. > > > > BTW, Jonathan quite significantly overhauled our implementation over the > > weekend. The WebSocketFrame no longer buffers data but just stores a > > reference to inputstream from which the data can be read by the client > code > > receiving the frame. This is not exactly what you originally implemented > > but I think it's conceptually the same. > > That sounds much better (I haven't looked at the code in detail). > > > In the future it would desirable to > > shield client code from the WebSocketFrame itself and just provide > streams. > > Indeed. That is how the current implementation in trunk works. > > > I personally think of WebSocketFrame as being more useful to internal > > protocol logic than client code which very often does not care about most > > of the stuff captured in the frame, just the data. > > Agree completely. > > > I added some fixes yesterday and the code now passes all Autobahn tests > > including proper closes and close status codes. > > Very nice. That is better than the current trunk can manage. > > > The only exception are > > UTF-8-related tests which require reading whole payload and checking that > > its valid UTF-8. > > Haven't looked too hard at those yet. Assuming Java can tell the UTF-8 > is invalid then handling that should be easy. > Yes, except that the java CharsetDecoder didn't seem to think that UTF-8 sent by Autobahn as invalid was indeed invalid. At least was my impression, but I might very well be wrong. > > Unfortunately it's no longer easily mergeable with current trunk because > of > > your fragmentation changes on Friday, but if you would be willing to look > > at it anyway, it's available in our GitHub repository: > > https://github.com/praus/tomcat (the repository is a fork of the > official > > github apache/tomcat mirror). > > That is pretty much inevitable when you have a large patch and one of > the main reasons I am trying to keep to small incremental changes in trunk. > > There are some of those trunk changes (I am thinking of > AbstractProcessor) that you'll need to pull into your fork. > I'll try to pull these during the weekend, maybe it'll still be useful. > I am borrowing from your fork where I can and am providing credit in the > commit message so you can see what I have used. You'll also get credit > in the changelog along with the other folks that have contributed code > to the WebSocket impl. > Thanks! > > Based on progress today, things should move along pretty quickly. > Next step is to get ping/pong working. > > Mark > > > > > Petr > > > > On Mon, Feb 20, 2012 at 14:35, Mark Thomas wrote: > > > >> On 20/02/2012 10:04, Mark Thomas wrote: > >>> On 20/02/2012 02:55, Petr Praus wrote: > >>>> but I wanted to ask - have you considered using Autobahn for > >>>> testing? It's rather extensive opensource websocket testing suite. > >>> > >>> I haven't. I just took a quick look. Looks like a nice tool. My test > >>> client is intended to be something we can run in the unit tests to look > >>> at very specific issues. It isn't intended to be a general WebSocket > >>> client, nor to test the full range of functionality. I think Autobhan > is > >>> something we would run separately - a bit like we do with the WebDAV > >>> test suite and the TCKs. > >> > >> I've been looking at this today and so far, my experience has been very > >> good. Thanks for the tip. It has been really useful. > >> > >> I now have all the framing tests passing but the message close is > >> currently unclean. Next step will be to fix that by adding support for > >> close. > >> > >> Mark > >> > >> - > >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > >> For additional commands, e-mail: dev-h...@tomcat.apache.org > >> > >> > > > > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
