Normalize null input behavior

[bcremer]

This PR normalizes the behavior for null or not string inputs. escapeHtml() and escapeUrl() are using a implicit string cast so $escaper->escapeHtml(null) and $escaper->escapeUrl(null) will return an empty string.

$escaper->escapeCss/escapeJs/escapeHtmlAttr(null) will throw the following Exception instead:

Zend\Escaper\Exception\RuntimeException: String to be escaped was not valid UTF-8 or could not be converted.

With this PR all inputs will be casted to a string so the behavior of all methods is consistent.

[Maks3w]

I think the exception raised on escapeJs has sense because if someone want to print null as JS output the function should be or return the PHP string "null" or warn the user he/she must check if the value is null and print the null value itself.

($value === null) ? "null" : escapeJs($value)

If the exception is not raised the user is not aware he may produce invalid JS code.

var foo = escapeJs(null);

Uncaught SyntaxError: Unexpected token ;

So, the method should throw exception or return the PHP string "null"

The same reasoning is applicable to CSS. If you try to escape an invalid value then exception should be thrown.

Finally, the idea behind of different escape strategies its language require his own set of rules so I don't think any normalization (in the output) its possible. The only normalization could be in the input.

[Ocramius]

I think the exception raised on escapeJs has sense because if someone want to print null as JS output the function should be or return the PHP string "null" or warn the user he/she must check if the value is null and print the null value itself.

A couple of things before we go anywhere:

• this would be a huge BC break
• the escapers (All of them) are supposed to work with string-alike values, with string-alike following PHP's semantics

Suggestion:

• verify that input value is either a string, an int, a float, a double or an object with __toString, then cast

• fail with an exception in all other cases

  So, the method should throw exception or return the PHP string "null"

Exception please: this would otherwise be a "WTF" from a user perspective

[bcremer]

the escapers (All of them) are supposed to work with string-alike values, with string-alike following PHP's semantics

Wouldn't that imply also casting null/false to '' and true to '1'?

verify that input value is either a string, an int, a float, a double or an object with __toString, then cast

That seems to be overly strict to me given the loosely typed nature of PHP:

protected function isCastableToString($string)
{
    return is_string($string)
        || is_int($string)
        || is_float($string)
        || is_double($string)
        || (is_object($string) && method_exists($string, '__toString'));
}

I would rather keep the existing null, true and false behavior of escapeHtml() and escapeUrl() to minimize the BC break:

protected function isCastableToString($string)
{
    return is_scalar($string) || ($string) && method_exists($string, '__toString'));
}

[Ocramius]

Wouldn't that imply also casting null/false to '' and true to '1'?

Agree

That seems to be overly strict to me given the loosely typed nature of PHP:

This is a security-sensitive component: make it fail a lot, and eagerly. Anyway, more strictness reduces space for misinterpretation as well, and is pretty much always welcome.

[weierophinney]

Closing. As noted, we cannot accept this as-is due to the huge BC and security implications. @Ocramius has given copious notes on approaches we would consider, if anybody would like to take this up again.