Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add get_the_ID() & remove post_class() from the safe list #758

Merged
merged 1 commit into from Dec 30, 2016

Conversation

grappler
Copy link
Member

Fixes #746

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to add get_the_ID(). We know it will always be a numeric value or false, so using that in an echo should be fine.

Not so sure about the get_post_class() function.
First of all, the function returns an array and shouldn't be echo-ed as is (not without a join/explode or something).
Second of all, the function has a filter just before the array is returned and could therefore be used for evil.
Considering that, I believe the post_class() function should actually be removed from the safe functions list until escaping of the array values is added to the function in core.
(The escaping in core is done before the filter, so it would just be a case of changing the code order)

Refs:
https://developer.wordpress.org/reference/functions/get_the_id/
https://developer.wordpress.org/reference/functions/the_id/
https://developer.wordpress.org/reference/functions/get_post_class/
https://developer.wordpress.org/reference/functions/post_class/

@jrfnl
Copy link
Member

jrfnl commented Dec 29, 2016

Related:
https://core.trac.wordpress.org/ticket/29259
https://core.trac.wordpress.org/ticket/20009 (discussed and closed without fixing it for fear of breaking funky plugin implementations)

@JDGrimes
Copy link
Contributor

I second the concerns that @jrfnl has raised in regard to get_post_class(). Of course, even esc_html() et al. apply filters after escaping the value. But in that case, anything hooked to the filter knows that the value is already escaped. It is entirely possible that folks could hook into the post class filter and not realize that escaping is needed on their part.

@grappler grappler changed the title Add get_post_class() & get_the_ID() to safe list Add get_the_ID() & remove post_class() from the safe list Dec 30, 2016
@grappler
Copy link
Member Author

I have update the PR and changed the title to match the code.

@JDGrimes JDGrimes added this to the 0.11.0 milestone Dec 30, 2016
@JDGrimes JDGrimes merged commit 3930b40 into WordPress:develop Dec 30, 2016
@grappler grappler deleted the feature/safe-functions branch December 31, 2016 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants