CumulativeProtocolDecoder/ DemuxingProtocolDecoder
--------------------------------------------------
Key: DIRMINA-672
URL: https://issues.apache.org/jira/browse/DIRMINA-672
Project: MINA
Issue Type: Bug
Components: Filter
Affects Versions: 2.0.0-M4, 2.0.0-M3, 2.0.0-M2, 2.0.0-M1
Environment: JDK / OS independent
Reporter: James Talmage
Excerpt from forum discussion at:
http://www.nabble.com/Potential-DemuxingProtocolDecoderBug-td22489974.html
I'm using 2.0.0-M4. Upon inspection of the source code, I can tell it's going
to be a JDK / OS independent issue. Also upon inspection, I've discovered the
bug is actually in the CumulativeProtocolDecoder (starting @ line 125):
if (!session.getTransportMetadata().hasFragmentation()) {
doDecode(session, in, out);
return;
}
This breaks the contract with the doDecode method as it is only called once
(the documentation says it should be called repeatedly until it returns false).
The following changes makes my previous test case pass, but it's probably a
little simplistic.
if (!session.getTransportMetadata().hasFragmentation()) {
while(in.hasRemaining() && doDecode(session, in, out)){
//Do nothing
}
return;
}
The code should probably make sure that if doDecode returns true, some of the
buffer was actually consumed (as the code for fragmented transports does).
Also, it may make sense to provide another method (i.e.
finishNonFragmentedDecode(...)), for handling the remainder of the buffer after
doDecode returns false.
I see where the author was headed with this code. Transports (such as UDP)
that don't support fragmentation probably don't jive with the concept of an
accumulating buffer (what do we do with the accumulation buffer if we drop a
UDP packet?). It does however make perfect sense to use such transports with
the concept of a DemuxingProtocolDecoder. Perhaps it would be better to
refactor the DemuxingProtocolDecoder so that it's not a subclass of
CumulativeProtocolDecoder. Create a helper class that handles the fragment
accumulation aspect. CumulativeProtocolDecoder would always use said helper
class (throwing an error if the protocol doesn't support fragmentation), but
DemuxingProtocolDecoder could opt to use it depending on the protocol it
encounters.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
