segfault in http kioslave

[Dawit A.]

On Tuesday 17 September 2002 02:03, Waldo Bastian wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Monday 16 September 2002 10:40 pm, Dawit A. wrote:
> > I know why this bug occurs.  We default to the strongest possible
> > authentication by default which in retrospect is not a good idea since
> > more information is needed to compute digest authentication.  It would
> > have worked without a problem if we defaulted to "Basic".  Will fix this
> > issue. BTW, how did you register the password before hand ?  If it was
> > done through the interface this should not have happened.
>
> We shouldn't default to anything in that case IMO and just make the server
> roundtrip.

Huh ? Come again... This will break everything and you will get bug reports 
that kio_http floods the servers when attempting to get a page, i.e. besides 
the kio_http is slow reports.  I think everybody completely misunderstands 
why and how authentication header is pre-emptively added.  For the sake of 
clarification I add it here:

1.) On every request kio_http determines whether or not it should send 
authorization header by checking the password cache and attempting to find a 
match for the current site based on protocol/hostname/port number and most 
importantly path.

2.) If no match is found it ignores it.  Otherwise, it determines which 
authorization scheme to use by checking whether or not digestInfo is
empty or not.  If not empty ==> digest otherwise basic....

> > Patch below should fix it.  It mostly likely happens because we recently
> > started accepting authentication names in non-case sensitive format, but
> > the code code that sends preemptive authentication info used "startWith"
> > to do the check.
>
> No, if we get "basic" as response we fix that to "Basic" so case sensitive
> comparisons will keep working.

Duh!!  Should have looked at the code more carefully :)  Scratch that then...

> > Hmm... Actually, checkCachedAuthentication should return false if
> > info.digestInfo is empty at least that was the way it was IIRC.
>
> No, the way it was is that info.digestInfo was empty in the case of Basic
> authentication.

Right.

> But that lead to a bug where we authenticated using Basic authentication
> while we should use Digest for some reason. 

When ?? I did not see any bug reports for such problems.  I have thoroughly 
tested authentication at least before the 3.0 release. I have not encountered 
or seen this bug at all.

There is one outstanding scenario I spoke with you about the proxy 
authentication being cached longer than intended since the http io-slaves are 
not immediately killed when the application that was using them is closed, 
but that is another un-related matter.

The only other scenario I can think of that might cause this, at least in the 
old code, is if you authenticated to a given site using Basic Authentication 
and click on another link within that site which requires Digest 
authentication.  Though unlikely in daily setup, this is a valid 
authentication setup and something that needs to be tested, but I seriously 
doubt that you encountered any such sites.  I also could not imagine that a 
site would change authentication schems on a production server on the fly.  
Other than these extreme scenarios I see no reason how this could happen.  
Even the current scenario happened because the DCOP interface was used to 
manually pre-register the authentication info...

> Probably in the same situation that triggers the current bug. 

I personally just don't like the sound of that :(  I think we need to get the 
the bottom of this.  Is there any bug reports on how to trigger this or when 
and how you saw this bug ?  

> Luckily we can now detect this situation and ignore it.

How ?

> > Anyways, here is a better patch to fix both yours and Jean's issue.
> > Default to. Basic and make case insensitive comparison:
>
> No, that's not the right patch. If digestnfo is empty we shouldn't send
> anything because we don't know what the server expects.

Try that and see what happens. :) I added the digestInfo variable to the 
AuthInfo class to store extra  information from digest authentication as its 
name implied. This variable is supposed to be empty if it is Basic 
authentication.  Basic Authentication only needs to store username/password.  
The old code completely relied on this to distinguish which type of 
authentication was supposed to be used.  

Regards,
Dawit A.