Skip to content
This repository has been archived by the owner on Jul 20, 2018. It is now read-only.

Adding three checks for non-escaped localization function calls. #315

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
101 changes: 101 additions & 0 deletions tests/checks/test-VIPEscapingCheck.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
<?php

require_once( 'CodeCheckTestBase.php' );

class VIPEscapingTest extends CodeCheckTestBase {

public function testEscaping() {
$lines = 3;

$expected_errors = array();


/*
* Why 4 * 5? Why a the loop?
* Because we are specifying very similar
* test-result, again and again.
*
* First, four tests for echo (without brackets)
* echo __( ... );
* echo _x( ... );
* echo _n( ... );
* echo _nx( ... );
*
* Second, four similar tests for echo with brackets
* Third, four similar tests for print without brackets
* Fourth, four similar tests for print with brackets
* Fifth, four simila tests for vprintf (with brackets)
*
* Note: If you find yourself adding 'ifs' within the
* for-loop, stop using a loop and write out each
* expected error individually.
*/

for ($i = 0; $i < 4 * 5; $i++) {
$expected_errors[] = array(
'slug' => 'functions-file',
'level' => BaseScanner::LEVEL_BLOCKER,
'description' => sprintf(
esc_html__( 'Printing output of non-escaping localization functions (i.e. %1$s) is potentially dangerous, as they do not escape HTML. An escaping function (e.g. %2$s) should be used rather.', 'vip-scanner' ),
'<code>__( ), _x( ), _n( ), _nx( )</code>',
'<code>esc_html__( ), esc_attr__( ), esc_html_x( ), esc_attr_x( )</code>'
),
'file' => 'VIPEscapingTest.inc',
'lines' => $lines++,
);
}

$lines++;


/*
* Now test for non-printing usage of __( ), _x( ),
* _n( ), and _nx( ).
*
* Note: If you find yourself adding 'ifs' within the
* for-loop, stop using a loop and write out each
* expected error individually.
*/

for ($i = 0; $i < 4; $i++) {
$expected_errors[] = array(
'slug' => 'functions-file',
'level' => BaseScanner::LEVEL_WARNING,
'description' => sprintf(
esc_html__( 'Usage of non-escaping localization functions (i.e. %1$s) is discouraged as they do not escape HTML. An escaping function (e.g. %2$s) should be used rather.', 'vip-scanner' ),
'<code>__( ), _x( ), _n( ), _nx( )</code>',
'<code>esc_html__( ), esc_attr__( ), esc_html_x( ), esc_attr_x( )</code>'
),
'file' => 'VIPEscapingTest.inc',
'lines' => $lines++,
);
}


/*
* Now test for usage of _e( ), _ex( ).
*
* Note: If you find yourself adding 'ifs' within the
* for-loop, stop using a loop and write out each
* expected error individually.
*/

for ($i = 0; $i < 2; $i++) {
$expected_errors[] = array(
'slug' => 'functions-file',
'level' => BaseScanner::LEVEL_BLOCKER,
'description' => sprintf(
esc_html__( 'Usage of non-escaping localization functions (i.e. %1$s) is discouraged as they do not escape HTML. An escaping function (e.g. %2$s) should be used rather.', 'vip-scanner' ),
'<code>_e( ), _ex( )</code>',
'<code>esc_html_e( ), esc_attr_e( ), esc_html_x( ), esc_attr_x( ), esc_attr__( )</code>'
),
'file' => 'VIPEscapingTest.inc',
'lines' => $lines++,
);
}


$actual_errors = $this->checkFile( 'VIPEscapingTest.inc' );
$this->assertEqualErrors( $expected_errors, $actual_errors );
}
}
30 changes: 30 additions & 0 deletions tests/data/VIPEscapingTest.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

die(); //Don't actually run the following code.

echo __( 'Message #1' );
echo _x( 'Message #1' );
echo _n( 'Message #1' );
echo _nx( 'Message #1' );
echo( __( 'Message #1' ) );
echo( _x( 'Message #1' ) );
echo( _n( 'Message #1' ) );
echo( _nx( 'Message #1' ) );
print __( 'Message #1' );
print _x( 'Message #1' );
print _n( 'Message #1' );
print _nx( 'Message #1' );
print( __( 'Message #1' ) );
print( _x( 'Message #1' ) );
print( _n( 'Message #1' ) );
print( _nx( 'Message #1' ) );
vprintf ( __( 'Message #%d' ), 1 );
vprintf ( _x( 'Message #%d' ), 1 );
vprintf ( _n( 'Message #%d' ), 1 );
vprintf ( _nx( 'Message #%d' ), 1 );
__( 'Message #1' );
_x( 'Message #1' );
_n( 'Message #1' );
_nx( 'Message #1' );
_e( 'Message #1' );
_ex( 'Message #1' );
75 changes: 75 additions & 0 deletions vip-scanner/checks/VIPEscapingCheck.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

/**
* Detect errors in escaping.
*/
class VIPEscapingCheck extends BaseCheck {
function check( $files ) {
$checks = array(
array(
/*
* Catch printing usage of __( ), etc.
* These are blockers because the results
* are being printed without HTML-escaping the
* results.
*/
'pattern' => '/([\s]|[;]){1,}(echo|vprintf|print)([\s]|[\(])+(__|_x|_n|_nx)[\s]*?\(/',
'level' => 'blocker',
'message' => sprintf(
esc_html__( 'Printing output of non-escaping localization functions (i.e. %1$s) is potentially dangerous, as they do not escape HTML. An escaping function (e.g. %2$s) should be used rather.', 'vip-scanner' ),
'<code>__( ), _x( ), _n( ), _nx( )</code>',
'<code>esc_html__( ), esc_attr__( ), esc_html_x( ), esc_attr_x( )</code>'
),
),
array(
/*
* Catch non-printing usage of __( ), etc
* These are warnings, because the results
* are not being immediately printed, and so
* could be escaped at a later point.
*/
'pattern' => '/([\s]|[;]|[=]){1,}[a-zA-Z]{0}[\s]+(__|_x|_n|_nx)[\s]*?\(/',
'level' => 'warning',
'message' => sprintf(
esc_html__( 'Usage of non-escaping localization functions (i.e. %1$s) is discouraged as they do not escape HTML. An escaping function (e.g. %2$s) should be used rather.', 'vip-scanner' ),
'<code>__( ), _x( ), _n( ), _nx( )</code>',
'<code>esc_html__( ), esc_attr__( ), esc_html_x( ), esc_attr_x( )</code>'
),
),
array(
/*
* Catch calls to _e( ), _ex( )
* These print directly, without HTML-escaping,
* and so are blockers.
*/
'pattern' => '/([\s]|[;]){1,}[a-zA-Z]{0}[\s]+(_e|_ex)[\s]*?\(/',
'level' => 'blocker',
'message' => sprintf(
esc_html__( 'Usage of non-escaping localization functions (i.e. %1$s) is discouraged as they do not escape HTML. An escaping function (e.g. %2$s) should be used rather.', 'vip-scanner' ),
'<code>_e( ), _ex( )</code>',
'<code>esc_html_e( ), esc_attr_e( ), esc_html_x( ), esc_attr_x( ), esc_attr__( )</code>'
),
),
);

$result = true;
foreach ( $checks as $check ) {
$this->increment_check_count();
foreach ( $this->filter_files( $files, 'php' ) as $path => $code ) {
$filename = $this->get_filename( $path );
$errors = $this->preg_file2( $check['pattern'], $code );
foreach ( (array) $errors as $line_number => $error ) {
$this->add_error(
'functions-file',
$check['message'],
$check['level'],
$filename,
array( $line_number => $error )
);
$result = false;
}
}
}
return $result;
}
}
1 change: 1 addition & 0 deletions vip-scanner/config-vip-scanner.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
'DeprecatedFunctionsCheck',
'DeprecatedParametersCheck',
'jQueryCheck',
'VIPEscapingCheck',
'VIPWhitelistCheck',
'VIPRestrictedClassesCheck',
'VIPRestrictedPatternsCheck',
Expand Down