On 21/02/2012 16:31, Konstantin Kolinko wrote:
> Regarding StreamInbound#doClose(), doPing(), doPong()
Thanks for the review.
>
> 1. in doClose():
> status = status + is.read();
> There is no check whether is.read() is not -1 above.
Added.
> 2.All 3 methods have such loops:
> while (read > -1) {
> data.position(data.position() + read);
> read = is.read(data.array(), data.position(),
> data.remaining());
> }
>
> I am not sure what happens if data.remaining() becomes 0 but there are
> still data in the stream, e.g. from ill-intended client.
This is handled by the underlying WsInputStream. It ensures that only
the number of bytes declared in the packet header may be read before EOF.
> I think that if there is more data than buffer can handle it will hang looping
> indefinitely with read=0.
Since these are all control frames the size of the payload is limited to
a maximum of 125 bytes therefore I believe we are safe here.
> 3. doPong() method has unbounded while() loop. It wouldn't hang, but I
> think it is bad that there is no control on how many data are
> consumed.
Again, the limit is 125 bytes. This is enforced by the payload length
checks in onData() above combined with the behaviour of WsInputStream.
Of course, I could have made a mistake here so if you see a hole in my
logic please shout. I'll add some comments to the code to clarify what
is going on.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
