[jira] Commented: (DIRMINA-385) SSLFilter starts writing to session before filter chain is complete

["Chris Audley (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/DIRMINA-385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12512149 ] 

Chris Audley commented on DIRMINA-385:
--------------------------------------

Its not like this is a particularly intractable issue.  The code could easily be adapted to heed the appropriate events from the session to determine when to start the SSL session.  

The problem is the filter may be added to the chain after it has processed the sessionOpened event.  In this case, there isn't sufficient information for the filter to determine whether or when to start the SSL session.  The current approach, just assume the programmer wants the session started immediately, is flawed.  It works when the assumption is correct, but it is absolutely not always correct, and you can't possibly state with any credibility how often it will be correct.  The better approach is to allow the developer using the filter to explicitly indicate when to start the session.  The means is already there, the startSSL(IoSession) method.  If the user wants to add the filter late, thats what they should be doing.  If it makes you fell better, add a constructor that allows the developer to turn immediate session negotiation on as a convenience, but don't make it a requirement.

Your suggestion that I can just add the SSLFilter later doesn't work.  As I indicated before, not everyone is using MINA directly.  If MINA is hidden from the developer behind another library interface, you can't just assume there will be an opportunity to add additional filters whenever necessary.

Please fix this code, it isn't that hard.


> 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
>            Assignee: Trustin Lee
>         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.