commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From d...@multitask.com.au
Subject RE: [HttpClient][Request] Ability to debranch some logic
Date Sat, 03 Aug 2002 07:18:28 GMT
"Jeff Dever" <jdever1@nortelnetworks.com> wrote on 08/03/2002 07:40:47 AM:

> Hey Vincent,
> 
> I'm not sure about dIon's Latka tags, but you've certainly touched on 
some
> nasty code in httpclient.  I had a closer look at your previous comment
> about httpclient going infinite in a given situation.  The
> HttpMethodBase.execute method contains 150 lines of forwarding, 
redirecting,
> authenticating logic in a for(;;) loop.  There is a little note in 
comments
> too: //this method is too big.  I'd say execute is in need of 
refactoring.
I started to refactor, but the method was huge initially, hence my FIXME 
comment. I got some of it done, but thought I'd commit while I can :-)

> There are some other limitations in there too, such as redirecting to
> another host or port, the infinite loop problem and the unavailable
> unauthorized status codes.
Yep, and they we're impossible to find originally. Now we've both done 
some refactoring it might be easier....

> I'm working on refactoring it now, but its pretty sensitive.
I'll look @ it tonight, linux upgrade allowing :)

> dIon,
> 
> Thanks. I am probably blind but I can't find an example of any 401 test
> in the URL you provided (authentication). I can test all response codes
> fine with HttpClient except 401, hence my initial email.
There aren't any 401 tests in there, but as you've pointed out, they're 
usually swallowed by HttpClient... so I think that the code is broken 
there....I'll have a look @ Jeff's changes tonight...

> Here is the HttpClient code that is involved (I think) :
> 
>             if (HttpStatus.SC_UNAUTHORIZED == statusCode) {
>                 Header wwwauth = getResponseHeader("WWW-Authenticate");
>                 if (null != wwwauth) {
> [some code to send another request to authenticate]
> 
> I haven't followed closely the code but I don't see any code to debranch
> this. For redirects it works fine as the code is :
> 
>             } else if (HttpStatus.SC_MOVED_TEMPORARILY == statusCode ||
>                HttpStatus.SC_MOVED_PERMANENTLY == statusCode ||
>                HttpStatus.SC_TEMPORARY_REDIRECT == statusCode) {
>                 if (getFollowRedirects()) {
> [...]
> 
> thus getFollowRedirects() returns false is HttpClient is set not to
> follow redirects.
--
dIon Gillard, Multitask Consulting
Work:      http://www.multitask.com.au
Developers: http://adslgateway.multitask.com.au/developers

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message