[
https://issues.apache.org/jira/browse/DIRMINA-385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12506311
]
Chris Audley commented on DIRMINA-385:
--------------------------------------
Only using addLast() to add filters to a Mina session will not prevent the
problem. It merely masks the problem in the most common use cases.
Here are some reasons *NOT* to just leave SSLFilter as is and just add filters
to the end of the chain.
1) This lack of robustness isn't documented, so affected users are going to
have to figure it out for themselves. Why should you have to anticipate that
SSLFilter is only going to work correctly if added first.
2) You don't always have control over how the filters are added. Say your
using another library built on top of MINA, such as QuickFIX/J, that gives you
a limited ability to add additional filters under the hosting library's
control. Third party libraries should have to compensate for SSLFilters
limitations, and users of those libraries should be excluded from taking
advantage of Mina's advanced features
3) Even adding filters in order can't get around all of the SSLFilters bad
behavior. The filter I've written needs to communicate with a proxy server to
establish the connection before any other traffic goes through. The
SSLFilter's practice of queuing up messages before the connection opens
prevents those filters from doing their job, no matter what order you place
them on the chain. For some proxies, I need to read a greeting banner before I
can formulate a correct first message, by then SSLFilter has already jammed its
handshake into the queue.
Why try to excuse bad implementation when it can be fixed?
> SSLFilter starts writing to session before filter chain is complete
> -------------------------------------------------------------------
>
> Key: DIRMINA-385
> URL: https://issues.apache.org/jira/browse/DIRMINA-385
> Project: MINA
> Issue Type: Bug
> Components: Filter
> Affects Versions: 1.0.3, 1.1.0
> Reporter: Chris Audley
> Fix For: 1.0.4, 1.1.1, 2.0.0-M1
>
> Attachments: SSLFilter-1.1.0.patch, SSLFilter-HEAD.patch
>
>
> The SSLFilter starts to write packets to the session as soon as it is added
> to the filter chain (in onPostAdd). However, this can result in errors if a
> filter exists or is added before SSLFilter in the processing order. Writing
> to the session in onPostAdd should be considered breaking the contract
> between filters and the session.
> If a filter is added in the processing order before the SSLFilter, but after
> the SSLFilter has been added, SSLFilter will have already sent some messages
> that the newly added filter will never have the opportunity to process. Any
> subsequent SSLFilter messages *will* be processed by the newly added filter,
> potentially creating inconsistencies.
> Even if the filter before the SSLFilter in processing order is added before
> the SSLFilter is added, having SSLFilter send messages in onPostAdd ignores
> the possibility that the other filter needs to send and/or receive messages
> before the connection is considered open by later filters in the chain.
> The specific circumstances of my problem is a web proxy traversal filter that
> needs to be placed in front of the SSLFilter in order to function correctly.
> This filter is responsible for sending the CONNECT message to the proxy to
> open an outside connection before the SSL session is negotiated. With the
> current behavior of SSLFilter, this is simply not possible.
> I have created a patch for SSLFilter that moves the code for initiated the
> handshake to sessionOpened. This works correctly in my situation, but breaks
> the case where an SSLFilter is added to the chain after the connection is
> opened. I try to address this by adding a conditional handshake initiation
> to onPostAdd only if the session is already connected. Unfortunately, the
> IoSession.isConnected() method only indicates if the session hasn't been
> closed. What SSLFilter.onPostAdd() needs is a way to determine if the
> sessionOpened event for the session has been fired. My patches disable the
> code in onPostAdd() until isConnected or another method is able to provide
> the necessary information.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
