scribu / wp-scb-framework

Utilities for speeding up WordPress plugin and theme development
https://github.com/scribu/wp-scb-framework/wiki
220 stars 59 forks source link

Use `wp_kses_data` for validating text fields #30

Closed meloniq closed 11 years ago

meloniq commented 11 years ago

Use wp_kses_data over wp_filter_kses for validating text fields - does the same, but will not slash escape the data.

Issue: scbForms::validate_post_data() strips the slashes from posted values, but in scbTextField::validate() they are added back by wp_filter_kses.

scribu commented 11 years ago

Does this work correctly in both in WP 3.5 and 3.6?

meloniq commented 11 years ago

I think so, wp_kses_data was introduced in WP 2.9

meloniq commented 11 years ago

Related/Duplicate: https://github.com/scribu/wp-posts-to-posts/issues/276

scribu commented 11 years ago

I think so, wp_kses_data was introduced in WP 2.9

That's not what I was asking. In the P2P ticket, I mentioned http://core.trac.wordpress.org/ticket/21767

meloniq commented 11 years ago

Will look closer at the mentioned ticket, test it on 3.6, and back with results...

meloniq commented 11 years ago

Didn't noticed anything strange on WP 3.6

Mentioned ticket has changed its direction ~2 weeks later after you have referenced it, 'breakable' changes has been reverted to WP 3.5 behavior.

scribu commented 11 years ago

I don't know; P2P 1.6.1 handles slashes correctly with scbFramework as is.

Besides, you can specify which function scbForms should use by passing 'validate' => 'some_callback'.

scribu commented 11 years ago

Sorry, I meant 'sanitize' => 'some_callback'

tylerhcarter commented 11 years ago

Core is making a move away from slash escaping data. I believe that it would be wise to make the same move for the sake of consistency.

scribu commented 11 years ago

Core is making a move away from slash escaping data.

You're going to have to be a bit more precise on that. :)

I haven't been following WP 3.6 development too closely, so a link to a commit in Core would be useful.

tylerhcarter commented 11 years ago

This is the place where they replaced wp_filter_kses with wp_kses_data. http://core.trac.wordpress.org/ticket/21767

scribu commented 11 years ago

That ticket has 120 comments.

The initial commit that mentions wp_kses_data: http://core.trac.wordpress.org/changeset/23416

Which was then reverted in http://core.trac.wordpress.org/changeset/23554

So, you'll have to be even more specific.

tylerhcarter commented 11 years ago

It actually doesn't have anything to do with that. wp_kses_data expects data to not be slashed, and wp_filter_kses expects data to be slashed, per documentation.

Because you stripslashes in validate_post_data, you have unslashed data. Passing that to wp_filter_kses which expects slashed data doesn't seem correct. wp_kses_data, which accepts unslashed data, seems more appropriate.

scribu commented 11 years ago

wp_filter_kses which expects slashed data

I'm pretty sure it expects unslashed data; here's the source (from WP 3.6-beta3-24385):

function wp_filter_kses( $data ) {
    return addslashes( wp_kses( stripslashes( $data ), current_filter() ) );
}
scribu commented 11 years ago

Oh, I see. It first strips slashes, then adds them back.

scribu commented 11 years ago

Alright, tested in a bunch of scenarios; seems fine. Thanks for bringing it up.

meloniq commented 11 years ago

Thank you both for reviewing!