On 02/10/2015 07:57 PM, Xinchen Hui wrote:
>> am I wrong?!
> seems I am wrong with this, it's a false alarm...  it can restore 
> automatically.

Yeah, declare() doesn't span files so that isn't a problem.

My worry is still the lack of type coercion for internal calls. I tested
it on some apps and it will take quite a bit of tedious and rather
useless effort to fix these to run in strict mode. Some examples of
common things that are fatal errors in strict mode:

  ini_set('precision', 14);

  ini_set('display_errors', 1);

  ini_set('display_errors', true);

  ok, not common, but tan(4/2) fatal, tan(5/2) no error

  Wordpress has this function, spot the error:

  function add_magic_quotes( $array ) {
        foreach ( (array) $array as $k => $v ) {
                if ( is_array( $v ) ) {
                        $array[$k] = add_magic_quotes( $v );
                } else {
                        $array[$k] = addslashes( $v );
                }
        }
        return $array;
  }

  $v may not always be a string (it died with a float for me), so the
  fix is to cast:

                        $array[$k] = addslashes( (string)$v );

  Also from Wordpress:

  $formatted = number_format( $number, absint( $decimals ),
$wp_locale->number_format['decimal_point'],
$wp_locale->number_format['thousands_sep'] );

  Here number_format() is expecting a float but $number is a string. So
  again, the only real fix is to cast.

  And in Drupal8 *without turning on strict*:

  use Drupal\Component\Utility\String;

  it dies with: "Fatal error: "string" cannot be used as a class name in
/var/www/drupal/core/includes/bootstrap.inc on line 11"

  That String class is everywhere in Drupal. They are going to have a
  fun time with that. See
https://gist.githubusercontent.com/anonymous/d9252deeeb2aae1a5af5/raw/053155130d22551b1404d0a9b94e27424544b6d1/gistfile1

  From Geeklog:

  if (strtolower($topic) != strtolower($archivetid)) {
    $sql .= " AND ta.tid != '{$archivetid}' ";
  }

  $topic can be null there. Looking at the logic, not really a bug,
  just a natural reliance on null being coerced to ""

  Also from Geeklog:

    if( empty( $date )) {
        // Date is empty, get current date/time
        $stamp = time();
    } else if( is_numeric( $date )) {
        // This is a timestamp
        $stamp = $date;
    } else {
        // This is a string representation of a date/time
        $stamp = strtotime( $date );
    }
    // Format the date
    $date = strftime( $dateformat, $stamp );

    strftime() expects an integer for the timestamp there, but as the
    above logic shows, they are expecting a numeric string to be fine.
    No bug, just another forced cast.

    And another number_format() instance where arg1 is not necessarily
    a float. Obviously not a bug, so we have to cast again:

    return number_format( $number, $dc, $ds, $ts );

    In Opencart:

    $this->image = imagecreatetruecolor($width, $height);
    imagecreatetruecolor() expects parameter 1 to be integer, string
given in /var/www/opencart/system/library/image.php on line 89

    You could argue this is a bug, I guess, but the width and height are
    coming from a database and are integers in the db, but since the db
    returns strings. Another cast...

I was genuinely hoping to find some bugs with this exercise. I suppose
it is because I did it on mature projects and at this stage those sorts
of bugs have already been fixed. But I still fear that people are going
to want to be enable strictness everywhere and then they will quickly
learn that they better cast stuff to be safe which makes the whole thing
rather pointless. And I feel pretty sorry for the Drupal folks. That
list of 1000+ instances of their String class is going to suck to fix.

-Rasmus

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to