Skip to:
Content

BuddyPress.org

Changeset 8129


Ignore:
Timestamp:
03/14/2014 12:07:18 AM (11 years ago)
Author:
boonebgorges
Message:

Don't improperly sanitize meta_key in _meta() functions

Many BP meta functions have always stripped certain characters from the
$meta_key parameter before performing their operations. This is a terrible idea
on a number of levels: it doesn't provide any feedback to the user, it silently
performs actions that are not equivalent to the ones intended by the user, and
it doesn't serve any real purpose (since any necessary sanitization happens at
the level of $wpdb). Moreover, it wasn't even applied consistently across all
functions. A truly delightful grab bag.

This changeset removes the sanitization, and updates the necessary unit tests
to reflect the change.

See #5399

Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/bp-activity/bp-activity-functions.php

    r8126 r8129  
    577577    }
    578578
    579     // Legacy - Sanitize key
    580     $meta_key = preg_replace( '|[^a-z0-9_]|i', '', $meta_key );
    581 
    582579    // Legacy - Trim off whitespace
    583580    $meta_value = trim( $meta_value );
    … …  
    627624        return false;
    628625    }
    629 
    630     // Legacy - Sanitize keys
    631     $meta_key = preg_replace( '|[^a-z0-9_]|i', '', $meta_key );
    632626
    633627    add_filter( 'query', 'bp_filter_metaid_column_name' );
    … …  
    684678        return false;
    685679    }
    686 
    687     // Legacy - Sanitize key
    688     $meta_key = preg_replace( '|[^a-z0-9_]|i', '', $meta_key );
    689680
    690681    add_filter( 'query', 'bp_filter_metaid_column_name' );

  • trunk/bp-blogs/bp-blogs-functions.php

    r8125 r8129  
    854854    }
    855855
    856     // Legacy - sanitize meta_key
    857     $meta_key = preg_replace( '|[^a-z0-9_]|i', '', $meta_key );
    858 
    859856    // Legacy - if no meta_key is passed, delete all for the blog_id
    860857    if ( empty( $meta_key ) ) {
    … …  
    898895function bp_blogs_get_blogmeta( $blog_id, $meta_key = '', $single = true ) {
    899896
    900     // Legacy - Sanitize meta_key
    901     $meta_key = preg_replace('|[^a-z0-9_]|i', '', $meta_key);
    902 
    903897    add_filter( 'query', 'bp_filter_metaid_column_name' );
    904898    $retval = get_metadata( 'blog', $blog_id, $meta_key, $single );
    … …  
    934928function bp_blogs_update_blogmeta( $blog_id, $meta_key, $meta_value, $prev_value = '' ) {
    935929
    936     // Legacy - Sanitize meta_key
    937     $meta_key = preg_replace( '|[^a-z0-9_]|i', '', $meta_key );
    938 
    939930    add_filter( 'query', 'bp_filter_metaid_column_name' );
    940931    $retval = update_metadata( 'blog', $blog_id, $meta_key, $meta_value, $prev_value );

  • trunk/bp-groups/bp-groups-functions.php

    r8125 r8129  
    10381038    }
    10391039
    1040     // Legacy - Sanitize keys
    1041     $meta_key = preg_replace( '|[^a-z0-9_]|i', '', $meta_key );
    1042 
    10431040    // Legacy - if no meta_key is passed, delete all for the item
    10441041    if ( empty( $meta_key ) ) {
    … …  
    10731070 */
    10741071function groups_get_groupmeta( $group_id, $meta_key = '', $single = true ) {
    1075 
    1076     // Legacy - Sanitize keys
    1077     $meta_key = preg_replace( '|[^a-z0-9_]|i', '', $meta_key );
    10781072
    10791073    add_filter( 'query', 'bp_filter_metaid_column_name' );

  • trunk/bp-xprofile/bp-xprofile-functions.php

    r8087 r8129  
    672672function bp_xprofile_update_meta( $object_id, $object_type, $meta_key, $meta_value, $prev_value = '' ) {
    673673
    674     // Legacy - sanitize meta_key
    675     $meta_key = preg_replace( '|[^a-z0-9_]|i', '', $meta_key );
    676 
    677674    add_filter( 'query', 'bp_filter_metaid_column_name' );
    678675    add_filter( 'query', 'bp_xprofile_filter_meta_query' );

  • trunk/tests/testcases/activity/functions.php

    r8125 r8129  
    126126     * @group activitymeta
    127127     * @group bp_activity_update_meta
     128     * @ticket BP5399
    128129     */
    129130    public function test_bp_activity_update_meta_with_illegal_key_characters() {
    … …  
    132133        bp_activity_update_meta( $a, $krazy_key, 'bar' );
    133134
    134         $this->assertSame( 'bar', bp_activity_get_meta( $a, 'foo' ) );
     135        $this->assertEmpty( bp_activity_get_meta( $a, 'foo' ) );
    135136    }
    136137
    … …  
    231232     * @group activitymeta
    232233     * @group bp_activity_get_meta
     234     * @ticket BP5399
    233235     */
    234236    public function test_bp_activity_get_meta_with_illegal_characters() {
    … …  
    237239
    238240        $krazy_key = ' f!@#$%^o *(){}o?+';
    239         $this->assertNotEmpty( bp_activity_get_meta( $a, 'foo' ) );
    240         $this->assertSame( bp_activity_get_meta( $a, 'foo' ), bp_activity_get_meta( $a, $krazy_key ) );
     241        $this->assertEmpty( bp_activity_get_meta( $a, $krazy_key ) );
    241242    }
    242243

  • trunk/tests/testcases/blogs/functions.php

    r8073 r8129  
    1616     * @group blogmeta
    1717     * @group bp_blogs_delete_blogmeta
     18     * @ticket BP5399
    1819     */
    1920    public function test_bp_blogs_delete_blogmeta_illegal_characters() {
    … …  
    2122        $this->assertSame( 'bar', bp_blogs_get_blogmeta( 1, 'foo' ) );
    2223        $krazy_key = ' f!@#$%^o *(){}o?+';
    23         $this->assertTrue( bp_blogs_delete_blogmeta( 1, $krazy_key ) );
    24         $this->assertSame( '', bp_blogs_get_blogmeta( 1, 'foo' ) );
     24        $this->assertFalse( bp_blogs_delete_blogmeta( 1, $krazy_key ) );
     25        $this->assertSame( 'bar', bp_blogs_get_blogmeta( 1, 'foo' ) );
    2526    }
    2627
    … …  
    110111     * @group blogmeta
    111112     * @group bp_blogs_get_blogmeta
     113     * @ticket BP5399
    112114     */
    113115    public function test_bp_blogs_get_blogmeta_illegal_characters() {
    114116        bp_blogs_update_blogmeta( 1, 'foo', 'bar' );
    115117        $krazy_key = ' f!@#$%^o *(){}o?+';
    116         $this->assertSame( 'bar', bp_blogs_get_blogmeta( 1, $krazy_key ) );
     118        $this->assertEmpty( bp_blogs_get_blogmeta( 1, $krazy_key ) );
    117119    }
    118120
    … …  
    167169     * @group blogmeta
    168170     * @group bp_blogs_update_blogmeta
     171     * @ticket BP5399
    169172     */
    170173    public function test_bp_blogs_update_blogmeta_illegal_characters() {
    171174        $krazy_key = ' f!@#$%^o *(){}o?+';
    172175        bp_blogs_update_blogmeta( 1, $krazy_key, 'bar' );
    173         $this->assertSame( 'bar', bp_blogs_get_blogmeta( 1, 'foo' ) );
     176        $this->assertEmpty( bp_blogs_get_blogmeta( 1, 'foo' ) );
    174177    }
    175178

  • trunk/tests/testcases/groups/functions.php

    r8056 r8129  
    357357    /**
    358358     * @group groupmeta
    359      *
    360      * @todo Why do we do this?
     359     * @group groups_get_groupmeta
     360     * @ticket BP5399
    361361     */
    362362    public function test_groups_get_groupmeta_with_illegal_key_characters() {
    … …  
    365365
    366366        $krazy_key = ' f!@#$%^o *(){}o?+';
    367         $this->assertSame( groups_get_groupmeta( $g, 'foo' ), groups_get_groupmeta( $g, $krazy_key ) );
     367        $this->assertEmpty( groups_get_groupmeta( $g, $krazy_key ) );
    368368    }
    369369
    … …  
    463463    /**
    464464     * @group groupmeta
     465     * @group groups_delete_groupmeta
     466     * @ticket BP5399
    465467     */
    466468    public function test_groups_delete_groupmeta_with_illegal_key_characters() {
    … …  
    469471
    470472        $krazy_key = ' f!@#$%^o *(){}o?+';
    471         $this->assertTrue( groups_delete_groupmeta( $g, $krazy_key ) );
    472         $this->assertSame( '', groups_get_groupmeta( $g, 'foo' ) );
     473        $this->assertSame( 'bar', groups_get_groupmeta( $g, 'foo' ) );
    473474    }
    474475

  • trunk/tests/testcases/xprofile/functions.php

    r8109 r8129  
    186186     * @group xprofilemeta
    187187     * @group bp_xprofile_delete_meta
     188     * @ticket BP5399
    188189     */
    189190    public function test_bp_xprofile_delete_meta_illegal_characters() {
    … …  
    193194
    194195        $krazy_key = ' f!@#$%^o *(){}o?+';
    195         $this->assertTrue( bp_xprofile_delete_meta( $g, 'group', 'foo' ) );
    196         $this->assertEquals( '', bp_xprofile_get_meta( $g, 'group', 'foo' ) );
     196        bp_xprofile_delete_meta( $g, 'group', $krazy_key );
     197        $this->assertSame( 'bar', bp_xprofile_get_meta( $g, 'group', 'foo' ) );
    197198    }
    198199
    … …  
    378379     * @group xprofilemeta
    379380     * @group bp_xprofile_update_meta
     381     * @ticket BP5399
    380382     */
    381383    public function test_bp_xprofile_update_meta_illegal_characters() {
    … …  
    383385        $krazy_key = ' f!@#$%^o *(){}o?+';
    384386        bp_xprofile_update_meta( $g, 'group', $krazy_key, 'bar' );
    385         $this->assertSame( 'bar', bp_xprofile_get_meta( $g, 'group', 'foo' ) );
     387        $this->assertEmpty( bp_xprofile_get_meta( $g, 'group', 'foo' ) );
    386388    }
    387389
Note: See TracChangeset for help on using the changeset viewer.