false positive for rule squid:S1989 (JAVA 8 syntax is not supported)

[Modestas Kapusinskas]

Hi,

Rule squid:S1989

False positive:

doesn’t understand JAVA 8 try with resource syntax

https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html

Here is offending code:

try (PrintWriter writer = response.getWriter()) {

            if (!hasAccess(request)) {

                // We need this for Akamai to detect our data centers being up

                writer.println("Version");

            } else {

                VersionUtils.writeVersionInfo(writer);

            }

 

            writer.flush();

        }

KAYAK

Modestas Kapusinskas

Director Engineering

[Nicolas Peru]

Hi, 

Can you specify which version of the java plugin you are using ? Can you also pinpoint the code raising the false positive ? and share the whole offending code as rule S1989 is about throwing exception from a servlet I struggle to see where there is a connection with try with resources (which was introduced in java 7). 

Thanks for clarifying your issue.

Cheers, 

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/F01DAFF8-71E1-4629-88D3-40F5E83273F3%40kayak.com.
For more options, visit https://groups.google.com/d/optout.

--

Nicolas PERU | SonarSource

Senior Developer
http://sonarsource.com

https://twitter.com/benzonico

[Modestas Kapusinskas]

Hi Nicolas,

JAVA Plugin 

• 4.2.1.6971 installed

The code:

@Override

    public void doGet(HttpServletRequest request,

                      HttpServletResponse response) throws IOException, ServletException {

        HeaderUtils.addNoCache(request, response);

        response.setContentType(HttpConstants.TEXT_PLAIN);

        try (PrintWriter writer = response.getWriter()) {

Add a "try/catch" block for "getWriter".   

3 months ago   L49     

Vulnerability   Critical    Resolved (False Positive)   Not assigned  20min effort   Comment

 cert, cwe, error-handling, owasp-a6

            if (!hasAccess(request)) {

                

                writer.println("Version");

            } else {

                VersionUtils.writeVersionInfo(writer);

            }

 

            writer.flush();

        }

    }

The connection is that the rule requires to surround with try catch, even tough it’s already try with resource.

KAYAK

Modestas Kapusinskas

Director Engineering

[Michael Gumowski]

Hello,

I'm not following you on this. As far as I can tell, this is not a FP and the issue is legit. The rule S1989 stands clearly the following:

Even though the signatures for methods in a servlet include throws IOException, ServletException, it's a bad idea to let such exceptions be thrown.

The try-with-resource only closes the resources at the end of its execution. It does not swallow any exception which may be thrown in is resource declaration part. In your case, having the call to "getWriter()" in the resource declaration part of the try-with-resource statement does not prevent any IOException to occur. 

Indeed, according to the signature of the "getWriter()" method, an "IOException" can be thrown when calling it. Consequently, you have to catch the exception, or you are going to let the exception be thrown by the "doGet(...)" method.

Regards,

Michael

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/9786EA9E-FFDA-4B60-A580-490324E3990F%40kayak.com.

For more options, visit https://groups.google.com/d/optout.

--

Michael GUMOWSKI | SonarSource

Software Developer @ Language Team
http://sonarsource.com

[Modestas Kapusinskas]

Hi Michael,

Your explanation makes sense. Thank you!

KAYAK

Modestas Kapusinskas

Director Engineering

<Screenshot 2016-11-17 12.40.14.png><Screenshot 2016-11-17 12.40.14.png>