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

[java] CommentUtil.multiLinesIn() could lead to StringIndexOutOfBoundsException #1174

Closed
phinehasz opened this Issue Jun 6, 2018 · 1 comment

Comments

Projects
None yet
3 participants
@phinehasz

phinehasz commented Jun 6, 2018

pmd/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/CommentUtil.java

Affects PMD Version: latest version

Description: I'm working to develop my own comment rule to find all code in comment , then I use CommentUtils. I tried to multiline the comment ,but found StringIndexOutOfBoundsException when testing by a test project. actually, it happens in line 108,when it goes to charAt(0)

if (line.charAt(0) == '*') {
                filteredLines.add(line.substring(1));
                continue;
}

because sometime we make a comment like this

/*
    sometime we use ctrl+shift+/ to make a multiLineComment(not use by /** )
     <--- here, thie line causes a string ""        
 */

In line 3,when using CommentUtil.multiLinesIn(),this variable string line = "",cause there is nothing!
so "".charAt(0) throws StringIndexOutOfBoundsException.
In my code,I add a StringUtil.isEmpty(line) to check it. In my comment rule, every empty string is also useful, because I use it to count beginLineNumber. I don't whether PMD provides a better way to multiLine the multiComment or JavaDocComment? I only find this CommentUtil useful, so I hope you guys to add a empty check beyond line 96 just like following code:

String line = rawLine.trim();
           
           if(StringUtil.isEmpty(line)){
           	continue;
           }
           
           if (line.startsWith("//")) {
               filteredLines.add(line.substring(2));
               continue;
           }
    ...

or we can use if("".equals(line)) to check it.

@jsotuyod jsotuyod added the a:bug label Jun 7, 2018

@jsotuyod jsotuyod added this to the 6.5.0 milestone Jun 7, 2018

@jsotuyod

This comment has been minimized.

Member

jsotuyod commented Jun 7, 2018

@phinehasz thanks for the report, that is effectively a bug.

However, bare in mind this class CommentUtil is largely unmaintained (no meaningful commits since moving to git back in 2014), and is largely unused even internally (the only method actually used is javadocTagsIn), there is even a main in that class that makes no sense at all.

We will for sure fix this in 6.5.0 release, but along the way we will most probably just deprecate the whole class to flag it for removal in 7.0.0. We will aim to move actually needed behavior into Comment itself (or it's subclasses). Your input as to what you need is welcomed to help us shape those APIs.

@jsotuyod jsotuyod changed the title from PMD/Java -- CommentUtil.multiLinesIn() could lead to StringIndexOutOfBoundsException to [java] CommentUtil.multiLinesIn() could lead to StringIndexOutOfBoundsException Jun 7, 2018

@adangel adangel self-assigned this Jun 21, 2018

adangel added a commit to adangel/pmd that referenced this issue Jun 21, 2018

@adangel adangel added the has:pr label Jun 21, 2018

adangel added a commit to adangel/pmd that referenced this issue Jun 22, 2018

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