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

Add a sniff to detect the usage of the backtick operator. #1073

Merged
merged 1 commit into from
Jul 26, 2016

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jul 16, 2016

Working on a project now which wants to add a sniff like this as part of their security-sniffs and figured it would be re-usable for others as well.

{
$errors = array();

if ($this->isSafeModeEnabled() === false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead you can:

  1. also expect errors
  2. add shouldSkipTest function that would call $this->isSafeModeEnabled() === false
  3. in fact you can inline the isSafeModeEnabled function into shouldSkipTest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point - adjusting now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally I intended to check for the backtick character in other tokens when safe mode was enabled and throw a warning, but decided against it. This code was left behind from that.

@gsherwood gsherwood merged commit 4543ab2 into squizlabs:master Jul 26, 2016
gsherwood added a commit that referenced this pull request Jul 26, 2016
@jrfnl jrfnl deleted the feature/no-backtick-eval branch July 26, 2016 01:19
@gsherwood
Copy link
Member

I ended up making the error message say "forbidden" instead of "strongly discouraged" because it was an error instead of a warning, and it also matches other error messages. But the message and type (error/warning) can be overridden in a ruleset.xml file, so I don't think that should be a problem.

Thanks for the new sniff.

jrfnl added a commit to WordPress/WordPress-Coding-Standards that referenced this pull request Jul 26, 2016
… to `extra`.

Execution of shell commands should be discouraged. A number of the other commands along the same lines are discouraged via function sniffs, however, the backtick operator was so far ignored.

For this sniff to be added, the minimum PHPCS version needs to be upped as it has only just been added in the latest release.

See: squizlabs/PHP_CodeSniffer#1073
@jrfnl
Copy link
Contributor Author

jrfnl commented Jul 26, 2016

@gsherwood That's absolutely fine by me and shouldn't be an issue for the depending projects (in my case).

jrfnl added a commit to WordPress/WordPress-Coding-Standards that referenced this pull request Jul 28, 2016
… to `extra`.

Execution of shell commands should be discouraged. A number of the other commands along the same lines are discouraged via function sniffs, however, the backtick operator was so far ignored.

For this sniff to be added, the minimum PHPCS version needs to be upped as it has only just been added in the latest release.

See: squizlabs/PHP_CodeSniffer#1073
jrfnl added a commit to WordPress/WordPress-Coding-Standards that referenced this pull request Aug 18, 2016
… to `extra`.

Execution of shell commands should be discouraged. A number of the other commands along the same lines are discouraged via function sniffs, however, the backtick operator was so far ignored.

For this sniff to be added, the minimum PHPCS version needs to be upped as it has only just been added in the latest release.

See: squizlabs/PHP_CodeSniffer#1073
jrfnl added a commit to WordPress/WordPress-Coding-Standards that referenced this pull request Aug 26, 2016
… to `extra`.

Execution of shell commands should be discouraged. A number of the other commands along the same lines are discouraged via function sniffs, however, the backtick operator was so far ignored.

For this sniff to be added, the minimum PHPCS version needs to be upped as it has only just been added in the latest release.

See: squizlabs/PHP_CodeSniffer#1073
jrfnl added a commit to WordPress/WordPress-Coding-Standards that referenced this pull request Sep 6, 2016
… to `extra`.

Execution of shell commands should be discouraged. A number of the other commands along the same lines are discouraged via function sniffs, however, the backtick operator was so far ignored.

For this sniff to be added, the minimum PHPCS version needs to be upped as it has only just been added in the latest release.

See: squizlabs/PHP_CodeSniffer#1073
jrfnl added a commit to WPTT/WPThemeReview that referenced this pull request Sep 6, 2016
Execution of shell commands should be discouraged. A number of the other commands along the same lines are discouraged via function sniffs, however, the backtick operator was so far ignored.

For this sniff to be added, the minimum PHPCS version needs to be upped as it has only just been added in the latest release.

See: squizlabs/PHP_CodeSniffer#1073
jrfnl added a commit to WordPress/WordPress-Coding-Standards that referenced this pull request Sep 7, 2016
… to `extra`.

Execution of shell commands should be discouraged. A number of the other commands along the same lines are discouraged via function sniffs, however, the backtick operator was so far ignored.

For this sniff to be added, the minimum PHPCS version needs to be upped as it has only just been added in the latest release.

See: squizlabs/PHP_CodeSniffer#1073
jrfnl added a commit to WordPress/WordPress-Coding-Standards that referenced this pull request Sep 11, 2016
… to `extra`.

Execution of shell commands should be discouraged. A number of the other commands along the same lines are discouraged via function sniffs, however, the backtick operator was so far ignored.

For this sniff to be added, the minimum PHPCS version needs to be upped as it has only just been added in the latest release.

See: squizlabs/PHP_CodeSniffer#1073
jrfnl added a commit to WPTT/WPThemeReview that referenced this pull request Oct 27, 2016
Execution of shell commands should be discouraged. A number of the other commands along the same lines are discouraged via function sniffs, however, the backtick operator was so far ignored.

For this sniff to be added, the minimum PHPCS version needs to be upped as it has only just been added in the latest release.

See: squizlabs/PHP_CodeSniffer#1073
grappler pushed a commit to WPTT/WPThemeReview that referenced this pull request Oct 27, 2016
#86)

Execution of shell commands should be discouraged. A number of the other commands along the same lines are discouraged via function sniffs, however, the backtick operator was so far ignored.

For this sniff to be added, the minimum PHPCS version needs to be upped as it has only just been added in the latest release.

See: squizlabs/PHP_CodeSniffer#1073
grappler pushed a commit to grappler/WordPress-Coding-Standards that referenced this pull request Feb 10, 2017
WordPress#86)

Execution of shell commands should be discouraged. A number of the other commands along the same lines are discouraged via function sniffs, however, the backtick operator was so far ignored.

For this sniff to be added, the minimum PHPCS version needs to be upped as it has only just been added in the latest release.

See: squizlabs/PHP_CodeSniffer#1073
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants