From cdb8b7df7aca8a54f19c81683fc3349ed76dfcd2 Mon Sep 17 00:00:00 2001 From: Matt Boynes Date: Tue, 10 Oct 2017 17:43:12 -0400 Subject: [PATCH 1/4] Cast scalar meta values as string Replaces #617 --- php/context/class-fieldmanager-context-post.php | 6 ++++++ php/context/class-fieldmanager-context-quickedit.php | 6 ++++++ php/context/class-fieldmanager-context-submenu.php | 6 ++++++ php/context/class-fieldmanager-context-term.php | 6 ++++++ php/context/class-fieldmanager-context-user.php | 6 ++++++ 5 files changed, 30 insertions(+) diff --git a/php/context/class-fieldmanager-context-post.php b/php/context/class-fieldmanager-context-post.php index 77609a5d72..1179a50112 100644 --- a/php/context/class-fieldmanager-context-post.php +++ b/php/context/class-fieldmanager-context-post.php @@ -310,6 +310,12 @@ protected function add_data( $post_id, $meta_key, $meta_value, $unique = false ) * Default empty. */ protected function update_data( $post_id, $meta_key, $meta_value, $data_prev_value = '' ) { + // Meta is always stored as a string, so if this is a scalar value, cast + // it as a string to ensure that `update_metadata()` is able to correctly + // compare the current value against the previous value. + if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { + $meta_value = strval( $meta_value ); + } return update_post_meta( $post_id, $meta_key, $meta_value, $data_prev_value ); } diff --git a/php/context/class-fieldmanager-context-quickedit.php b/php/context/class-fieldmanager-context-quickedit.php index 8923b8e475..73d5b7f46a 100644 --- a/php/context/class-fieldmanager-context-quickedit.php +++ b/php/context/class-fieldmanager-context-quickedit.php @@ -281,6 +281,12 @@ protected function add_data( $post_id, $meta_key, $meta_value, $unique = false ) * Default empty. */ protected function update_data( $post_id, $meta_key, $meta_value, $data_prev_value = '' ) { + // Meta is always stored as a string, so if this is a scalar value, cast + // it as a string to ensure that `update_metadata()` is able to correctly + // compare the current value against the previous value. + if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { + $meta_value = strval( $meta_value ); + } return update_post_meta( $post_id, $meta_key, $meta_value, $data_prev_value ); } diff --git a/php/context/class-fieldmanager-context-submenu.php b/php/context/class-fieldmanager-context-submenu.php index 46708b0ebc..8b63f909bc 100644 --- a/php/context/class-fieldmanager-context-submenu.php +++ b/php/context/class-fieldmanager-context-submenu.php @@ -239,6 +239,12 @@ protected function add_data( $data_id, $option_name, $option_value, $unique = fa * @return bool Option updated successfully. */ protected function update_data( $data_id, $option_name, $option_value, $option_prev_value = '' ) { + // Meta is always stored as a string, so if this is a scalar value, cast + // it as a string to ensure that `update_metadata()` is able to correctly + // compare the current value against the previous value. + if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { + $meta_value = strval( $meta_value ); + } return update_option( $option_name, $option_value ); } diff --git a/php/context/class-fieldmanager-context-term.php b/php/context/class-fieldmanager-context-term.php index f2081f829e..3681f1602e 100644 --- a/php/context/class-fieldmanager-context-term.php +++ b/php/context/class-fieldmanager-context-term.php @@ -384,6 +384,12 @@ protected function add_data( $term_id, $meta_key, $meta_value, $unique = false ) * @param bool $meta_prev_value The previous meta value. */ protected function update_data( $term_id, $meta_key, $meta_value, $meta_prev_value = '' ) { + // Meta is always stored as a string, so if this is a scalar value, cast + // it as a string to ensure that `update_metadata()` is able to correctly + // compare the current value against the previous value. + if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { + $meta_value = strval( $meta_value ); + } if ( $this->use_fm_meta ) { return fm_update_term_meta( $term_id, $this->current_taxonomy, $meta_key, $meta_value, $meta_prev_value ); } else { diff --git a/php/context/class-fieldmanager-context-user.php b/php/context/class-fieldmanager-context-user.php index 62081ce797..15c7d4496a 100644 --- a/php/context/class-fieldmanager-context-user.php +++ b/php/context/class-fieldmanager-context-user.php @@ -174,6 +174,12 @@ protected function add_data( $user_id, $meta_key, $meta_value, $unique = false ) * @param mixed $data_prev_value The previous meta data. */ protected function update_data( $user_id, $meta_key, $meta_value, $data_prev_value = '' ) { + // Meta is always stored as a string, so if this is a scalar value, cast + // it as a string to ensure that `update_metadata()` is able to correctly + // compare the current value against the previous value. + if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { + $meta_value = strval( $meta_value ); + } return call_user_func( /** * Filters function used to update user meta. This improves compatibility with From fc5af6395e033a686e227fe3b1a1b651704229ad Mon Sep 17 00:00:00 2001 From: Matt Boynes Date: Tue, 10 Oct 2017 18:19:34 -0400 Subject: [PATCH 2/4] Adhere to wp documentation standards --- php/context/class-fieldmanager-context-post.php | 8 +++++--- php/context/class-fieldmanager-context-quickedit.php | 8 +++++--- php/context/class-fieldmanager-context-term.php | 8 +++++--- php/context/class-fieldmanager-context-user.php | 8 +++++--- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/php/context/class-fieldmanager-context-post.php b/php/context/class-fieldmanager-context-post.php index 1179a50112..1f8969ead9 100644 --- a/php/context/class-fieldmanager-context-post.php +++ b/php/context/class-fieldmanager-context-post.php @@ -310,9 +310,11 @@ protected function add_data( $post_id, $meta_key, $meta_value, $unique = false ) * Default empty. */ protected function update_data( $post_id, $meta_key, $meta_value, $data_prev_value = '' ) { - // Meta is always stored as a string, so if this is a scalar value, cast - // it as a string to ensure that `update_metadata()` is able to correctly - // compare the current value against the previous value. + /* + * Meta is always stored as a string, so if this is a scalar value, cast + * it as a string to ensure that `update_metadata()` is able to correctly + * compare the current value against the previous value. + */ if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { $meta_value = strval( $meta_value ); } diff --git a/php/context/class-fieldmanager-context-quickedit.php b/php/context/class-fieldmanager-context-quickedit.php index 73d5b7f46a..8c48a4b9b8 100644 --- a/php/context/class-fieldmanager-context-quickedit.php +++ b/php/context/class-fieldmanager-context-quickedit.php @@ -281,9 +281,11 @@ protected function add_data( $post_id, $meta_key, $meta_value, $unique = false ) * Default empty. */ protected function update_data( $post_id, $meta_key, $meta_value, $data_prev_value = '' ) { - // Meta is always stored as a string, so if this is a scalar value, cast - // it as a string to ensure that `update_metadata()` is able to correctly - // compare the current value against the previous value. + /* + * Meta is always stored as a string, so if this is a scalar value, cast + * it as a string to ensure that `update_metadata()` is able to correctly + * compare the current value against the previous value. + */ if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { $meta_value = strval( $meta_value ); } diff --git a/php/context/class-fieldmanager-context-term.php b/php/context/class-fieldmanager-context-term.php index 3681f1602e..f57f93497e 100644 --- a/php/context/class-fieldmanager-context-term.php +++ b/php/context/class-fieldmanager-context-term.php @@ -384,9 +384,11 @@ protected function add_data( $term_id, $meta_key, $meta_value, $unique = false ) * @param bool $meta_prev_value The previous meta value. */ protected function update_data( $term_id, $meta_key, $meta_value, $meta_prev_value = '' ) { - // Meta is always stored as a string, so if this is a scalar value, cast - // it as a string to ensure that `update_metadata()` is able to correctly - // compare the current value against the previous value. + /* + * Meta is always stored as a string, so if this is a scalar value, cast + * it as a string to ensure that `update_metadata()` is able to correctly + * compare the current value against the previous value. + */ if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { $meta_value = strval( $meta_value ); } diff --git a/php/context/class-fieldmanager-context-user.php b/php/context/class-fieldmanager-context-user.php index 15c7d4496a..1eb473d0db 100644 --- a/php/context/class-fieldmanager-context-user.php +++ b/php/context/class-fieldmanager-context-user.php @@ -174,9 +174,11 @@ protected function add_data( $user_id, $meta_key, $meta_value, $unique = false ) * @param mixed $data_prev_value The previous meta data. */ protected function update_data( $user_id, $meta_key, $meta_value, $data_prev_value = '' ) { - // Meta is always stored as a string, so if this is a scalar value, cast - // it as a string to ensure that `update_metadata()` is able to correctly - // compare the current value against the previous value. + /* + * Meta is always stored as a string, so if this is a scalar value, cast + * it as a string to ensure that `update_metadata()` is able to correctly + * compare the current value against the previous value. + */ if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { $meta_value = strval( $meta_value ); } From 0e7650da3d354886fbdd8706f7a81fb7b98e05ed Mon Sep 17 00:00:00 2001 From: Matt Boynes Date: Tue, 10 Oct 2017 18:19:57 -0400 Subject: [PATCH 3/4] Don't refer to options as meta --- php/context/class-fieldmanager-context-submenu.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/php/context/class-fieldmanager-context-submenu.php b/php/context/class-fieldmanager-context-submenu.php index 8b63f909bc..b7e2c48449 100644 --- a/php/context/class-fieldmanager-context-submenu.php +++ b/php/context/class-fieldmanager-context-submenu.php @@ -239,11 +239,13 @@ protected function add_data( $data_id, $option_name, $option_value, $unique = fa * @return bool Option updated successfully. */ protected function update_data( $data_id, $option_name, $option_value, $option_prev_value = '' ) { - // Meta is always stored as a string, so if this is a scalar value, cast - // it as a string to ensure that `update_metadata()` is able to correctly - // compare the current value against the previous value. - if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { - $meta_value = strval( $meta_value ); + /* + * Options are always stored as strings, so if this is a scalar value, + * cast it as a string to ensure that `update_option()` is able to + * correctly compare the current value against the previous value. + */ + if ( is_scalar( $option_value ) && ! is_string( $option_value ) ) { + $option_value = strval( $option_value ); } return update_option( $option_name, $option_value ); } From 58ec5fde7da37266de23670b0caa6dde95d2c229 Mon Sep 17 00:00:00 2001 From: Matt Boynes Date: Tue, 10 Oct 2017 19:55:34 -0400 Subject: [PATCH 4/4] make scalar value sanitization DRY and testable --- .../class-fieldmanager-context-post.php | 9 +----- .../class-fieldmanager-context-quickedit.php | 9 +----- .../class-fieldmanager-context-storable.php | 17 ++++++++++ .../class-fieldmanager-context-submenu.php | 9 +----- .../class-fieldmanager-context-term.php | 9 +----- .../class-fieldmanager-context-user.php | 9 +----- .../test-fieldmanager-context-storable.php | 32 +++++++++++++++++++ 7 files changed, 54 insertions(+), 40 deletions(-) create mode 100644 tests/php/test-fieldmanager-context-storable.php diff --git a/php/context/class-fieldmanager-context-post.php b/php/context/class-fieldmanager-context-post.php index 1f8969ead9..7601feaa9d 100644 --- a/php/context/class-fieldmanager-context-post.php +++ b/php/context/class-fieldmanager-context-post.php @@ -310,14 +310,7 @@ protected function add_data( $post_id, $meta_key, $meta_value, $unique = false ) * Default empty. */ protected function update_data( $post_id, $meta_key, $meta_value, $data_prev_value = '' ) { - /* - * Meta is always stored as a string, so if this is a scalar value, cast - * it as a string to ensure that `update_metadata()` is able to correctly - * compare the current value against the previous value. - */ - if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { - $meta_value = strval( $meta_value ); - } + $meta_value = $this->sanitize_scalar_value( $meta_value ); return update_post_meta( $post_id, $meta_key, $meta_value, $data_prev_value ); } diff --git a/php/context/class-fieldmanager-context-quickedit.php b/php/context/class-fieldmanager-context-quickedit.php index 8c48a4b9b8..c5feec75bd 100644 --- a/php/context/class-fieldmanager-context-quickedit.php +++ b/php/context/class-fieldmanager-context-quickedit.php @@ -281,14 +281,7 @@ protected function add_data( $post_id, $meta_key, $meta_value, $unique = false ) * Default empty. */ protected function update_data( $post_id, $meta_key, $meta_value, $data_prev_value = '' ) { - /* - * Meta is always stored as a string, so if this is a scalar value, cast - * it as a string to ensure that `update_metadata()` is able to correctly - * compare the current value against the previous value. - */ - if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { - $meta_value = strval( $meta_value ); - } + $meta_value = $this->sanitize_scalar_value( $meta_value ); return update_post_meta( $post_id, $meta_key, $meta_value, $data_prev_value ); } diff --git a/php/context/class-fieldmanager-context-storable.php b/php/context/class-fieldmanager-context-storable.php index 331fa9f0ae..a37dca24f7 100644 --- a/php/context/class-fieldmanager-context-storable.php +++ b/php/context/class-fieldmanager-context-storable.php @@ -163,6 +163,23 @@ protected function load_walk_children( $field ) { } } + /** + * Meta and options are always stored as strings, so it's best to ensure + * that scalar values get cast as strings to ensure that `update_metadata()` + * and `update_option()` are able to correctly compare the current value + * against the previous value. + * + * @param mixed $value Value being stored. + * @return string|array If $value is scalar, a string is returned. Otherwise, + * $value returns untouched. + */ + public static function sanitize_scalar_value( $value ) { + if ( is_scalar( $value ) && ! is_string( $value ) ) { + return strval( $value ); + } + return $value; + } + /** * Method to get data from the context's storage engine. * diff --git a/php/context/class-fieldmanager-context-submenu.php b/php/context/class-fieldmanager-context-submenu.php index b7e2c48449..42e391d4b1 100644 --- a/php/context/class-fieldmanager-context-submenu.php +++ b/php/context/class-fieldmanager-context-submenu.php @@ -239,14 +239,7 @@ protected function add_data( $data_id, $option_name, $option_value, $unique = fa * @return bool Option updated successfully. */ protected function update_data( $data_id, $option_name, $option_value, $option_prev_value = '' ) { - /* - * Options are always stored as strings, so if this is a scalar value, - * cast it as a string to ensure that `update_option()` is able to - * correctly compare the current value against the previous value. - */ - if ( is_scalar( $option_value ) && ! is_string( $option_value ) ) { - $option_value = strval( $option_value ); - } + $option_value = $this->sanitize_scalar_value( $option_value ); return update_option( $option_name, $option_value ); } diff --git a/php/context/class-fieldmanager-context-term.php b/php/context/class-fieldmanager-context-term.php index f57f93497e..715ff2837f 100644 --- a/php/context/class-fieldmanager-context-term.php +++ b/php/context/class-fieldmanager-context-term.php @@ -384,14 +384,7 @@ protected function add_data( $term_id, $meta_key, $meta_value, $unique = false ) * @param bool $meta_prev_value The previous meta value. */ protected function update_data( $term_id, $meta_key, $meta_value, $meta_prev_value = '' ) { - /* - * Meta is always stored as a string, so if this is a scalar value, cast - * it as a string to ensure that `update_metadata()` is able to correctly - * compare the current value against the previous value. - */ - if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { - $meta_value = strval( $meta_value ); - } + $meta_value = $this->sanitize_scalar_value( $meta_value ); if ( $this->use_fm_meta ) { return fm_update_term_meta( $term_id, $this->current_taxonomy, $meta_key, $meta_value, $meta_prev_value ); } else { diff --git a/php/context/class-fieldmanager-context-user.php b/php/context/class-fieldmanager-context-user.php index 1eb473d0db..bd7990e70e 100644 --- a/php/context/class-fieldmanager-context-user.php +++ b/php/context/class-fieldmanager-context-user.php @@ -174,14 +174,7 @@ protected function add_data( $user_id, $meta_key, $meta_value, $unique = false ) * @param mixed $data_prev_value The previous meta data. */ protected function update_data( $user_id, $meta_key, $meta_value, $data_prev_value = '' ) { - /* - * Meta is always stored as a string, so if this is a scalar value, cast - * it as a string to ensure that `update_metadata()` is able to correctly - * compare the current value against the previous value. - */ - if ( is_scalar( $meta_value ) && ! is_string( $meta_value ) ) { - $meta_value = strval( $meta_value ); - } + $meta_value = $this->sanitize_scalar_value( $meta_value ); return call_user_func( /** * Filters function used to update user meta. This improves compatibility with diff --git a/tests/php/test-fieldmanager-context-storable.php b/tests/php/test-fieldmanager-context-storable.php new file mode 100644 index 0000000000..e7997079ed --- /dev/null +++ b/tests/php/test-fieldmanager-context-storable.php @@ -0,0 +1,32 @@ +assertSame( $expected, \Fieldmanager_Context_Post::sanitize_scalar_value( $test ) ); + } +} \ No newline at end of file