Mailing-List: contact dev-help@directory.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Apache Directory Developers List" <dev@directory.apache.org>
Received-SPF: neutral (herse.apache.org: 212.27.42.35 is neither permitted nor
 denied by domain of elecharny@gmail.com)
Message-ID: <45F1E8AC.1050309@gmail.com>
Date: Sat, 10 Mar 2007 00:07:24 +0100
From: Emmanuel Lecharny <elecharny@gmail.com>
User-Agent: Mozilla Thunderbird 1.0.7 (X11/20050923)
MIME-Version: 1.0
To: Apache Directory Developers List <dev@directory.apache.org>
Subject: Re: [LDAP] Merging server-ssl with protocol-ldap
References: <568753d90703091316r25322499x6c4a09e7e28f1039@mail.gmail.com>
	 <45F1D94E.8070700@gmail.com>
 <568753d90703091423n1744e93l3fa3d15b818e8a50@mail.gmail.com>
In-Reply-To: <568753d90703091423n1744e93l3fa3d15b818e8a50@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

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