You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Frederic Soulier <fr...@wallaby.uklinux.net> on 2007/11/01 11:59:44 UTC
Issue upgrading from 1.1.3 to 1.1.4
Hi
We are seeing a really odd behaviour after upgrading yesterday to
1.1.4 from 1.1.3
Basically if we see an invalid message (we deem it invalid as part of
our business logic) we just throw an exception and close the ioSession.
It worked perfectly up to 1.1.3 and is completely buggered in 1.1.4...
In 1.1.4 we get tons of exception because of the newly introduced
while loop (while (in.hasRemaining())) in the try {} catch block of
the ProtocolCodecFilter messageReceived(...)
MINA 1.1.3
==========
@Override
public void messageReceived(NextFilter nextFilter, IoSession
session,
Object message) throws Exception {
if (!(message instanceof ByteBuffer)) {
nextFilter.messageReceived(session, message);
return;
}
ByteBuffer in = (ByteBuffer) message;
ProtocolDecoder decoder = getDecoder(session);
ProtocolDecoderOutput decoderOut = getDecoderOut(session,
nextFilter);
try {
synchronized (decoderOut) {
decoder.decode(session, in, decoderOut);
}
} catch (Throwable t) {
ProtocolDecoderException pde;
if (t instanceof ProtocolDecoderException) {
pde = (ProtocolDecoderException) t;
} else {
pde = new ProtocolDecoderException(t);
}
pde.setHexdump(in.getHexDump());
throw pde;
} finally {
// Dispose the decoder if this session is connectionless.
if (session.getTransportType().isConnectionless()) {
disposeDecoder(session);
}
// Release the read buffer.
in.release();
decoderOut.flush();
}
}
MINA 1.1.4
==========
@Override
public void messageReceived(NextFilter nextFilter, IoSession
session,
Object message) throws Exception {
if (!(message instanceof ByteBuffer)) {
nextFilter.messageReceived(session, message);
return;
}
ByteBuffer in = (ByteBuffer) message;
ProtocolDecoder decoder = getDecoder(session);
ProtocolDecoderOutput decoderOut = getDecoderOut(session,
nextFilter);
try {
while (in.hasRemaining()) { <<< What for???
int oldPos = in.position();
try {
synchronized (decoderOut) {
decoder.decode(session, in, decoderOut);
}
// Finish decoding if no exception was thrown.
decoderOut.flush();
break;
} catch (Throwable t) {
ProtocolDecoderException pde;
if (t instanceof ProtocolDecoderException) {
pde = (ProtocolDecoderException) t;
} else {
pde = new ProtocolDecoderException(t);
}
pde.setHexdump(in.getHexDump());
// Fire the exceptionCaught event.
decoderOut.flush();
nextFilter.exceptionCaught(session, pde);
// Stop retrying if the buffer position didn't
change
// because retrying can cause an infinite loop.
if (in.position() == oldPos) {
break;
}
}
}
} finally {
// Release the read buffer.
in.release();
}
}
I can't see anything related to this change in the changelog and the
reason why.
This is a major regression considering that if you have a message
that is 1000 bytes long and the 1st byte is deemed invalid and you
want to close this ioSession you will get 1000 times the same
exception that fills up MBs of logs in seconds!!!
Right now we're going back to 1.1.3
--
Frederic P. Soulier
OpenPGP key available on http://pgpkeys.mit.edu/
1024D/BA6700ED 49A6 8E8E 4230 8D41 1ADE B649 3203 1DD2 BA67 00ED
Re: Issue upgrading from 1.1.3 to 1.1.4
Posted by Frederic Soulier <fr...@wallaby.uklinux.net>.
Hi Trustin
Thx for looking into this.
I think having a way to define the behaviour when a
ProtocolDecoderException is thrown (carry on decoding or stop) would
be just fine.
On 1 Nov 2007, at 13:44, Trustin Lee wrote:
> On 11/1/07, Trustin Lee <tr...@gmail.com> wrote:
>> Hi Frederic,
>>
>> On 11/1/07, Frederic Soulier <fr...@wallaby.uklinux.net> wrote:
>>> Hi
>>>
>>> We are seeing a really odd behaviour after upgrading yesterday to
>>> 1.1.4 from 1.1.3
>>>
>>> Basically if we see an invalid message (we deem it invalid as
>>> part of
>>> our business logic) we just throw an exception and close the
>>> ioSession.
>>> It worked perfectly up to 1.1.3 and is completely buggered in
>>> 1.1.4...
>>> In 1.1.4 we get tons of exception because of the newly introduced
>>> while loop (while (in.hasRemaining())) in the try {} catch block of
>>> the ProtocolCodecFilter messageReceived(...)
>>
>> <snip/>
>>
>>> I can't see anything related to this change in the changelog and the
>>> reason why.
>>>
>>> This is a major regression considering that if you have a message
>>> that is 1000 bytes long and the 1st byte is deemed invalid and you
>>> want to close this ioSession you will get 1000 times the same
>>> exception that fills up MBs of logs in seconds!!!
>>>
>>> Right now we're going back to 1.1.3
>>
>> First off, I apologize for your inconvenience. I had to be more
>> careful. :-(
>>
>> The change occurred while I fix DoS bug in TextLineDecoder. The
>> latest TextLineDecoder now can keep decoding incoming data even if
>> any
>> broken data is received (i.e. too long line). With the
>> ProtocolCodecFilter implementation in 1.1.3, it was not able to
>> continue decoding until any new data is received even if there was
>> more than one line in the previous buffer. This means, if client is
>> sending a too long line + a short line, the messageReceived event for
>> the short line might not be fired. That was why inserted a loop.
>>
>> I'd better revert the change back in 1.1.5 and 1.0.8, and provide a
>> way to take care of TextLineDecoder case in 2.0.0. We could
>> provide a
>> subclass of ProtocolDecoderException such as
>> RecoverableProtocolDecoderException so ProtocolCodecFilter can decide
>> to loop or not. WDYT?
>
> I filed the related JIRA issues in case you want to keep track of
> the problem.
>
> https://issues.apache.org/jira/browse/DIRMINA-465
> https://issues.apache.org/jira/browse/DIRMINA-466
>
> HTH,
> Trustin
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
> --
> PGP Key ID: 0x0255ECA6
--
Frederic P. Soulier
OpenPGP key available on http://pgpkeys.mit.edu/
1024D/BA6700ED 49A6 8E8E 4230 8D41 1ADE B649 3203 1DD2 BA67 00ED
Re: Issue upgrading from 1.1.3 to 1.1.4
Posted by Trustin Lee <tr...@gmail.com>.
On 11/1/07, Trustin Lee <tr...@gmail.com> wrote:
> Hi Frederic,
>
> On 11/1/07, Frederic Soulier <fr...@wallaby.uklinux.net> wrote:
> > Hi
> >
> > We are seeing a really odd behaviour after upgrading yesterday to
> > 1.1.4 from 1.1.3
> >
> > Basically if we see an invalid message (we deem it invalid as part of
> > our business logic) we just throw an exception and close the ioSession.
> > It worked perfectly up to 1.1.3 and is completely buggered in 1.1.4...
> > In 1.1.4 we get tons of exception because of the newly introduced
> > while loop (while (in.hasRemaining())) in the try {} catch block of
> > the ProtocolCodecFilter messageReceived(...)
>
> <snip/>
>
> > I can't see anything related to this change in the changelog and the
> > reason why.
> >
> > This is a major regression considering that if you have a message
> > that is 1000 bytes long and the 1st byte is deemed invalid and you
> > want to close this ioSession you will get 1000 times the same
> > exception that fills up MBs of logs in seconds!!!
> >
> > Right now we're going back to 1.1.3
>
> First off, I apologize for your inconvenience. I had to be more careful. :-(
>
> The change occurred while I fix DoS bug in TextLineDecoder. The
> latest TextLineDecoder now can keep decoding incoming data even if any
> broken data is received (i.e. too long line). With the
> ProtocolCodecFilter implementation in 1.1.3, it was not able to
> continue decoding until any new data is received even if there was
> more than one line in the previous buffer. This means, if client is
> sending a too long line + a short line, the messageReceived event for
> the short line might not be fired. That was why inserted a loop.
>
> I'd better revert the change back in 1.1.5 and 1.0.8, and provide a
> way to take care of TextLineDecoder case in 2.0.0. We could provide a
> subclass of ProtocolDecoderException such as
> RecoverableProtocolDecoderException so ProtocolCodecFilter can decide
> to loop or not. WDYT?
I filed the related JIRA issues in case you want to keep track of the problem.
https://issues.apache.org/jira/browse/DIRMINA-465
https://issues.apache.org/jira/browse/DIRMINA-466
HTH,
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6
Re: Issue upgrading from 1.1.3 to 1.1.4
Posted by Trustin Lee <tr...@gmail.com>.
Hi Frederic,
On 11/1/07, Frederic Soulier <fr...@wallaby.uklinux.net> wrote:
> Hi
>
> We are seeing a really odd behaviour after upgrading yesterday to
> 1.1.4 from 1.1.3
>
> Basically if we see an invalid message (we deem it invalid as part of
> our business logic) we just throw an exception and close the ioSession.
> It worked perfectly up to 1.1.3 and is completely buggered in 1.1.4...
> In 1.1.4 we get tons of exception because of the newly introduced
> while loop (while (in.hasRemaining())) in the try {} catch block of
> the ProtocolCodecFilter messageReceived(...)
<snip/>
> I can't see anything related to this change in the changelog and the
> reason why.
>
> This is a major regression considering that if you have a message
> that is 1000 bytes long and the 1st byte is deemed invalid and you
> want to close this ioSession you will get 1000 times the same
> exception that fills up MBs of logs in seconds!!!
>
> Right now we're going back to 1.1.3
First off, I apologize for your inconvenience. I had to be more careful. :-(
The change occurred while I fix DoS bug in TextLineDecoder. The
latest TextLineDecoder now can keep decoding incoming data even if any
broken data is received (i.e. too long line). With the
ProtocolCodecFilter implementation in 1.1.3, it was not able to
continue decoding until any new data is received even if there was
more than one line in the previous buffer. This means, if client is
sending a too long line + a short line, the messageReceived event for
the short line might not be fired. That was why inserted a loop.
I'd better revert the change back in 1.1.5 and 1.0.8, and provide a
way to take care of TextLineDecoder case in 2.0.0. We could provide a
subclass of ProtocolDecoderException such as
RecoverableProtocolDecoderException so ProtocolCodecFilter can decide
to loop or not. WDYT?
Thanks,
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6