Re: Changing the default value of "true" for CURLOPT_SSL_VERIFYHOST

On 10/24/2012 11:34 PM, Sherif Ramadan wrote:

On Thu, Oct 25, 2012 at 1:03 AM, JJ <jawed@php.net> wrote:

Hey all - I'd like start a discussion around pull request 221 (https://github.com/php/php-src/pull/221). In short, there's a high volume of [incorrect] code out there which looks like: curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, true); Instead of what, in all likelyhood, the code meant to do: curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, 2); This is due to the convert_to_long_ex call which converts "true" to 1L. CURLOPT_SSL_VERIFYHOST being set to 1L bypasses common name validation within libcurl. My solution was to check the type for CURLOPT_SSL_VERIFYHOST: if it is boolean and true, the opt value for libcurl is set to 2L. I understand that engineers should have the proper option value to begin with but weighing the impact of this (MITM attacks) against doing what they probably meant anyways is worth the presumption. Please discuss and adjust the patch if necessary. - JJ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php

While I think it's a good idea to set the value of the option to 2, as is recommended for production in the documentation, I think the idea of implicitly converting a bool(true) to 2L internally might lead to unexpected behavior since some people might actually depend on normal PHP behavior to cast a bool(true) to 1 (and that might be what they actually intended). 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. We should probably just elaborate on this point a bit more in the documentation. Perhaps add a note and an example to illustrate. I notice that people tend to pay more attention to examples than anything else in the docs.

Booleans ought to be 1 and 0. Casting a boolean to 2 is just wrong, a way to fix badly written code a few people have written and in so doing risk the breakage of far more code that is correct.