[ 
https://issues.apache.org/jira/browse/DIRMINA-672?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Julien Vermillard updated DIRMINA-672:
--------------------------------------

    Fix Version/s:     (was: 2.0.0-M5)
                   2.0.0-RC1

> 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.

Reply via email to