Bugfix for redirection handling in Zend\Http\Client

[driehle]

When working with Zend\Http\Client today I discovered two issues:

1. Client does not count redirects correctly, so maxredirects=1 did not follow any redirect, maxredirects=2 followed 1 redirect.
2. When following a redirect, the client looses the HTTP authentication, if one was set previously, which causes unexpected results when working with websites which are protected by HTTP basic authentication

The first issue is do to a wrong way of counting the redirects, which I fixed in e196bc3. Basically a compare operator was wrong (< instead of <=). I added a test case for that.

The second issue is due to the fact that the send() method makes a call to resetParameters() prior to following the redirect. Unfortunatelly this removes the HTTP authentication as well. I added a new method clearAuth() (similar to clearCookies()) and changed resetParameters() to only clear the authentication when the newly introduced second parameter is true. To not break backward-compatibilty, this parameter is true by default. All this was done in 954a549, a test case is added as well.

[weierophinney]

Authentication should only be forwarded when inside the same domain (or one of its subdomains); otherwise, you run the risk of exposing your credentials to a third-party if redirected to another domain.

Can you implement logic to clearAuth() if a redirect goes to a different domain?

[driehle]

That's quite a good point, however, in ZF1 this is definitely not done, as Zend_Http_Client::request() is more or less the same than Zend\Http\Client::send() and Zend_Http_Client::resetParameters() does not reset the authentication. Just to mention that you might want to fix ZF1 as well.

I did not implemented the logic in clearAuth() as you suggestet, as I think that this function should always clear the authentication, no matter what. I implemented the logic in the send() method as this is where all the redirection stuff is handled.

I added a test with 3 assertions which basically check the following:

• example.org -> example.com = authentication should be cleared
• example.org -> sub.example.org = authentication should be kept
• sub.example.org -> example.org = authentication should be cleared

I'm still a bit wondering what about the case of a redirection of www.example.org to sub.example.org. Zend\Http\Client would clear the authentication now, even though I think current desktop browsers might handle this case differently.

Another thing I feel a little uncomfortable about is that for the following scenario it is completely up to the response of the first server, how the request to the second server looks like:

$client->setUri('http://www.example.org/')
    ->setAuth('myusername', 'mypassword')
    ->setMethod('GET');
$response = $client->send();

$client->setUri('http://www.google.com/')
    ->setMethod('GET');
$response = $client->send();

If example.org is not answering with a redirect to a different server, then my credentials will be send to Google in the latter request. Otherwise the first request will trigger clearAuth() and the latter request will not contain a HTTP authentication header.

So maybe it would be good to mention in the docs that you should always reset the client between two requests, unless you know exactly what you're doing. The following would work:

$client->setUri('http://www.example.org/')
    ->setAuth('myusername', 'mypassword')
    ->setMethod('GET');
$response = $client->send();

$client->resetParameters(true, true)
    ->setUri('http://www.google.com/')
    ->setMethod('GET');
$response = $client->send();

As resetParameters() is probably not a good name for a function that - if given the right arguments - totally resets the client, it might be a good idea to add a function like this to Zend\Http\Client:

public function reset() 
{
    $this->resetParameters(true, true);
}

[weierophinney]

I'm still a bit wondering what about the case of a redirection of www.example.org to sub.example.org. Zend\Http\Client would clear the authentication now, even though I think current desktop browsers might handle this case differently.

I would expect peer subdomains to allow redirection without clearing authentication.

[weierophinney]

If example.org is not answering with a redirect to a different server, then my credentials will be send to Google in the latter request. Otherwise the first request will trigger clearAuth() and the latter request will not contain a HTTP authentication header.

I would expect that if you call setUri() with a URI that's in a different domain, we should follow the same rules for clearing authentication as with a redirect.

[weierophinney]

As resetParameters() is probably not a good name for a function that - if given the right arguments - totally resets the client, it might be a good idea to add a function like this to Zend\Http\Client:

public function reset()
{
$this->resetParameters(true, true);
}

I agree here.

[weierophinney]

@driehle If you address the feedback I've just provided, I'd be happy to include this in 2.2.0. Can you have the changes by the end of the week?

[driehle]

@weierophinney, I will give it a try ;)

I would expect peer subdomains to allow redirection without clearing authentication.

Well, even though this is a good idea, it might be a bit hart to actually detect peer subdomains. That would require a build-in list of TLDs, since there are some TLDs like .co.uk and others, where the actual domain is on third level. This is why browsers like Firefox use lists (e.g. https://wiki.mozilla.org/TLD_List) to detect what they call an "effective TLD" (see #331510@Mozilla). (Edit: see also http://publicsuffix.org/list/.)

If such a list of effective TLDs is not shipped with ZF, the actually intented security advancement will not be archived, because e.g. google.co.uk could redirect to microsoft.co.uk without triggering a reset of authentication.

Alternatively, we could keep the current implementation or do some kind of left-trimming the string www. of the hostname. Latter would probably be practical, even though it is definetly not the best solution.

[weierophinney]

@driehle Oof -- I'd forgotten about the TLD list. Let's instead drop authentication headers on peer domains, and document why and what approaches developers should take.

[driehle]

@weierophinney Done ;)

Maybe we can create a EffectiveTldService for ZF2 in an latter version. Would Zend\Http\Client\EffectiveTldService be a good place for where to put this service? And how could the list of publicsuffix.org (Mozilla Public License) be distributed with Zend Framework?

[weierophinney]

Maybe we can create a EffectiveTldService for ZF2 in an latter version. Would Zend\Http\Client\EffectiveTldService be a good place for where to put this service? And how could the list of publicsuffix.org (Mozilla Public License) be distributed with Zend Framework?

Yes, absolutely that's a possibility. While that namespace/classname seems appropriate given the initial use case, I could see an argument for having it as a validator, too, as there may be cases when you want to validate that a given TLD is correct (there could even be integration in the Hostname validator for consuming this).

The suffixes can be done as an array -- look at how the hostname validator does this, as well as the new I18n filters and validators that @mwillbanks has been working on (though I think he's now using ResourceCollection from ext/intl).

[weierophinney]

Merged to develop for release with 2.2.0 -- thanks @driehle -- and looking forward to upcoming contributions. :)