[ https://issues.apache.org/jira/browse/DIRMINA-672?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Emmanuel Lecharny resolved DIRMINA-672. --------------------------------------- Resolution: Fixed As far as I remember, the bug has been marked as fix, reopened because of an error in the patch I applied, but should now be considered as closed. > 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.