[
https://issues.apache.org/jira/browse/DIRMINA-672?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12683654#action_12683654
]
Emmanuel Lecharny commented on DIRMINA-672:
-------------------------------------------
It seems pretty obvious that there is a problem :)
I'm testing the test and patching the code. Thanks !
> 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-M1, 2.0.0-M2, 2.0.0-M3, 2.0.0-M4
> Environment: JDK / OS independent
> Reporter: James Talmage
> Assignee: Emmanuel Lecharny
> Fix For: 2.0.0-RC1
>
> Attachments: DemuxingProtocolDecoderBugTest.java
>
>
> 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.
