stripslashes_deep() casts booleans to strings

[DemoIgor]

older
a:1:{s:13:"administrator";b:1;}
newer (PHP v5.3)
a:1:{s:13:"administrator";s:1:"1";}

Function stripslashes_deep() change bool "true" >to> string "1"

[knutsp]

What is the bug that needs to fixed? If this is some kind of enhancement, why do you want to do this?

[nacin]

I've noticed this. I think the report is that stripslashes_deep() is now moving (bool) true to (string) "1". I'd call that a bug.

[scribu]

Confirmed:

var_dump( stripslashes_deep( array( 'foo' => true, 'bar' => false ) ) );

// Result: array(2) { ["foo"]=> string(1) "1" ["bar"]=> string(0) "" }

[knutsp]

I see. That makes sense.

I found this comment on ​http://php.net/manual/en/function.stripslashes.php

---

2) both stripslashes and str_replace functions always return strings, so:

• TRUE will become a string "1",
• FALSE will become an empty string,
• integers and floats will become strings,
• NULL will become an empty string.

On the other hand you only need to process strings, so use the is_string function to check;

---

If so, then the function could be exptended a bit like this:

function stripslashes_deep($value) {
    $value = is_array($value) ?
        array_map('stripslashes_deep', $value) :
        (is_string($value)?
            stripslashes($value):
            $value);
 
    return $value;
 }

But I don't like the $value = $value so I would prefer using if blocks instead of nested ternal if's.

Test:

var_dump( stripslashes_deep( array( 'foo' => true, 'bar' => false, 'string' => 'str\ing', 'array' => array( 'foo1' => true, 'bar1' => false, 'arr1' => 'arr\1', 'arr2' => 'arr\2') ) ) );

Result:
array(4) { foo?=> bool(true) bar?=> bool(false) string?=> string(6) "string" array?=> array(4) { foo1?=> bool(true) bar1?=> bool(false) arr1?=> string(4) "arr1" arr2?=> string(4) "arr2" } }

[kawauso]

is_string() on stripslashes()

[knutsp]

Only stripslash strings (and arrays, objects)

[knutsp]

Ok, I'll do another easy one :-)

Lessons for a newbee:

• Always do a last check if a patch has already been uploded
• Name the .diff as the <ticket_number>.diff
• Remember the documentation @headers

Question: If I am working on a patch, even a small one, should I accept it or reassign it to me?

[SergeyBiryukov]

Replying to knutsp:

Question: If I am working on a patch, even a small one, should I accept it or reassign it to me?

According to Codex, it's up to you:

A WordPress developer (which could be you) decides to fix the bug. The developer may choose to take responsibility for the bug by clicking on the Accept ticket option near the bottom of the ticket, though this is not required. Then the developer figures out how to fix the bug, creates one or more patch files, and uploads them to Trac.

​http://codex.wordpress.org/Reporting_Bugs

Generally the owner of a ticket is either a person who supplies the patch or the one who oversees the ticket (i.e. a committer).

[coffee2code]

Re-verified bug. Tested 18026.diff​ and it still applies cleanly as of r20598.

Added unit tests (see ​[UT698]) for stripslashes_deep() in general, including a test reflecting this bug. Test relating to this bug passes after aforementioned patch is applied.

Setting version to 3.2 which was current as of the original bug submission (though the bug harkens back to the introduction of stripslashes_deep() in 2.0).

[nacin]

18026.diff​ and ​[UT698] both look good.

[nacin]

Since we use stripslashes_deep() quite a bit in core, I'm trying to think of the potential side effects. For one, an update_option() of two equal serialized values (that are no longer equal due to the change here) would force an update. I don't see this as a big problem, but something to keep in mind. A deploy to WP.com for instance might cause a spike in writes.

[westi]

Replying to nacin:

Since we use stripslashes_deep() quite a bit in core, I'm trying to think of the potential side effects. For one, an update_option() of two equal serialized values (that are no longer equal due to the change here) would force an update. I don't see this as a big problem, but something to keep in mind. A deploy to WP.com for instance might cause a spike in writes.

This patch looks good to me too.

The possibly wide reaching effect is a bit of a worry but no one should be updating options from most requests anyway so I don't think it would have too much of an effect.

[SergeyBiryukov]

Related: #20712

[nacin]

In [21292]:

Only run stripslashes() in stripslashes_deep() for strings, not other scalar values. props Kawauso, knutsp. props coffee2code for ​[UT698]. fixes #18026.