CURLOPT_SSL_VERIFYHOST - this option just sounds like "should I verify host?", when it must sound like "what verifying strategy should I use?"

On Thu, 25 Oct 2012 10:19:24 +0400, Adam Harvey <ahar...@php.net> wrote:

On 25 October 2012 13:46, JJ <ja...@php.net> wrote:
On Wed, Oct 24, 2012 at 10:34 PM, Sherif Ramadan
<theanomaly...@gmail.com> wrote:
I understand there are people out there that don't read the
documentation and aren't aware of the difference between
curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, 2); and curl_setopt($ch,
CURLOPT_SSL_VERIFYHOST, true); but still... I don't think this is a
good idea either.

I highly doubt code that sets CURLOPT_SSL_VERIFYHOST => true meant to
imply CURLOPT_SSL_VERIFYHOST => 1...which essentially bypasses host
verification.

They may have, even in spite of it being a bad idea, since that's how
boolean → integer conversion works in PHP. I don't think we can assume
that every single person who's written that line of code didn't check
whether CURLOPT_SSL_VERIFYHOST was a boolean or integer option.

According to libcurl, CURLOPT_SSL_VERIFYHOST => 1 is "not ordinarily a
useful setting".

I agree, but I don't think we can start arbitrarily changing well
defined type conversion behaviour for one corner case. The
CURLOPT_SSL_VERIFYHOST option has been documented as expecting integer
0, 1 or 2 since at least April 2002 (and probably quite a bit earlier
than that), complete with the meanings of each value — there's only so
much we can do to protect developers from themselves. (In fairness,
the wording strongly recommending using option 2 only came in last
August thanks to Ilia, but nobody should have been treating the option
as a boolean option to start with.)

Fundamentally, it's a bad API on the part of curl, but that's a
separate issue. There's nothing stopping somebody proposing a saner
API on top of libcurl (as Anthony did recently with the password
hashing API atop crypt()).

In summary, I'm against changing ext/curl here.

I do have a couple of specific comments on elements of the patch
itself in the event that the changed behaviour is wanted, but I'll
post those on GitHub, since it's probably a better UI for that sort of
granular discussion.

Adam

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to