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 check to ban use of specified methods #604

Closed
raghavgautam opened this Issue Sep 11, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@raghavgautam
Copy link
Contributor

raghavgautam commented Sep 11, 2017

I have some code where I want to forbid usage of certain methods. I want to forbid exit & exit2 methods. System.exit() can shut down application and lead to DoS attack. I also want to forbid 1 argument variant of assertTrue and assertFalse methods because test failures without an explanation is less useful. Here is a sample output of the check:

$mvn clean checkstyle:check
[INFO] Scanning for projects...
[INFO]                                                                         
[INFO] ------------------------------------------------------------------------
[INFO] Building maven-project 0.0.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO] 
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ maven-project ---
[INFO] Deleting /Users/raghav/play/checkstyle-samples/maven-project/target
[INFO] 
[INFO] --- maven-checkstyle-plugin:2.17:check (default-cli) @ maven-project ---
[INFO] There are 4 errors reported by Checkstyle 8.4 with checkstyle.xml ruleset.
[ERROR] src/main/java/com/github/sevntu/checkstyle/InputForbidCertainMethodsCheck.java:[22,20] (extension) ForbidCertainMethods: Call to 'exit' method (matches pattern exit.*) is forbidden.
[ERROR] src/main/java/com/github/sevntu/checkstyle/InputForbidCertainMethodsCheck.java:[29,26] (extension) ForbidCertainMethods: Call to 'assertTrue' method (matches pattern assert(True|False)) with '1' arguments (matches pattern 1) is forbidden.
[ERROR] src/main/java/com/github/sevntu/checkstyle/InputForbidCertainMethodsCheck.java:[31,31] (extension) ForbidCertainMethods: Call to 'exit2' method (matches pattern exit.*) is forbidden.
[ERROR] src/main/java/com/github/sevntu/checkstyle/InputForbidCertainMethodsCheck.java:[32,40] (extension) ForbidCertainMethods: Call to 'exit2' method (matches pattern exit.*) is forbidden.
[WARNING] checkstyle:check violations detected but failOnViolation set to false
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1.225 s
[INFO] Finished at: 2017-10-30T17:35:23-07:00
[INFO] Final Memory: 11M/211M
[INFO] ------------------------------------------------------------------------

To produce the above example I started with the following project:
https://github.com/sevntu-checkstyle/checkstyle-samples/tree/master/maven-project
And made some changes to it.

$ cat checkstyle.xml

<?xml version="1.0" ?>

<!DOCTYPE module PUBLIC
  "-//Puppy Crawl//DTD Check Configuration 1.2//EN"
  "http://www.puppycrawl.com/dtds/configuration_1_2.dtd">

<module name="Checker">
  <module name="TreeWalker">
    <module name="com.github.sevntu.checkstyle.checks.coding.ForbidCertainMethodsCheck">
      <property name="methodName" value="exit.*"/>
      <property name="argumentCount" value=""/>
    </module>
    <module name="com.github.sevntu.checkstyle.checks.coding.ForbidCertainMethodsCheck">
      <property name="methodName" value="assert(True|False)"/>
      <property name="argumentCount" value="1"/>
    </module>
  </module>
</module>

$ cat src/main/java/com/github/sevntu/checkstyle/InputForbidCertainMethodsCheck.java

////////////////////////////////////////////////////////////////////////////////
// Test case file for checkstyle.
// Created: 2017
////////////////////////////////////////////////////////////////////////////////
package com.github.sevntu.checkstyle.checks.coding;

import org.junit.Assert;

import java.io.File;
import java.util.Arrays;

/**
 * Test case for detecting usage of forbidden methods & constructors.
 * @author Raghav Kumar Gautam
 **/
class InputForbidCertainMethodsCheck
{
    /**
     * no param constructor
     */
    InputForbidCertainMethodsCheck() {
        System.exit(1);
    }

    /**
     * non final param method
     */
    void method(String s) {
        Assert.assertTrue(1 != 2);
        Assert.assertTrue("Good assert with some reason.", true);
        ForbiddenMethods.exit2();
        ForbiddenMethods.INSTANCE.exit2();
        //method call that does not need "." in it
        method("");
        // new usage that does not invoke constructor
        String[] strs = new String[4];
        Arrays.stream(strs).map(File::new).count();
    }

    private static class ForbiddenMethods {
        static final ForbiddenMethods INSTANCE = new ForbiddenMethods();
        static void exit2() {
        }
    }
}

More backgroud information about this issue can be found at: checkstyle/checkstyle#5083

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Sep 13, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Sep 13, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Sep 13, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Sep 13, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Sep 13, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Sep 24, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Sep 25, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Oct 2, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Oct 2, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Oct 24, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Oct 25, 2017

@rnveach

This comment has been minimized.

Copy link
Contributor

rnveach commented Oct 27, 2017

@romani I assume this issue is approved?

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Oct 27, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Oct 27, 2017

@raghavgautam raghavgautam changed the title Add a check to ban use of specified methods and constructors Add a check to ban use of specified methods Oct 28, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Oct 30, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Oct 30, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Oct 30, 2017

@romani

This comment has been minimized.

Copy link
Member

romani commented Oct 31, 2017

@raghavgautam ,

<property name="argumentCount" value=""/>

argumentCount should be always integer, any other value should result in error.
Default value should be 0.
Please update code and description of issue, and place mention this in javadoc, please put example to javadoc too

@rnveach

This comment has been minimized.

Copy link
Contributor

rnveach commented Oct 31, 2017

@romani argumentCount is a regular expression. It is allowed to be empty to completely forbid a method no matter how many parameters is given.
If it isn't allowed to be a regular expression or a wildcard, you could never forbid a vararg method call as it can have 0 to infinity number of parameters.

@raghavgautam

This comment has been minimized.

Copy link
Contributor

raghavgautam commented Oct 31, 2017

@romani I agree with @rnveach. An additional benefit of having argumentCount as regex is that it will be easier to ban a series of overloaded methods with different number of arguments.

@romani

This comment has been minimized.

Copy link
Member

romani commented Nov 9, 2017

@raghavgautam ,
I do not see how you plan to do this ... please put examples to javadoc to let me and all others see hidden value of this.

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Nov 20, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Nov 28, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Nov 28, 2017

@romani romani added the approved label Nov 28, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Dec 7, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Dec 8, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Dec 17, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Dec 17, 2017

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Jan 10, 2018

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Jan 13, 2018

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Jan 13, 2018

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Jan 18, 2018

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Jan 21, 2018

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Jan 31, 2018

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Feb 1, 2018

romani added a commit that referenced this issue Feb 2, 2018

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Feb 5, 2018

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Feb 6, 2018

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Feb 10, 2018

raghavgautam added a commit to raghavgautam/sevntu.checkstyle that referenced this issue Feb 10, 2018

romani added a commit that referenced this issue Feb 11, 2018

@romani romani added this to the 1.28.0 milestone Feb 11, 2018

@romani

This comment has been minimized.

Copy link
Member

romani commented Feb 11, 2018

Fix is merged.

@rnveach , please do release

@romani romani closed this Feb 11, 2018

kariem added a commit to kariem/sevntu.checkstyle that referenced this issue Jul 26, 2018

kariem added a commit to kariem/sevntu.checkstyle that referenced this issue Jul 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment