Opened 9 years ago
Last modified 4 years ago
#34405 reviewing defect (bug)
Retrieval of meta value that is an array.
Reported by: | alexwbaumann | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 4.3.1 |
Component: | Options, Meta APIs | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
Simply using $post->meta_value automatically runs get_post_meta(). This happens in https://core.trac.wordpress.org/browser/tags/4.3.1/src/wp-includes/post.php#L748 (post.php line 748).
That value then goes into sanitize_post_field() https://core.trac.wordpress.org/browser/tags/4.3.1/src/wp-includes/post.php#L751 (post.php line 751). The value then gets modified at https://core.trac.wordpress.org/browser/tags/4.3.1/src/wp-includes/post.php#L2215 (post.php line 2251) through esc_attr().
Codex entry for https://codex.wordpress.org/Function_Reference/esc_attr (esc_attr()) shows that the only parameter that should be sent is a string.
This process works for strings. However, if our meta value is an array, we get a PHP notice (Notice: Array to string conversion in /var/www/html/wp/wp-includes/formatting.php on line 959). Then returns a sting(5) 'Array' instead of the actual array.
I think that the best option may be to edit esc_attr() or the subsequent function wp_check_invalid_utf8() to allow for arrays to be passed to them. Alternatively the quick dirty fix would be to not send an array into sanitize_post_field.
Attachments (4)
Change History (10)
#3
@
9 years ago
This is my first time attempting to do a unit test. I was having trouble with the original. After submitting, I soon found why it was not working for me as expected.
I think that the new file is what you are looking for. If not, let me know if you need anything else and I'll do my best.
Thanks for the detailed ticket, alexwbaumann, and welcome to Trac.
I've always thought that
$post->foo
to get arbitrary 'foo' metadata from the database is a pretty weird syntax, but that's the way it works, so oh well :)Sanitizing a non-scalar value for the 'edit' context feels odd to me. WP uses the 'edit' context when pulling up a post to display on
wp-admin/post.php
, in which case the values are being sanitized for entry into textareas and input fields. But this is clearly not the case for objects and arrays, soesc_attr()
doesn't seem like the right kind of sanitization. In the case of 'edit', it's not clear to me that we should be doing any sanitization at all - who knows how this data is intended to be used?That said, I don't think
esc_attr()
will be harmful here - anyone using post data in this way ought to know what they're doing. Since postmeta can be in any non-scalar format, including objects and multi-dimensional arrays, I recommend a more general solution:As SergeyBiryukov notes, this needs unit tests.