-
Notifications
You must be signed in to change notification settings - Fork 3.1k
HTML API: Use strict in_array comparison for checking URI attributes #7196
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
HTML API: Use strict in_array comparison for checking URI attributes #7196
Conversation
@@ -3667,7 +3667,7 @@ public function set_attribute( $name, $value ): bool { | |||
* | |||
* @see https://html.spec.whatwg.org/#attributes-3 | |||
*/ | |||
$escaped_new_value = in_array( $comparable_name, wp_kses_uri_attributes() ) ? esc_url( $value ) : esc_attr( $value ); | |||
$escaped_new_value = in_array( $comparable_name, wp_kses_uri_attributes(), true ) ? esc_url( $value ) : esc_attr( $value ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xknown, @aaronjorbin, @audrasjb This was added in https://core.trac.wordpress.org/changeset/58473 where you're propped. Can you confirm whether this loose in_array
was intentional?
It seems like we want strict as we expect to be comparing strings, but I want to confirm first.
If we'd prefer to keep the loose comparison that's fine, I'd like to apply a different change because PHPCS is flagging this line as a warning now.
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Would definitely be best to ensure that a filter for wp_kses_uri_attribute()
doesn't introduce a value that casts to some string equivalent.
This patch modifies the URL-escaping code in the HTML API to rely on strict comparisons. This prevents accidental matching via type-coercion. Developed in #7196 Follow-up to [58473]. Props jonsurrell. git-svn-id: https://develop.svn.wordpress.org/trunk@58897 602fd350-edb4-49c9-b593-d223f7449a82
This patch modifies the URL-escaping code in the HTML API to rely on strict comparisons. This prevents accidental matching via type-coercion. Developed in WordPress/wordpress-develop#7196 Follow-up to [58473]. Props jonsurrell. Built from https://develop.svn.wordpress.org/trunk@58897 git-svn-id: http://core.svn.wordpress.org/trunk@58293 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This patch modifies the URL-escaping code in the HTML API to rely on strict comparisons. This prevents accidental matching via type-coercion. Developed in WordPress/wordpress-develop#7196 Follow-up to [58473]. Props jonsurrell. Built from https://develop.svn.wordpress.org/trunk@58897 git-svn-id: https://core.svn.wordpress.org/trunk@58293 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This patch modifies the URL-escaping code in the HTML API to rely on strict comparisons. This prevents accidental matching via type-coercion. Developed in WordPress#7196 Follow-up to [58473]. Props jonsurrell. git-svn-id: https://develop.svn.wordpress.org/trunk@58897 602fd350-edb4-49c9-b593-d223f7449a82
Add a strict argument to an
in_array
check that seems to be comparing strings with no need for loose comparison.Follow-up to [58473].
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.