On 19/06/2012 06:16, Michael Roberts wrote: > Hi, I have been playing with the web sockets support in 7.0.27. I have > some comments and, as requested in the readme, am sending them to the dev > list.
Thanks. Much appreciated. > Caveat: it's been a while since .27, so perhaps some of these issues > are addressed in the nightly builds. Anyhow, here we go: No worries. 7.0.28 is on the mirrors now if you want to try it after reading the rest of the my response below. The announcement will go out later today. > 1) In the createWebSocketInbound method implemented by the chat example's > subclass of WebSocketServlet, there is seemingly no way to interrogate the > incoming request for information like post/get parms as in a conventional > servlet. Since these can be sent later via a message, so this is only a > slight problem. However, a more serious problem is that I can't get at the > IP address of the incoming request when the connection is being created. > This means I can't implement something like a block list (common in games) > to remove annoying things like DOS attacks and misbehaving players from > hitting my server at the point where a connection is created (and hence > resources used). Access to some form of parms would allow me to do > additional validation (like user name and login) at this stage as well. Fair point. Would making the request object available be sufficient? It would almost certainly be wrapped in a façade with the wrapped request set to null after the method completes to prevent sub-classes retaining a reference to the request which would create all sorts of problems. > 2) MessageInbound is oddly named. Really this is a persistent connection, > so likely it should likely be named something like WebSocketConnection. > The chat example hints at this, and maintains a list of "connections", > which are instances of the subclass of MessageInbound. Since you have > onTextMessage and onBinaryMessage methods from which to hang actual message > processing, this rename would be consistent. Another fair point. I can't decide whether to change this now or to wait until JSR 356 is more advanced and change it then. Thoughts? > 3) As mentioned, the chat example maintains a list of connections. > Connections (MessageInbound instances) are added to this list when > connections are established. However, they are never removed when the > connection is closed, so the list grows. The broadcast method silently > ignores the IOException presumably caused by writing to these connections. Opps. We'll get that fixed. > 4) I get what look like a timeout (and the connection closes) in case of > inactivity in the chat example. However, I can't see a way to adjust this, > or turn the behavior off. See previous comment about the non-removal from > the list of connections. Previously, in comet, we dealt with things like > this by doing an immediate reconnect from the javascript side of networking > code, but it would be nice to have control over this during this time > around the wheel. That was a 'feature' of the first implementation. WebSocket now uses an infinite time out by default (this required some low level changes to the APR/native connector which slowed down the 7.0.28 release a little) and the application can change the time out on a per connection basis. Cheers, Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
