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

Full property visibility/staticness review & fixing of custom property merging #841

Merged
merged 15 commits into from
Feb 25, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions WordPress/Tests/CSRF/NonceVerificationUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,38 @@ $b = function () {

do_something( $_POST['abc'] ); // Bad.
};

/*
* Test using custom properties, setting & unsetting (resetting).
*/
// @codingStandardsChangeSetting WordPress.CSRF.NonceVerification customNonceVerificationFunctions my_nonce_check
// @codingStandardsChangeSetting WordPress.CSRF.NonceVerification customSanitizingFunctions sanitize_pc,sanitize_twitter
// @codingStandardsChangeSetting WordPress.CSRF.NonceVerification customUnslashingSanitizingFunctions do_something

function foo_5() {

sanitize_twitter( $_POST['foo'] ); // OK.
sanitize_pc( $_POST['bar'] ); // OK.
my_nonce_check( do_something( $_POST['tweet'] ) ); // OK.
}

// @codingStandardsChangeSetting WordPress.CSRF.NonceVerification customSanitizingFunctions sanitize_pc
// @codingStandardsChangeSetting WordPress.CSRF.NonceVerification customUnslashingSanitizingFunctions false

function foo_5() {

do_something( $_POST['foo'] ); // Bad.
sanitize_pc( $_POST['bar'] ); // OK.
sanitize_twitter( $_POST['bar'] ); // Bad.
my_nonce_check( sanitize_twitter( $_POST['tweet'] ) ); // OK.
}

// @codingStandardsChangeSetting WordPress.CSRF.NonceVerification customNonceVerificationFunctions false
// @codingStandardsChangeSetting WordPress.CSRF.NonceVerification customSanitizingFunctions false

function foo_5() {

do_something( $_POST['foo'] ); // Bad.
sanitize_pc( $_POST['bar'] ); // Bad.
my_nonce_check( sanitize_twitter( $_POST['tweet'] ) ); // Bad.
}
5 changes: 5 additions & 0 deletions WordPress/Tests/CSRF/NonceVerificationUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ public function getErrorList() {
114 => 1,
122 => 1,
126 => 1,
148 => 1,
150 => 1,
159 => 1,
160 => 1,
161 => 1,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,10 @@ echo "This is $PHP_SELF with $HTTP_RAW_POST_DATA"; // Ok.
/*
* Unit test whitelisting.
*/
// @codingStandardsChangeSetting WordPress.NamingConventions.ValidVariableName customPropertiesWhitelist varName
// @codingStandardsChangeSetting WordPress.NamingConventions.ValidVariableName customPropertiesWhitelist varName,DOMProperty
echo MyClass::$varName; // Ok, whitelisted.
echo $this->varName; // Ok, whitelisted.
echo $this->DOMProperty; // Ok, whitelisted.
echo $object->varName; // Ok, whitelisted.
// @codingStandardsChangeSetting WordPress.NamingConventions.ValidVariableName customPropertiesWhitelist false

echo $object->varName; // Bad, no longer whitelisted.
79 changes: 40 additions & 39 deletions WordPress/Tests/NamingConventions/ValidVariableNameUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,45 +25,46 @@ public function getErrorList() {
2 => 1,
5 => 1,
8 => 1,
11 => 1,
13 => 1,
16 => 1,
18 => 1,
21 => 1,
23 => 1,
26 => 1,
29 => 1,
32 => 1,
34 => 1,
38 => 1,
40 => 1,
43 => 1,
51 => 1,
54 => 1,
55 => 1,
57 => 1,
60 => 1,
61 => 1,
64 => 1,
65 => 1,
70 => 1,
77 => 1,
78 => 1,
79 => 1,
80 => 1,
82 => 1,
86 => 1,
87 => 1,
88 => 1,
99 => 1,
100 => 1,
101 => 1,
102 => 1,
103 => 1,
104 => 1,
105 => 1,
121 => 1,
126 => 1,
11 => 1,
13 => 1,
16 => 1,
18 => 1,
21 => 1,
23 => 1,
26 => 1,
29 => 1,
32 => 1,
34 => 1,
38 => 1,
40 => 1,
43 => 1,
51 => 1,
54 => 1,
55 => 1,
57 => 1,
60 => 1,
61 => 1,
64 => 1,
65 => 1,
70 => 1,
77 => 1,
78 => 1,
79 => 1,
80 => 1,
82 => 1,
86 => 1,
87 => 1,
88 => 1,
99 => 1,
100 => 1,
101 => 1,
102 => 1,
103 => 1,
104 => 1,
105 => 1,
121 => 1,
126 => 1,
138 => 1,
);

return $errors;
Expand Down
76 changes: 76 additions & 0 deletions WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,79 @@ $b = function () {

return $listofthings;
};

/*
* Test using custom properties, setting & unsetting (resetting).
*/
// @codingStandardsChangeSetting WordPress.VIP.DirectDatabaseQuery customCacheGetFunctions my_cacheget
// @codingStandardsChangeSetting WordPress.VIP.DirectDatabaseQuery customCacheSetFunctions my_cacheset,my_other_cacheset
// @codingStandardsChangeSetting WordPress.VIP.DirectDatabaseQuery customCacheDeleteFunctions my_cachedel
function cache_custom() {
global $wpdb;

$quux = my_cacheget( 'quux' );
if ( false !== $quux ) {
$wpdb->get_results( 'SELECT X FROM Y' ); // DB call ok; OK.
my_cacheset( 'key', 'group' );
}
}

function cache_custom() {
global $wpdb;

$quux = my_cacheget( 'quux' );
if ( false !== $quux ) {
$wpdb->get_results( 'SELECT X FROM Y' ); // DB call ok; OK.
my_other_cacheset( 'key', 'group' );
}
}

function cache_custom() {
global $wpdb;

$wpdb->query( 'SELECT X FROM Y' ); // DB call ok; OK.
my_cachedel( 'key', 'group' );
}

// @codingStandardsChangeSetting WordPress.VIP.DirectDatabaseQuery customCacheSetFunctions my_cacheset
// @codingStandardsChangeSetting WordPress.VIP.DirectDatabaseQuery customCacheDeleteFunctions false

function cache_custom() {
global $wpdb;

$quux = my_cacheget( 'quux' );
if ( false !== $quux ) {
$wpdb->get_results( 'SELECT X FROM Y' ); // DB call ok; OK.
my_cacheset( 'key', 'group' );
}
}

function cache_custom() {
global $wpdb;

$quux = my_cacheget( 'quux' );
if ( false !== $quux ) {
$wpdb->get_results( 'SELECT X FROM Y' ); // DB call ok; Bad.
my_other_cacheset( 'key', 'group' );
}
}

function cache_custom() {
global $wpdb;

$wpdb->query( 'SELECT X FROM Y' ); // DB call ok; Bad.
my_cachedel( 'key', 'group' );
}

// @codingStandardsChangeSetting WordPress.VIP.DirectDatabaseQuery customCacheGetFunctions false
// @codingStandardsChangeSetting WordPress.VIP.DirectDatabaseQuery customCacheSetFunctions false

function cache_custom() {
global $wpdb;

$quux = my_cacheget( 'quux' );
if ( false !== $quux ) {
$quux = $wpdb->get_results( 'SELECT X FROM Y' ); // DB call ok; Bad.
my_cacheset( 'key', 'group' );
}
}
19 changes: 11 additions & 8 deletions WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,17 @@ class WordPress_Tests_VIP_DirectDatabaseQueryUnitTest extends AbstractSniffUnitT
*/
public function getErrorList() {
return array(
6 => 1,
8 => 1,
32 => 1,
34 => 1,
50 => 1,
78 => 1,
79 => 1,
80 => 1,
6 => 1,
8 => 1,
32 => 1,
34 => 1,
50 => 1,
78 => 1,
79 => 1,
80 => 1,
170 => 1,
178 => 1,
190 => 1,
);

}
Expand Down
29 changes: 29 additions & 0 deletions WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,32 @@ isset( $_POST['abc'] ); // Validation in global scope, not function scope.
$b = function () {
return sanitize_text_field( wp_unslash( $_POST['abc'] ) ); // Error for no validation.
};

/*
* Test using custom properties, setting & unsetting (resetting).
*/
function test_this() {
if ( ! isset( $_POST['abc_field'] ) ) {
return;
}

$abc = sanitize_color( wp_unslash( $_POST['abc_field'] ) ); // Bad x1 - sanitize.

// @codingStandardsChangeSetting WordPress.VIP.ValidatedSanitizedInput customSanitizingFunctions sanitize_color,sanitize_twitter_handle

$abc = sanitize_color( wp_unslash( $_POST['abc_field'] ) ); // OK.
$abc = sanitize_facebook_id( wp_unslash( $_POST['abc_field'] ) ); // Bad x1 - sanitize.
$abc = sanitize_twitter_handle( $_POST['abc_field'] ); // Bad x1 - unslash.

// @codingStandardsChangeSetting WordPress.VIP.ValidatedSanitizedInput customSanitizingFunctions sanitize_color,sanitize_facebook_id
// @codingStandardsChangeSetting WordPress.VIP.ValidatedSanitizedInput customUnslashingSanitizingFunctions sanitize_twitter_handle

$abc = sanitize_color( wp_unslash( $_POST['abc_field'] ) ); // OK.
$abc = sanitize_facebook_id( wp_unslash( $_POST['abc_field'] ) ); // OK.
$abc = sanitize_twitter_handle( $_POST['abc_field'] ); // OK.

// @codingStandardsChangeSetting WordPress.VIP.ValidatedSanitizedInput customSanitizingFunctions false
// @codingStandardsChangeSetting WordPress.VIP.ValidatedSanitizedInput customUnslashingSanitizingFunctions false

$abc = sanitize_twitter_handle( $_POST['abc_field'] ); // Bad x2, sanitize + unslash.
}
4 changes: 4 additions & 0 deletions WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ public function getErrorList() {
105 => 1,
114 => 2,
121 => 1,
132 => 1,
137 => 1,
138 => 1,
150 => 2,
);

}
Expand Down
24 changes: 24 additions & 0 deletions WordPress/Tests/XSS/EscapeOutputUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,27 @@ _deprecated_hook( 'some_filter', '1.3.0', esc_html__( 'The $arg is deprecated.'
_deprecated_hook( "filter_{$context}", '1.3.0', __( 'The $arg is deprecated.' ), sprintf( __( 'Some parsed message %s', $variable ) ) ); // Bad.

echo add_filter( get_the_excerpt( get_the_ID() ) ; // Bad, but ignored as code contains a parse error.


Copy link
Contributor

Choose a reason for hiding this comment

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

Extra blank line here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

/*
* Test using custom properties, setting & unsetting (resetting).
*/
// @codingStandardsChangeSetting WordPress.XSS.EscapeOutput customPrintingFunctions to_screen,my_print
to_screen( $var1, esc_attr( $var2 ) ); // Bad x 1.
my_print( $var1, $var2 ); // Bad x 2.

// @codingStandardsChangeSetting WordPress.XSS.EscapeOutput customEscapingFunctions esc_form_field
// @codingStandardsChangeSetting WordPress.XSS.EscapeOutput customAutoEscapedFunctions post_info,cpt_info

echo esc_form_field( $var ); // Ok.
echo post_info( $post_id, 'field' ); // Ok.
echo cpt_info( $post_type, 'query' ); // Ok.
to_screen( esc_form_field( $var1), esc_attr( $var2 ) ); // Ok.

// @codingStandardsChangeSetting WordPress.XSS.EscapeOutput customPrintingFunctions false
// @codingStandardsChangeSetting WordPress.XSS.EscapeOutput customEscapingFunctions false
// @codingStandardsChangeSetting WordPress.XSS.EscapeOutput customAutoEscapedFunctions false

echo esc_form_field( $var ); // Bad.
echo post_info( $post_id, 'field' ); // Bad.
echo cpt_info( $post_type, 'query' ); // Bad.
5 changes: 5 additions & 0 deletions WordPress/Tests/XSS/EscapeOutputUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ public function getErrorList() {
172 => 1,
173 => 1,
182 => 3,
191 => 1,
192 => 2,
206 => 1,
207 => 1,
208 => 1,
);

} // end getErrorList()
Expand Down