Hi Mike,

> is there a need for someone to supply their own AuthScheme?  it 
seems that all of the choices are now hard coded.

I am perfectly aware of limitations of the current design in this regard. Clearly, 
adding custom authentication schemes or extending/customizing existing ones should be 
possible. However, such plugability mechanism, in my opinion, should have wider scope 
than just authentication schemes. HttpClient should provide capability to 'plug-in' 
custom cookie policies, cookie implementations, redirect validators, and what not, 
without having to recompile standard HttpClient package. Besides, such plugability 
mechanism would also call for a better configuration architecture. I just felt such a 
radical change had to be postponed at least until 2.0 release

> should the various auth/* classes be public?

Well, it depends how we envisage the usage of those classes. If we assume that custom 
authentication schemes would be placed in 'httpclient.auth' package, package 
visibility would clearly suffice. That would, however, rule out possibility of putting 
custom authentication schemes into a different package, such as 
'com.mycompany.onehellofauthscheme'. Would not that be a bit too constraining?

> HttpState should use standard bean naming conventions for 
preemptiveAuthentication, something like isAuthenticationPreemptive() 
and setAuthenticationPreemptive()

Agreed

> there are some small style problems, and unused imports

I'll try to clean up a bit. 

If no other objects are raised by 21:00GMT today, I'll commit the patch. I think we 
should move on and work out minor quirks as they are discovered

Many thanks for your feedback, Mike

Cheers

Oleg


-----Original Message-----
From: Michael Becke [mailto:[EMAIL PROTECTED]
Sent: Donnerstag, 27. März 2003 04:19
To: Commons HttpClient Project
Subject: Re: [FEEDBACK NEEDED]: Authentication logic completely
refactored


Hi Oleg,

I like the new design.  It makes things much simpler and more modular.  
I have only a few minor questions/comments:

  - should the various auth/* classes be public?
  - is there a need for someone to supply their own AuthScheme?  it 
seems that all of the choices are now hard coded.
  - HttpState should use standard bean naming conventions for 
preemptiveAuthentication, something like isAuthenticationPreemptive() 
and setAuthenticationPreemptive()
  - there are some small style problems, and unused imports

Again, very nice work.

Mike

On Wednesday, March 19, 2003, at 03:09 PM, Oleg Kalnichevski wrote:

> Folks,
>
> I know I have been a pain in the rear ;-)
>
> Your feedback would be highly appreciated. I know it is quite a bit of 
> a
> patch ;-) So, you are welcome to start throwing bad tomatoes at me
>
> [Taking cover]
>
> Oleg
>
> On Wed, 2003-03-19 at 20:59, [EMAIL PROTECTED] wrote:
>> DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG
>> RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
>> <http://nagoya.apache.org/bugzilla/show_bug.cgi?id=17884>.
>> ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND
>> INSERTED IN THE BUG DATABASE.
>>
>> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=17884
>>
>> Multiple DIGEST authentication attempts with same credentials
>>
>>
>>
>>
>>
>> ------- Additional Comments From [EMAIL PROTECTED]  2003-03-19 19:59 
>> -------
>> While working on a fix for this bug I have come to realize that any 
>> sort of
>> clean solution would require an almost complete authentication logic 
>> redesign.
>> Authenticator#authenticate method needed to be more modular, so that 
>> HttpClient
>> class could access information about authentication scheme being 
>> used. Besides,
>> authentication parsing logic was a complete mess. I was not sure I 
>> could fix it
>> without introducing subtle bugs
>>
>> IMPORTANT: The patch retains full API compatibility with the existing 
>> version.
>> No existing code should be broken.
>>
>> This patch should also fix the following bugs:
>> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=17158
>> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=16861
>>
>> You'll have to perform some manual adjustments after having applied 
>> the patch:
>> - create org.apache.commons.httpclient.auth package
>> - move AuthChallengeParser, AuthenticationException,
>> MalformedChallengeException, AuthScheme, AuthSchemeBase, BasicScheme,
>> DigestScheme, NTLMScheme, RFC2617Scheme, HttpAuthenticator classes to 
>> the newly
>> created package
>>
>> Oleg
>> PS: New classes have not been documented yet
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: 
>> [EMAIL PROTECTED]
>> For additional commands, e-mail: 
>> [EMAIL PROTECTED]
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: 
> [EMAIL PROTECTED]
> For additional commands, e-mail: 
> [EMAIL PROTECTED]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to