absint to return 0 for values with non-numeric characters

[sc0ttkclark]

Currently, if you put in a string into absint which contains non-numeric characters after numeric characters it will return the number before the non-numeric characters as the number, which I believe isn't the valid return for absint($maybeint)

The solution I am proposing is to check if $maybeint isn't numeric, and if not then return 0;

See the attached patch.

[sc0ttkclark]

Oops, sorry for the tab difference, my editor is set to use spaces vs tabs.

[aaroncampbell]

We talked a little on IRC. What he's trying to fix is that absint( '123abc' ) returns 123 but absint( 'abc123' ) returns 0.

His original issue came from the fact that ​http://wordpress.org/news/?p=100 is the same as ​http://wordpress.org/news/?p=100test but not the same as ​http://wordpress.org/news/?p=test100

[scribu]

I always assumed that the definition for absint was return abs( (int) $maybeint ) );

Apparently not.

[scribu]

The original commit for absint() also mentioned casting: [6222]

[sc0ttkclark]

@scribu - It is, I just believe that if $maybeint contains non-numeric characters it should return zero to avoid canonical issues with the values it deals with. Otherwise, a significant number of absint usages in WP Core would need additional checks to prevent the usage of non-numeric characters in their values.

[scribu]

Yeah, it seems intval() is equivalent to using (int), so nevermind.

Note that ​is_numeric() will also match floats and other representations.

[sc0ttkclark]

Went the direction if is_numeric vs is_int so that it would avoid issues with strings not being handles as numbers (is_int requires an integer)

[scribu]

Yeah, there's no built-in function for strict casting. Might have to resort to a simple regex, since most cases where absint() is used is for validating a database ID.

[duck_]

The logic of your if statement appears to be the wrong way around, ! is_numeric surely.

[sc0ttkclark]

Absolutely correct @duck_, My wife and I just had a baby a week ago and have suffered the resulting #fail on a simple piece of code :)

Patching now..

[westi]

I don't see a valid reason for changing this.

In fact I see more valid reasons for keeping what we have.

I would much rather that: ​http://wordpress.org/news/?p=100test still worked than didn't work and I don't want to possibly break a whole load of plugin / theme code just to chage this function to work how you think it should work rather than how it works.

PHP is by design a loosely typed language and WP itself lives within this loosely typed world.

The whole point of absint is to make sure that the thing you are treating as an integer really is treated as an integer not a string so that when you later use this value you get what you expect.

[sc0ttkclark]

@westi - Why would you want ?p=100test to work? And what plugin / theme code would break from this? If anyone can give me an example, I'd love to see it because I just don't think there are any. Maybe that's me being naive, but I haven't heard anyone provide any use-cases for absint usage within core or plugins/themes which the result of absint should allow for non-numeric characters.

[westi]

Replying to sc0ttkclark:

@westi - Why would you want ?p=100test to work?

Because urls end up getting bunched up against other text by accident in comments, emails etc.

[nacin]

Strong +1 for the wontfix.

Imagine a ?p= URL that was made clickable, but the trailing whitespace was lost. ​http://wordpress.org/news/?p=100and would work. So would http://wordpress.org/news/?p=100), in systems where the ) becomes clickable.

That's just one specific instance. I am also for wontfix on the theory basis. absint() doesn't perform validation, it performs sanitization. We're using core PHP functions here, so this is expected behavior. I see no reason to modify this.

[nacin]

A change like this has huge potential cost, and offers no practical benefit.

[aaroncampbell]

That's the way I was leading when talking to him in IRC as well. I think absint() is just used in too many places to make a change like this.

I'd instead recommend that if a site wants to handle things differently they grab p and filter it before it gets processed normally.

[sc0ttkclark]

Gotcha, I see what you mean here though I believe ​http://wordpress.org/news/?p=100and itself is a bug and should be redirected to normalize 'p' in the query (and other places). absint hides issues like this, which is what I'm against.

I'll let it go, will put my own solution together for usage in my work.

[sc0ttkclark]

Was thinking more (one more try), perhaps absint could have an additional variable, so it'd be like:

function absint ($maybeint, $strict = false) {
    if ( $strict && ! is_numeric ( trim ( $maybeint ) ) ) {
        return 0;
    }
    return abs( intval( $maybeint ) );
}

This seems like a perfect solution, which wouldn't run the is_numeric / trim unless told to do so by the usage of absint($value,true);

[nacin]

At that point, why even use absint()? Or only use absint() after using is_numeric(). Core won't use this, and you're not required to leverage absint.

[sc0ttkclark]

You're the boss

[scribu]

Replying to sc0ttkclark:

Gotcha, I see what you mean here though I believe ​http://wordpress.org/news/?p=100and itself is a bug and should be redirected to normalize 'p' in the query (and other places).

I'm pretty sure there's a canonical redirect in place for this case. If not, there should be.

[sc0ttkclark]

Replying to scribu:

Replying to sc0ttkclark:

Gotcha, I see what you mean here though I believe ​http://wordpress.org/news/?p=100and itself is a bug and should be redirected to normalize 'p' in the query (and other places).

I'm pretty sure there's a canonical redirect in place for this case. If not, there should be.

Only redirects if you are using non-p permalinks. Like in the case of this site:

pcgamingcorner.com/wordpress/?p=820and