Is this the right way to write a ProtocolDecoder in MINA?

Question

public class CustomProtocolDecoder extends CumulativeProtocolDecoder{
byte currentCmd = -1;
int currentSize = -1;
boolean isFirst = false;
@Override
protected boolean doDecode(IoSession is, ByteBuffer bb, ProtocolDecoderOutput pdo) throws Exception {
        if(currentCmd == -1)
        {
            currentCmd = bb.get();
            currentSize = Packet.getSize(currentCmd);
            isFirst = true;
        }
        while(bb.remaining() > 0)
        {
            if(!isFirst)
            {
                currentCmd = bb.get();
                currentSize = Packet.getSize(currentCmd);
            }
            else
                isFirst = false;
            //System.err.println(currentCmd + " " + bb.remaining() + " " + currentSize);
            if(bb.remaining() >= currentSize - 1)
            {
                Packet p = PacketDecoder.decodePacket(bb, currentCmd);
                pdo.write(p);
            }
            else
            {
                bb.flip();
                return false;
            }
        }
        if(bb.remaining() == 0)
            return true;
        else
            return false;
}

}

Anyone see anything wrong with this code? When a lot of packets are received at once, even when only one client is connected, one of them might get cut off at the end (12 bytes instead of 15 bytes, for example) which is obviously bad.

Answer 1

Hi there, I'm finding it a bit difficult to understand what protocol you're trying to decode here. It's definitely looking a bit confused in there ;)

Are you writing something which expects many requests on the same connection? If so, then great, that's what Mina's good at ...

Normally, I'd expect a MINA decoder to be checking whether it's got a complete message, and then, if not, to return the IoBuffer's pointer back to the position it held at the start of the method.

Normally a complete message would be determined by a delimiter, or perhaps a length field at the start of the message.

The example provided in the api docs is pretty good. It's looking for a delimiter of Carriage Return + Line Break:

http://mina.apache.org/report/trunk/apidocs/org/apache/mina/filter/codec/CumulativeProtocolDecoder.html

hth

Answer 2

Figured it out with some help from the example -- I had true and false confused, and didn't realize I was supposed to keep track of the position of the input buffer. And on top of that, I didn't know that I didn't need the while loop. Thanks!

    protected boolean doDecode(IoSession is, ByteBuffer bb, ProtocolDecoderOutput pdo) throws Exception {
    int start = bb.position();
    currentCmd = bb.get();
    currentSize = Packet.getSize(currentCmd);
    //System.err.println(currentCmd + " " + bb.remaining() + " " + currentSize);
    if(bb.remaining() >= currentSize - 1)
    {
        Packet p = PacketDecoder.decodePacket(bb, currentCmd);
        pdo.write(p);
        if(bb.remaining() == 0)
            return false;
        else
            return true;
    }
    else
    {
        bb.position(start);
        return false;
    }
}