Skip to content
This repository has been archived by the owner on Sep 24, 2018. It is now read-only.

Change the post_title database sanitization function towp_filter_kses() #2788

Open
rachelbaker opened this issue Oct 10, 2016 · 2 comments
Open

Comments

@rachelbaker
Copy link
Member

We should match WordPress Core's sanitization function when adding/updating the title of a Post. WordPress Core uses wp_filter_kses() but we use wp_filter_post_kses().

@westonruter
Copy link
Member

I recommend against using wp_filter_post_kses() and wp_filter_kses() because they eat slashes. Consider:

echo wp_filter_kses( '\o/' ); // => "o/"
echo wp_filter_post_kses( '\o/' ); // => "o/"

The core function seems to assumes pre-slashed data but in my experience even with slashing it can be lossy. And since $request isn't pre-slashed, I think instead of calling wp_filter_kses() I suggest calling the the underlying code without the slashing:

$sanitized_post_title = wp_kses( $data, current_filter() );

I think current_filter() would be title_save_pre.

kadamwhite added a commit that referenced this issue Oct 15, 2016
May fix #2788

This PR switches from using `wp_filter_post_kses()` for `post_title`
sanitization to calling `wp_kses` directly: #2788 describes that the
core behavior is to call `wp_filter_kses()`, but @westonruter notes
in that thread that the slash handling in `wp_filter_kses` is lossy so
using the underlying implementation of `wp_filter_kses` without the
de-slashing and re-slashing should provide adequate sanitization without
compromising the integrity of the content.

Review requested especially from @rachelbaker or @westonruter
@kadamwhite
Copy link
Contributor

In progress in #2840 but backslashes are still being removed after the proposed change. We need to dig deeper into how core is sanitizing these fields so we can better replicate that behavior.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging a pull request may close this issue.

3 participants