Hi Jonathan, did you had time to review teh diff ?
I'd like to push it this week-end if you don't have any remark or
comment on it.
Many thanks !
Le 09/04/2018 à 17:17, Jonathan Valliere a écrit :
> I’d be interested in the diff. Why not propagate the sent down the chain
> instead of just to the handler? I have a generic event system that I use
> to notify users fo server events, like pending shutdown or app reloads.
>
> On Sun, Apr 8, 2018 at 11:56 AM, Emmanuel Lécharny <elecha...@gmail.com>
> wrote:
>
>> Ok, I have some kind of 'elegant' soution that is generic enough, with
>> the fire(event) method added.
>>
>> Each filter may fire some specific event, they will all be caught in the
>> IoHandler in one single method, up to the application to take care of
>> the kind of event it is interested in.
>>
>> I'm not using an 'int' as it will be problematic for those implementing
>> filters (one may not want to check every existing filter to know which
>> integer it can use safely). I'm using an interface (FilterEvent) and
>> enum implemting this interface :
>>
>> public interface FilterEvent {
>> }
>>
>> and
>>
>> public enum SslEvent implements FilterEvent {
>> SECURED,
>> UNSECURED
>> }
>>
>> In the IoHandler implementation, that gives :
>>
>> public void fire(IoSession session, FilterEvent event) throws Exception{
>> if (event == SslFilter.SESSION_UNSECURED ) {
>> counter.countDown();
>> }
>> }
>>
>> The big plus is that we have a typed event, we don't have to take care
>> of what we don't know (ie, other filters event details)
>>
>> And in the SslHandler itself :
>>
>> ...
>> // Send the SECURE event only if it's the first SSL handshake
>> if (firstSSLNegociation) {
>> firstSSLNegociation = false;
>> nextFilter.fire(session, SslEvent.SECURED);
>> }
>> ...
>>
>> Tests are passing green.
>>
>> Wdyt ?
>>
>> (side note, ther eis more code than just what I exposed, I will probably
>> create a diff for those of you interested in the details. The code is
>> pretty straightforward, there is really nothing complicated)
>>
>> Tahnks !
>>
>>
>> Le 05/04/2018 à 15:42, Jonathan Valliere a écrit :
>>> Yes, its different in that it may act like a buffer, in some phases, but
>>> doesn’t necessarily break the pattern of filter use. What your
>> FilterChain
>>> needs is fire(index) options so the SSL can fire writes relative to
>> itself,
>>> in response to a read, without having to choke down the whole chain with
>>> special exceptions via flags in setAttributes. Make a lot of the design
>>> easier. IMHO, I think the biggest problem is the design of SSL filter
>> and
>>> the limitations of the Filter API.
>>>
>>> On Thu, Apr 5, 2018 at 9:22 AM, Emmanuel Lécharny <elecha...@gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> Le 05/04/2018 à 14:18, Jonathan Valliere a écrit :
>>>>> I am concerned that it is bad precedent to add handler methods based on
>>>>> specific filters. The purpose of the filter system is that each filter
>>>> has
>>>>> no direct knowledge of what is before or after it. Maybe there could
>> be
>>>> a
>>>>> generic “event” handler as part of the receive chain that the SECURED
>>>> event
>>>>> could flow down instead?
>>>>
>>>> Years agao, we had a discussion about the place where the SSL/TLS
>>>> handling should be done. I strongly believe that it does not belong to a
>>>> filter (this is a source of confusion, because it always has to be the
>>>> very first filter in the chain), but to the HeadFilter (or something
>>>> like that).
>>>>
>>>> In other words, establishing a secured session is intimely coupled with
>>>> any IO operation, regardless of the application built on top of it. Any
>>>> other filter is functional, and related to the protocol being set, but
>>>> TLS/SSL is different, IMHO.
>>>>
>>>> Now, your idea to have a otherEvent() method - or whatever could be the
>>>> name - that covers any event we would like to generate in any filter
>>>> sounds like a good idea too. We can explore this route.
>>>>
>>>> And, to be frank, this is the reason I asked on the mailing list :
>>>> having the opportunity to ear about other proposals that could make more
>>>> sense !
>>>>
>>>>
>>>> Thanks !
>>>>
>>>> --
>>>> Emmanuel Lecharny
>>>>
>>>> Symas.com
>>>> directory.apache.org
>>>>
>>>>
>>>
>>
>> --
>> Emmanuel Lecharny
>>
>> Symas.com
>> directory.apache.org
>>
>>
>
--
Emmanuel Lecharny
Symas.com
directory.apache.org
pEpkey.asc
Description: application/pgp-keys
