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

Reply via email to