A question about ReplayingDecoder

[Trustin Lee]

Hi Nick,

On Sun, Apr 5, 2009 at 9:02 PM, Nicholas Clare <nickclare at gmail.com> wrote:
> Hi all,
>
> As a learning excercise, I'm writing a decoder for the STOMP messaging
> protocol. After having a peek at the HTTP decoder in Netty, I was
> extremely impressed by the ReplayingDecoder class. It was a more
> powerful implementation of what I was planning to build myself.

Thanks for the feed back.  I appreciate it! :)

> Not wanting to just copy straight from the HTTP code, but rather learn
> myself, I came up with the following way to read a single line of text
> from the ChannelBuffer:
>
> String line = buffer.readBytes(LF).toString("UTF-8").trim();
>
> where LF is a static import from ChannelBufferIndexFinders. This works
> beautifully, *unless* the connection is terminated half way through a
> line. When that happens, a NoSuchElementException is thrown. At the
> moment, I'm overriding exceptionCaught() to check for the case where
> this exception is thrown, and the connection is no longer open, in
> which case I ignore the exception.

This is a bug.  Here's an explanation:

If a connection is not terminated, NoSuchElementException is not
raised at all because we never know the end of the buffer (i.e. LF
might appear at next read).  However, when a connection is closed,
suddenly we get to know where the buffer ends and therefore know if LF
exists or not.

So, if a connection is closed and the buffer does not contain LF,
NoSuchElementException is raised.  This behavior is inconsistent and
ReplayingDecoder should not propagate NoSuchElementException to you
and ignore it silently.

> So, my question is, supposing I do want to use the
> readBytes(ChannelBufferIndexFinder) method, is this the best way to
> get around the exception problem? Should I just put a try/catch around
> the code above, and if so, what should I do when I catch that
> exception? Just return null? Also, is the overhead of
> readBytes(ChannelBufferIndexFinder) high, and should I rather do it
> the way it's done in the HTTP code?

readBytes(LF) is risky because it doesn't limit the length of the read
bytes.  For example, a malicious client could send 1 gigabyte stuff
without LF, then your server is in trouble.
readBytes(ChannelBufferIndexFinder) is not very useful when the size
of ChannelBuffer is infinite.

If your application uses only LF-delimited messages, I'd suggest you
to use DelimiterBasedFrameDecoder.  Otherwise, I would use the way
done in the HTTP codec.

> Thanks for all your help, and for the amazing Netty library,

I hope my comments were helpful, and let me look forward to your
continuous feed back. :)

— Trustin Lee, http://gleamynode.net/