directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lecharny <elecha...@gmail.com>
Subject Re: [LDAP] Merging server-ssl with protocol-ldap
Date Fri, 09 Mar 2007 23:07:24 GMT
Enrique Rodriguez a écrit :

> On 3/9/07, Emmanuel Lecharny <elecharny@gmail.com> wrote:
>
>> ...
>> > server-ssl only has 2 classes in it and it is setting up a really
>> > problematic cyclic dependency with core-unit, server-unit, and
>> > server-jndi.
>>
>> I think that the cyclic dependency can be solved if we modify the
>> LdapsITest class : it uses Attribute(s)Impl when it should use
>> BasicAttribute(s) instead.
>
>
> If you look at server-ssl, you'll see that LdapsInitializer is minor
> utility code for loading a cert from a keystore and using it to
> initialize an SSLContext.  The other class is a stub for an
> X509TrustManager.  The bulk of the SSL work is in mina-filter-ssl, so
> it's not like I'm suggesting we merge major SSL functionality into
> protocol-ldap.

Sorry, I didn't had a deep look at these classes, so my opinion might 
not have been solid.

>
> The AttributeImpl you mention is from shared-ldap.  The problem is
> that the test uses AbstractServerTest from server-unit, which uses
> MutableServerStartupConfiguration from server-jndi.  

Yes, that's bad.

> server-jndi also
> contains ServerContextFactory, which is where the SslFilter is being
> changed from reflection to construction, hence the cyclic dependency.
> The problem may be solved by moving the ITest to server-unit, where
> the other integration tests seem to be migrating.

That's seems to be a good idea.

> But then we're back
> to having 2 minor classes in their own module.

Well, rereading your initial mail and your explanations, plus the code, 
yes, I think you are right. Let's move this code to protocol-ldap. May 
be some part of it might be push to some common project (like 
shared-utils) (I'm thinking about ServerX509TrustManager class), but as 
this project does not exist, I will favor simplicity.

>
>> > We never noticed the cycle before because the SslFilter
>> > was class-loaded using reflection and when I use normal construction
>> > m2 flags it and ceases to build.  The only reason I could see to have
>> > server-ssl separate was, again, the JDK 1.5+ issue, which is gone now.
>> >
>> > I would do this in the SASL branch.  I think it makes sense to deal
>> > with more tightly integrating SSL at the same time as SASL, since it's
>> > all part of a cohesive LDAP protocol impl.
>>
>> Yeah, but the point is that it would also be cool to separate SSL
>> handling from Ldap protocol., IMHO. For SASL, that's a totally different
>> story, but anyway, a server-sasl could also be a good idea, because we
>> never know if another mechanism may be invented or added.
>
>
> Again, the SSL handling is in mina-filter-ssl.  

Damn, you are right !

> Also, when Start TLS
> moves over, we'll have redundant code since LdapsInitializer returns a
> built filter chain and StartTLS does dynamic filter insertion on
> receipt of a Start TLS command.  So, we'll have even less in
> server-ssl.

Ok, you convinced me. I give you my +1 for that. And as you do it in 
your own branch, that's fine. If anybody else have a better idea, this 
mailing list will be the way to express it. I guess that it won't be the 
case.

Anyway, thanks a lot for your mails, I get some more knowledge of parts 
of the server I was not aware of !

Emmanuel


Mime
View raw message