Odi, I think it is OK to throw an unchecked exception from createCnonce. I just dislike the idea of throwing plain vanilla RuntimeException. Would it be OK with you to sub-class RuntimeException and give it a reasonably self-explanatory name (MD5NotSupportedYouDummyRuntinmeException)?
Otherwise the patch is good to be committed, IMO Oleg -----Original Message----- From: Ortwin Gl�ck [mailto:[EMAIL PROTECTED] Sent: Thursday, September 18, 2003 10:22 To: Commons HttpClient Project Subject: Re: [PATCH] Reworked digest auth Oleg Kalnichevski wrote: > Odi, the patch looks fine to me. I just dislike this part: > > try { > cnonce = createCnonce(); > } catch(AuthenticationException e) { > throw new RuntimeException(e.toString()); > } > > > At the very least I would rethrow the exception as > MalformedChallengeException, or, preferably, change createCnonce to > throw MalformedChallengeException instead of AuthenticationException in > the very first place. Agreed. It's not nice. The original exception that is generated is because MD5 is not available. I think this should generally trigger an unchecked exception since it is a basic feature needed for HttpClient to work (and should never happen anyway). I think createCnonce should just throw a RuntimeException instead of AuthenticationException. The AuthenticationException is sematically wrong here anyway. > As to the stateful authentication framework, it sounds like a reasonable > improvement, but I suggest that we hold it off until the comprehensive > API redesign that should follow the 2.1 release. Feel free to file a bug > report meanwhile. Okay. --------------------------------------------------------------------- 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]
