Couple of comments:
1. Any specific reason why you chose “fire” for the base name of the
handler function instead of something like “event” ?
2. Instead of calling nextFilter.fire; you might want to call
session.getFilterChain().fire() or
session.getFilterChain().getEntry(this).fire() force correct downward
behavior regardless of current processing direction.
On Fri, Apr 13, 2018 at 3:15 AM, Emmanuel Lécharny <elecha...@gmail.com>
wrote:
> 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
>
>
