[RFC] Safe Casting Functions

Hi!

Wait, we actually already have FILTER_VALIDATE_INT and
FILTER_VALIDATE_FLOAT,

Actually, to_int is very close to FILTER_VALIDATE_INT, and I’m not
sure, but I think to_float may be close to FILTER_VALIDATE_FLOAT. The
main difference with integers is rejection of whitespace and
toString-able objects.

So essentially we have a number of rules, which all are only slightly
different. And when somebody wants to skip spaces, but not tabs, we'd
have yet another set of functions? I don't think it's good for the
language to have a set of functions doing exactly the same, but in
slightly different way, because some people had different preferences.
The language should not be just a bag of use-cases that somebody wanted
to implement. We're getting a lot of criticism for parts of the language
not always playing cohesively together, why make it worse?

The main point of the RFC is to add casting functions as an
alternative to the built-in explicit casts. Currently, the easiest
way to convert to an integer is (int), but this is quite dangerous as
it performs no validation whatsoever and cannot fail. to_int() and

So, why not have filter_var($suspected_int, FILTER_VALIDATE_INT) and
filter_var($suspected_int, FILTER_CONVERT_INT)? We already have
infrastructure for that, why ignore it completely and build another
solution that does exactly the same but treats whitespace differently
and has couple of other tweaks? OK, you want to treat the whitespace
differently, I get it - but why ignore whole filter infrastructure?

the like are intended to be just as convenient as an explicit cast,
so that doing the safer thing (failing on garbage input) is not any
more difficult. The hope is that the lazy developer will use is_int()
instead of (int) when they need to explicitly cast, and avoid the
problems of the latter.

The lazy developer won't check the return value anyway and would get 0
as the result of false-to-int conversion, thus making the whole exercise
pointless anyway :)

Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/

Throwing an exception or even returning NULL seems so much better than returning "false" -- "false" is a boolean, not an error, and despite some historical cases of PHP using "false" as a poor person's error code, it really isn’t.

Why isn’t FALSE an error value? NULL signifies an absence of a value, not a bad value. We use FALSE in a lot of places to indicate an error. Heck, filter_var uses it for errors here.

FALSE is not an error value because it is already something else. It's already a value, it's boolean false. The fact that you couldn't build a reasonable to_bool even if you wanted to, as I as well as others pointed out on this thread, indicates the very real problem with the conflation of purposes here. Derick Rethans said it pretty well:

Then to_bool() whould return false... or true? So hence, it should be
NULL, and that would also be consistent with ext/filter.

You also said:

Also, that we couldn't introduce a meaningful to_bool function, even if we decided we wanted it, indicates to me that returning false is the wrong thing to do.

It would be possible to use a special value in that specific case. Though, again, booleans have their own problems. I don’t see the point in to_bool().

Whether or not we actually should add to_bool is a separate issue; we, in principle, should be able to cleanly add it, since it's a very natural extension to what you've already designed here. The fact that, if we used FALSE for error that to_bool would have to be a special case, inconsistent with the rest, indicates to me that using FALSE is the wrong thing -- again, it's the conflation of the boolean value FALSE, which already has this meaning, with some error code. It shouldn't be conflated, FALSE should not be an error return, it means something else.

If you want error codes, use error codes, or use exceptions, but having two kinds of failure, null vs false, is really confusing.

What two kinds of error? We’re only using FALSE here.

But FALSE is a different kind of error as you're trying to use it here, different than NULL which is specifically designed to be an error / lack of value. NULL doesn't mean anything else, it's a specially-designated value. And if you don't want to use NULL for failure because you don't think this is quite the sort of failure that NULL expresses, then throw an exception -- FALSE is just totally the wrong thing to return.

• If strict type hinting were added, they would fail for a nullable typehint

Throwing an exception also addresses this, in a much cleaner way. If you're worried about strict typing, then returning "false" is even worse, since the return type of, for example, to_int is now "int | bool" as opposed to "nullable-int" or "int" (if you throw).

Exceptions make chaining more difficult. There’s also no precedent for using them.

I think the argument about chaining was addressed by someone else. With precedent, just because there's no precedent doesn't mean we can't start now :) They really feel like the right thing to do here and elsewhere, and we have to start somewhere.

• `FALSE` is a traditional error value in PHP

Since this is a new function, one that doesn't interoperate in any complicated way with the existing library or affect BC, this doesn't seem that important. IMO a language should have one failure/absense-of-value, in most cases "null", and having a weird second case seems, well, weird.

Failure and absence of value are different things and should not use the same value. Otherwise, you are liable to confuse missing data and errors.

But if you want to make that distinction, then you shouldn't use a value that means something else to indicate failure. You should throw an exception. Returning FALSE is liable to replace confusion between missing data and errors with confusion between some other data (i.e., the boolean FALSE) and errors.

I might also argue that missing the data makes it clear there was an error, and then if someone has a nullable typehint they expressly said they accept a lack-of-int, but an exception seems a much cleaner way to deal with this anyways.

If you have more interesting failure cases, just throw an exception, or return null if you want, don't continue propagating a weird second kind of null (that isn't actually null, it's a boolean false).

It’s not a “weird second kind of null”, it’s a value FALSE.

Which is being used as a weird second kind of failure, as opposed to NULL which is the typical failure lack-of-value, as I argue above.

It's also interesting to look at how other languages handle failures of this kind. Most imperative languages I know of either throw or return null; in particular, Python, which has fairly similar type system to PHP in a lot of ways, throws. Functional languages do more interesting things; Haskell does something complicated but morally equivalent to returning null. But I'm not aware of any language which specifically returns false.

JavaScript returns NaN here and C sets errno to an error value. PHP uses FALSE, in some respects, like JavaScript uses NaN.

But it probably shouldn't -- NaN is a special number which is not a number (and which would probably pass a float type annotation, no?). FALSE isn't a special integer which isn't an integer -- it is already something else, it's a boolean. JS has a few special values which are specifically designated as different failures or lack-of-values -- undefined, null, NaN, etc. PHP's FALSE does not fall into that category -- it does mean something, it means something else, the boolean value false. (FWIW, I don't think that the way JS does this feels quite right either, I don't like that there are multiple sorts of non-exception failure values, but at least they return a designated failure value that doesn't have another totally different non-error meaning.)

Josh Watzman

some notes:

• it's probably make sense to implement these function as a new
  opcode(s) in VM

That could be an optimisation later, yes. I note that you’ve added
function replacement with opcodes for certain commonly-used functions. We
could make these functions use that. Then they’d still be usable as
callbacks.

I may help with implementation, if RFC accepted.
(It would be better if ZEND_STRLEN, ZEND_TYPE_CHECK, etc were introduced in
first place instead of stlen(), is_long() and family).

• in case of conversion failure it's better to throw exception (to_int()
  returning FALSE is a pain).

I am gradually warming to throwing an exception. An interesting idea I’ve
had suggested on Twitter was by Matt Parker, who suggested that I add an
optional 2nd argument. Without the argument, it throws an exception. With
an argument, it returns that value (as a default) instead of throwing an
exception.

Together with https://wiki.php.net/rfc/engine_exceptions_for_php7 throwing
exceptions would be the best choice. I would make it permanent, (without
optional areguments)

Thanks. Dmitry.

--
Andrea Faulds
http://ajf.me/

I am gradually warming to throwing an exception. An interesting idea I’ve had suggested on Twitter was by Matt Parker, who suggested that I add an optional 2nd argument. Without the argument, it throws an exception. With an argument, it returns that value (as a default) instead of throwing an exception.

Now that sounds like a nice compromise in a few places where exceptions
have been introduced, but I still think this is still a little chicken
and egg. If it is going to 'fail' then the reason for the failure is
more important than simply producing no answer. Especially if it is
going to fail in different inputs to other methods.

Again forcing an exception only solution sidesteps that debate!

--
Lester Caine - G8HFL

Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

So, what exactly changes here if we have a second parameter or just return null by default?
It doesn’t make any difference, it’s just another way to write it:

to_int($a, $default)
or
to_int($a) ?? $default

Also, if you want exceptions, you always can wrap a userland function around it — but I’d rather not wrap an userland function around something throwing an exception… inefficient and weird.

Thanks,
Bob

Am 22.10.2014 um 12:27 schrieb Dmitry Stogov dmitry@zend.com:

"null" or "false" return value would make these functions not really
useful, because they won't guarantee to return desired type.

printf("%d\n", to_int("abcd")); // will print 0

The only reliable option to support wrong input is exceptions.
On the other hand, exceptions maybe difficult to use or inefficient.
We may avoid exceptions throwing, if provide a default value:

function to_int(mixed $a , int $default_value = null): int;
function to_double(mixed $a , double $default_value = null): double;
function to_string(mixed $a, string $default_value = null): string;

Thanks. Dmitry.

I know we have that already discussed a lot now, but I’d like to expose my
points on the return value here:

I imagine code like (supposing that we ever will have scalar typehints):

function acceptsInt (int $i = null) {
if ($i === null) {
$i = 2 /* default value /;
}
/ do something with $i */
}

When we return false:
acceptInt(($tmp = to_int($_GET["userinput"])) === false ? null : $tmp);

When we throw an exception:
try {
acceptInt(to_int($_GET["userinput"]));
} catch (CastingException $e) {
acceptInt(null);
}

When we just return null:
acceptInt(to_int($_GET["userinput"]));

Also, when we want to pass a default value defined outside of the
function, it’s a lot easier now with the coalesce operator:
acceptInt(to_int($_GET["userinput“]) ?? 2 /* default value */);

Also, independently of possible scalar typehints:

Generally exceptions are also a bad idea as the casts probably will be
used on external input and exceptions are not a way to handle malformed
user input. Really not.
Furthermore, false is a bad idea in the same sense (if we get scalar type
hints once), because people then might just catch the EngineException…

Also, null means "no value"; that’s exactly what we need. If the
to_{type}() functions cannot return a meaningful value, just return "no
value", that means null. And not false, which is a real value.

That’s why I strongly feel that null is the only true thing to return here.

Thanks,
Bob

Am 21.10.2014 um 00:57 schrieb Andrea Faulds ajf@ajf.me:

Good evening,

I am presenting a new RFC to add a set of three functions to do
validated casts for scalar types:

https://wiki.php.net/rfc/safe_cast

Please read it.

Thanks!

Andrea Faulds
http://ajf.me/

Trying to think what real world example may look like. With exceptions:

try {
$i = lossless_int($sth);
} catch (SomeException $e) {
// error out / provide default / custom error handling
}

If we use FALSE:

$i = lossless_int($sth);
If ($i===false) {
// error out / provide default / custom error handling
}

At first glance they look mostly equivalent, but it really depends on the
use case people will use it for. For instance, if you use it for input
validation for several input fields, the try/catch approach allows you to
put several conversions in one block, whereas the false approach doesn't,
and requires separate checks for every input field. There's still the
question on whether or not the exception would provide enough context for a
meaningful error message or not.

I think a key missing piece in the RFC is an explanation for the assertion
that the current type conversion system "[] makes it difficult to write
robust applications which handle user data.", and more importantly, how
these new functions will fix it. It focuses on how what these functions
would behave, but not so much about how real world usage of these functions
would behave.

My assumption is that with handling user input being the main goal, in case
of failed conversion the likely handling would be erroring out in case of
'garbage' input, as it's impossible to safely determine what were the remote
user's intentions (if we were to 'guess' what he meant with some default,
I'd argue that the current conversion rules are as good a guess as any). If
that's the case, wouldn't it make more sense to change the semantics of this
new family of functions from functions that do the conversion, to ones that
just check whether it can be done?

Something like:

If (!int_convertible($sth)) { // open to new ideas about the name :)
// error out
}
$i = (int) $sth;

Or even safe_to_conver($sth, 'int')) (i.e. one function that can handle all
types)?

Zeev

-----Original Message-----
From: Dmitry Stogov [mailto:dmitry@zend.com]
Sent: Wednesday, October 22, 2014 1:28 PM
To: Bob Weinand
Cc: Andrea Faulds; PHP Internals
Subject: Re: [PHP-DEV] [RFC] Safe Casting Functions

"null" or "false" return value would make these functions not really
useful,
because they won't guarantee to return desired type.

printf("%d\n", to_int("abcd")); // will print 0

The only reliable option to support wrong input is exceptions.
On the other hand, exceptions maybe difficult to use or inefficient.
We may avoid exceptions throwing, if provide a default value:

function to_int(mixed $a , int $default_value = null): int; function
to_double(mixed $a , double $default_value = null): double; function
to_string(mixed $a, string $default-value = null): string;

Thanks. Dmitry.

On Wed, Oct 22, 2014 at 12:37 PM, Bob Weinand bobwei9@hotmail.com
wrote:

I know we have that already discussed a lot now, but I’d like to
expose my points on the return value here:

I imagine code like (supposing that we ever will have scalar typehints):

function acceptsInt (int $i = null) {
if ($i === null) {
$i = 2 /* default value /;
}
/ do something with $i */
}

When we return false:
acceptInt(($tmp = to_int($_GET["userinput"])) === false ? null :
$tmp);

When we throw an exception:
try {
acceptInt(to_int($_GET["userinput"]));
} catch (CastingException $e) {
acceptInt(null);
}

When we just return null:
acceptInt(to_int($_GET["userinput"]));

Also, when we want to pass a default value defined outside of the
function, it’s a lot easier now with the coalesce operator:
acceptInt(to_int($_GET["userinput“]) ?? 2 /* default value */);

Also, independently of possible scalar typehints:

Generally exceptions are also a bad idea as the casts probably will be
used on external input and exceptions are not a way to handle
malformed user input. Really not.
Furthermore, false is a bad idea in the same sense (if we get scalar
type hints once), because people then might just catch the
EngineException…

Also, null means "no value"; that’s exactly what we need. If the
to_{type}() functions cannot return a meaningful value, just return
"no value", that means null. And not false, which is a real value.

That’s why I strongly feel that null is the only true thing to return
here.

Thanks,
Bob

Am 21.10.2014 um 00:57 schrieb Andrea Faulds ajf@ajf.me:

Good evening,

I am presenting a new RFC to add a set of three functions to do
validated casts for scalar types:

https://wiki.php.net/rfc/safe_cast

Please read it.

Thanks!

Andrea Faulds
http://ajf.me/

--
To unsubscribe,
visit: http://www.php.net/unsub.php

Marc Bennewitz wrote on 22/10/2014 20:12:

I know we have that already discussed a lot now, but I’d like to expose my points on the return value here:

I imagine code like (supposing that we ever will have scalar typehints):

function acceptsInt (int $i = null) {
if ($i === null) {
$i = 2 /* default value /;
}
/ do something with $i */
}
NULL isn't a pointer for a default value - it's simply a type with no
value - no more - no less.
From your example: why do you accept NULL if you need a integer default
value?

function acceptsInt (int $i = 2) { ...

Those are two distinct types of default:

• the default value to use when a programmer calls the function with
  fewer parameters (e.g. acceptsInt())
• the default value to use when a value is provided by the programmer,
  but turns out to be null at runtime (e.g. acceptsInt($_GET['foo']))

You addresses data loss on convert float to int.
Do you also address data loss on int to float?

|to_float(||9223372036854774784) -> pass |as it results in
9223372036854774784
|to_float(|||9223372036854774785|) -> |fail as it results in
9223372036854774784

Floats aren’t expected to be precise, so I don’t see why this shouldn’t pass. It’s a loss of data, sure, but merely of precision, which is expected here. The reason I have to_int fail is because float overflow to int completely mangles your input.

Andrea Faulds
http://ajf.me/

Hi Andrea,

I would like to have DbC to harden app security as well.
I'm looking for something like D language.

http://dlang.org/contracts.html

With DbC, checking parameter types/range/etc happen only when
development. Therefore, app runs faster for production. All of runtime
checks cannot be removed from app code by DbC, so this RFC is nice to
have even with DbC.

Something like contracts is actually an idea I’ve had before. Even some
of the most expressive type systems (like Haskell’s) seem to neglect that
only a certain range of values of a primitive type may be desired. Having
something like that in PHP might be useful.

But I wouldn’t say it’s really on-topic.

--
Andrea Faulds
http://ajf.me/

while briefly looking through the conversion examples, i see some weird
results

string(5) “31e+7” - shouldn't this be valid for int?
string(4) “0x10” - hex, but that's int, no?
string(3) “010” - octal, but that's int, no?
string(4) “10.0” - this would be casting to 10, so int valid
object(Stringable)#2 (0) {} - and similar actually, what if _toString()
returns some int/float literal? that should pass as well, no?

Generally I'd say no to this RFC. The current casting is not perfect, but
as for me - the one suggested is highly questionable as well. IMO as long
as there are no proper strict types in PHP, any other rule set for casting
would be just another coordinate system for the same, which isn't worth
while at least.

Regards

Anatol

Hi Stas,

On Thu, Nov 20, 2014 at 10:28 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:

I like this RFC overall. Precise parameter checks is good for security
always.

I don't see how it matters for security at all. If you need an int,
(int) works as well as any proposed check, security-wise. You may want
different diagnostics, etc. but this doesn't have to do much with
security. In other words, if the security depends on any differences
between (int) and to_int, it's probably not done right.

Please refer to CWE/SANS TOP 25, Monster Mitigation especially.

http://cwe.mitre.org/top25/#Mitigations

and ISO 27000. (I cannot provide link to it, since one should buy the
document to read)

Programmer should control over all inputs as the most important security
measure.
There are two strategies in general.

• Convert inputs to secure values and ignore possible attacks.
  (Sanitization)
• Validate inputs to reject malformed values and record possible attacks.
  (Validation and logging)

(int) is sanitization. It works, but it cannot log/detect possible attack
(or bug).

to_int can be used as validation. It has advantage to record possible
attack (or bug). Logging is
one of important security feature. Therefore, validation could be said more
secure than sanitization.

Which strategy to adopt is that depends on organization/application policy.
Public web sites may ignore
invalid inputs due to large amount of attacks while private web sites may
require to record all
possible attacks (or bugs), for example.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net