`wp_unslash()` and `wp_slash()` do not (un)slash the same data.

[peterwilsoncc]

The wp_unslash() and wp_slash() functions do not do exactly the opposite thing, ie running $data = wp_unslash( wp_slash( $data ) ) may modify the data.

The issue occurs with the treatment of objects, wp_unslash() uses map_deep() and therefore accounts for objects; whereas wp_slash() does not traverse objects.

<?php
/**
 * @ticket 56119
 */
public function test_slash_unslash_reversable() {
        $expected = (object) array( 'data' => 'sl\sh' );
        $actual = wp_unslash( wp_slash( clone( $expected ) ) );

        $this->assertSame( $expected->data, $actual->data );
}

/**
 * @ticket 56119
 */
public function test_unslash_slash_reversable() {
        $expected = (object) array( 'data' => 'sl\sh' );
        $actual = wp_slash( wp_unslash( clone( $expected ) ) );

        $this->assertSame( $expected->data, $actual->data );
}

[costdev]

It seems that using the following from the deprecated wp_slash_strings_only ​Ref does the trick here:

<?php
function wp_slash( $value ) {
        return map_deep( $value, 'addslashes_strings_only' );
}

addslashes_strings_only() ​Ref is also deprecated, but this might mean it's a candidate for undeprecating?

I modified the tests to use slashable characters, as wp_slash() has no way to know that the second "s" should be slashed in slsh:

<?php
/**
 * @ticket 56119
 */
public function test_slash_unslash_reversable() {
        $expected = (object) array( 'data' => 'sl"sh' );
        $actual   = wp_unslash( wp_slash( clone( $expected ) ) );

        $this->assertSame( $expected->data, $actual->data );
}

/**
 * @ticket 56119
 */
public function test_unslash_slash_reversable() {
        $expected = (object) array( 'data' => 'sl\"sh' );
        $actual   = wp_slash( wp_unslash( clone( $expected ) ) );

        $this->assertSame( $expected->data, $actual->data );
}

What do you think @peterwilsoncc?

[johnbillion]

It's worth noting that the parameter type of both wp_slash() and wp_unslash() is documented as string|array even though the latter handles objects via its use of map_deep().

[SergeyBiryukov]

I think wp_slash_strings_only() was deprecated because it appeared to be the same as wp_slash() as of [48433], though the difference in object handling was probably not considered.

Good catch on the inconsistency in object handling! Looks like that was the case ever since wp_slash() and wp_unslash() were both introduced in [23555].

Looking at #21767, it seems that:

• wp_slash() was introduced as a replacement for addslashes() and add_magic_quotes().
• wp_unslash() was introduced as a replacement for stripslashes() and an alias for stripslashes_deep().
• wp_unslash() intentionally unslashes objects: comment:53:ticket:21767.

I guess the question to answer would be: do we need to slash objects in wp_slash(), or would that be unexpected? This comment seems tangentially related: comment:17:ticket:21767, and this commit too: [24731].

I think wp_slash() and wp_unslash() may not necessarily have been intended to be the opposite, but that would probably make sense if there are no unexpected site effects.