SONARJAVA-4638 Improve Jakarta support (#4623)

johann-beleites-sonarsource · web-flow · commit 616bfdd924b2 · 2023-12-21T10:29:16.000+01:00
Add support for the Jakarta namespace for the following checks:

- RegexComplexityCheck
- ReluctantQuantifierWithEmptyContinuationCheck
- SecureCookieCheck
- SerializableObjectInSessionCheck
- ServletInstanceFieldCheck
diff --git a/its/autoscan/src/test/java/org/sonar/java/it/AutoScanTest.java b/its/autoscan/src/test/java/org/sonar/java/it/AutoScanTest.java
@@ -181,7 +181,7 @@ public void javaCheckTestSources() throws Exception {
     }
 
     // store new unexpected diffs in JSON files - serializable
-    Files.createDirectory(pathFor(TARGET_ACTUAL + "autoscan-diffs/"));
+    Files.createDirectories(pathFor(TARGET_ACTUAL + "autoscan-diffs/"));
     for (var newDiff : newDiffs) {
       if (!newDiff.equals(knownDiffs.get(newDiff.ruleKey))) {
         Files.writeString(pathFor(TARGET_ACTUAL + "autoscan-diffs/diff_" + newDiff.ruleKey + ".json"), GSON.toJson(newDiff));
diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S1874.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S1874.json
@@ -1,6 +1,6 @@
 {
   "ruleKey": "S1874",
   "hasTruePositives": true,
-  "falseNegatives": 93,
+  "falseNegatives": 111,
   "falsePositives": 0
 }
diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S1948.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S1948.json
@@ -1,6 +1,6 @@
 {
   "ruleKey": "S1948",
   "hasTruePositives": true,
-  "falseNegatives": 0,
+  "falseNegatives": 1,
   "falsePositives": 0
 }
diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2092.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2092.json
@@ -1,6 +1,6 @@
 {
   "ruleKey": "S2092",
   "hasTruePositives": true,
-  "falseNegatives": 42,
+  "falseNegatives": 93,
   "falsePositives": 0
 }
diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2160.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2160.json
@@ -1,6 +1,6 @@
 {
   "ruleKey": "S2160",
   "hasTruePositives": true,
-  "falseNegatives": 1,
+  "falseNegatives": 2,
   "falsePositives": 0
 }
diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2226.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2226.json
@@ -1,6 +1,6 @@
 {
   "ruleKey": "S2226",
   "hasTruePositives": false,
-  "falseNegatives": 5,
+  "falseNegatives": 9,
   "falsePositives": 0
 }
diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S2441.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S2441.json
@@ -1,6 +1,6 @@
 {
   "ruleKey": "S2441",
   "hasTruePositives": true,
-  "falseNegatives": 0,
+  "falseNegatives": 1,
   "falsePositives": 0
 }
diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S3330.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S3330.json
@@ -1,6 +1,6 @@
 {
   "ruleKey": "S3330",
   "hasTruePositives": true,
-  "falseNegatives": 51,
+  "falseNegatives": 77,
   "falsePositives": 0
 }
diff --git a/java-checks-test-sources/default/src/main/java/checks/ServletInstanceFieldCheckJakarta.java b/java-checks-test-sources/default/src/main/java/checks/ServletInstanceFieldCheckJakarta.java
@@ -0,0 +1,81 @@
+package checks;
+
+import java.util.function.Function;
+import jakarta.annotation.Resource;
+import jakarta.servlet.http.HttpServlet;
+import org.apache.struts.action.Action;
+
+class HttpServletAJakarta {
+  private String userName;
+}
+
+class HttpServletBJakarta extends HttpServlet {
+  private String userName; // Noncompliant [[sc=18;ec=26]] {{Remove this misleading mutable servlet instance field or make it "static" and/or "final"}}
+  private static String staticVar;
+  private final String finalVar;
+  private String storageType;
+  private static final Function<Integer, Integer> LAMBDA = lambdaParam -> {
+    Integer lambdaVar = null;
+    return lambdaVar;
+  };
+
+  public HttpServletBJakarta(String x) {
+    String localVar;
+    finalVar = x;
+  }
+
+  public void init(jakarta.servlet.ServletConfig config) {
+    storageType = StorageType.valueOf(config.getInitParameter("storageType"));
+  }
+
+  private static class StorageType {
+    public static String valueOf(String storageType) {
+      return null;
+    }
+  }
+}
+
+class HttpServletCJakarta extends Action {
+
+  private String userName; // Noncompliant
+  private static String staticVar;
+  private final String finalVar;
+
+  public HttpServletCJakarta(String x) {
+    finalVar = x;
+  }
+}
+
+class HttpServletDJakarta extends HttpServlet {
+
+  @jakarta.inject.Inject private String userName; // compliant annotated with inject;
+  @Inject private String userName1; // Noncompliant
+  @Resource private String city; // compliant annotated with resource;
+  private static String staticVar;
+}
+
+public class ServletInstanceFieldCheckJakarta extends HttpServlet {
+  @org.springframework.beans.factory.annotation.Autowired
+  private javax.sql.DataSource myDB; // Noncompliant - filtered by the SpringFilter
+}
+
+class HttpServletEJakarta extends HttpServlet {
+  private String userName; // Noncompliant [[sc=18;ec=26]] {{Remove this misleading mutable servlet instance field or make it "static" and/or "final"}}
+  private final String finalVar;
+  private String storageType; // Compliant, initialized in init() method
+
+  public HttpServletEJakarta(String x) {
+    String localVar;
+    finalVar = x;
+  }
+
+  public void init() {
+    storageType = StorageType.valueOf(getServletConfig().getInitParameter("storageType"));
+  }
+
+  private static class StorageType {
+    public static String valueOf(String storageType) {
+      return null;
+    }
+  }
+}
diff --git a/java-checks-test-sources/default/src/main/java/checks/regex/RegexComplexityCheck.java b/java-checks-test-sources/default/src/main/java/checks/regex/RegexComplexityCheck.java
@@ -16,6 +16,10 @@ public class RegexComplexityCheck {
   @Email(regexp = "((((a|b)|(c|d))+|((e|f)|(g|h))+)+|(((h|i)|(j|j))+|((k|l)|(m|n))+)+)")
   private String email;
 
+  // Noncompliant@+1
+  @jakarta.validation.constraints.Email(regexp = "((((a|b)|(c|d))+|((e|f)|(g|h))+)+|(((h|i)|(j|j))+|((k|l)|(m|n))+)+)")
+  private String emailJakarta;
+
   void noncompliant(String str) {
     // Noncompliant@+2 [[sc=7;ec=8]] {{Simplify this regular expression to reduce its complexity from 106 to the 20 allowed.}}
     str.matches(
diff --git a/java-checks-test-sources/default/src/main/java/checks/regex/ReluctantQuantifierWithEmptyContinuationCheck.java b/java-checks-test-sources/default/src/main/java/checks/regex/ReluctantQuantifierWithEmptyContinuationCheck.java
@@ -50,6 +50,9 @@ void noncompliant(String str) {
   @Email(regexp = ".*?") // Noncompliant {{Remove the '?' from this unnecessarily reluctant quantifier.}}
   void fullMatch() { }
 
+  @jakarta.validation.constraints.Email(regexp = ".*?") // Noncompliant
+  void fullMatchJakarta() { }
+
   Matcher compliant(String str) {
     str.matches(".*?x");
     str.matches(".*?x?");
diff --git a/java-checks-test-sources/default/src/main/java/checks/security/SecureCookieCheck.java b/java-checks-test-sources/default/src/main/java/checks/security/SecureCookieCheck.java
@@ -30,10 +30,10 @@ void foo(Cookie cookie) {
   }
 
   Cookie servletCookie(
-      Cookie firstParam,
-      Cookie secondParam,
-      Cookie thirdParam,
-      boolean param) {
+    Cookie firstParam,
+    Cookie secondParam,
+    Cookie thirdParam,
+    boolean param) {
     firstParam.setSecure(false); // Noncompliant [[sc=25;ec=32]] {{Make sure creating this cookie without the "secure" flag is safe here.}}
     secondParam.setSecure(true);
 
@@ -61,7 +61,7 @@ Cookie servletCookie(
 
     Cookie c7 = new Cookie("name", "value");
     boolean b = false;
-    c7.setSecure(b); // Noncompliant [[secondary=63]]
+    c7.setSecure(b); // Noncompliant [[secondary=-1]]
 
     Cookie c8 = new Cookie("name", "value");
     c8.setSecure(param);
diff --git a/java-checks-test-sources/default/src/main/java/checks/security/SecureCookieCheckJakarta.java b/java-checks-test-sources/default/src/main/java/checks/security/SecureCookieCheckJakarta.java
@@ -0,0 +1,142 @@
+package checks.security;
+
+import java.util.Date;
+import jakarta.servlet.http.Cookie;
+import jakarta.servlet.http.HttpServletResponse;
+import jakarta.ws.rs.core.NewCookie;
+
+class SecureCookieCheckJakarta {
+
+  Cookie field1 = new Cookie("name", "value"); // Noncompliant
+  jakarta.ws.rs.core.Cookie field3 = new jakarta.ws.rs.core.Cookie("name", "value"); // Noncompliant
+  jakarta.ws.rs.core.Cookie cookie;
+  NewCookie secureCookie = new NewCookie(cookie, "2", 3, true);
+  NewCookie unsecureCookie = new NewCookie(cookie, "2", 3, false); // Noncompliant
+  Cookie field4;
+  Cookie field5;
+
+  private static final boolean FALSE_CONSTANT = false;
+
+  void foo(Cookie cookie) {
+  }
+
+  Cookie servletCookie(
+    Cookie firstParam,
+    Cookie secondParam,
+    Cookie thirdParam,
+    boolean param) {
+    firstParam.setSecure(false); // Noncompliant [[sc=25;ec=32]] {{Make sure creating this cookie without the "secure" flag is safe here.}}
+    secondParam.setSecure(true);
+
+    field5.setSecure(false); // Noncompliant
+    this.field4 = new Cookie("name", "value"); // Noncompliant
+
+    Cookie cookie = new Cookie("name", "value");
+    cookie.setSecure(true);
+
+    Cookie cookie2 = new Cookie("name", "value"); // Noncompliant [[sc=26;ec=32]] {{Make sure creating this cookie without the "secure" flag is safe here.}}
+
+    Cookie cookie3 = new Cookie("name", "value");
+    cookie3.setSecure(false);  // Noncompliant {{Make sure creating this cookie without the "secure" flag is safe here.}}
+
+    Cookie cookie5 = new Cookie("name", "value");
+    cookie5.setSecure(FALSE_CONSTANT); // Noncompliant
+
+    Cookie c6 = new Cookie("name", "value");
+    if (param) {
+      c6.setSecure(false); // Noncompliant
+    } else {
+      c6.setSecure(true);
+    }
+
+    Cookie c7 = new Cookie("name", "value");
+    boolean b = false;
+    c7.setSecure(b); // Noncompliant [[secondary=-1]]
+
+    Cookie c8 = new Cookie("name", "value");
+    c8.setSecure(param);
+
+    Object c9 = new Cookie("name", "value"); // Noncompliant
+
+    Cookie c10;
+    c10 = new Cookie("name", "value");
+    c10.setSecure(true);
+
+    Object c12;
+    c12 = new Cookie("name", "value"); // Noncompliant [[sc=15;ec=21]] {{Make sure creating this cookie without the "secure" flag is safe here.}}
+
+    Cookie c13 = new Cookie("name", "value");
+    boolean value = false;
+    c13.setSecure(!value);
+
+    return new Cookie("name", "value"); // Noncompliant
+  }
+
+  NewCookie jaxRsNewCookie(jakarta.ws.rs.core.Cookie cookie) {
+    NewCookie c1 = new NewCookie(cookie); // Noncompliant
+    NewCookie c2 = new NewCookie(cookie, "2", 3, false); // Noncompliant
+    NewCookie c3 = new NewCookie(cookie, "2", 3, true);
+    NewCookie c4 = new NewCookie(cookie, "2", 3, new Date(), false, true); // Noncompliant
+    NewCookie c5 = new NewCookie(cookie, "2", 3, new Date(), true, false);
+
+    NewCookie c6 = new NewCookie("1", "2"); // Noncompliant
+
+    NewCookie c7 = new NewCookie("1", "2", "3", "4", "5", 6, false, true); // Noncompliant
+    NewCookie c8 = new NewCookie("1", "2", "3", "4", "5", 6, true, true);
+    NewCookie c9 = new NewCookie("1", "2", "3", "4", 5, "6", 7, new Date(), false, true);  // Noncompliant
+    NewCookie c10 = new NewCookie("1", "2", "3", "4", 5, "6", 7, new Date(), true, false);
+
+    NewCookie c11 = new NewCookie("1", "2", "3", "4", "5", 6, true);
+    NewCookie c12 = new NewCookie("1", "2", "3", "4", "5", 6, false); // Noncompliant
+    NewCookie c13 = new NewCookie("1", "2", "3", "4", "5", 6, false, false);  // Noncompliant
+    NewCookie c14 = new NewCookie("1", "2", "3", "4", "5", 6, true, false);
+
+    return new NewCookie(cookie); // Noncompliant
+  }
+
+  class SecureCookieCheckBJakarta extends Cookie {
+    public Cookie c;
+
+    public SecureCookieCheckBJakarta(String name, String value) {
+      super(name, value);
+    }
+
+    public void setSecure(boolean bool) {
+    }
+
+    void foo() {
+      setSecure(false); // FN (to avoid implementation complexity)
+    }
+
+    Date d = new Date();
+
+    void bar(boolean x) {
+      setSecure(x);
+    }
+
+    void baz() {
+      setSecure(true);
+      return; // code coverage
+    }
+
+    Date codeCoverage(Cookie cookie) {
+      SecureCookieCheckJakarta a = new SecureCookieCheckJakarta();
+      a.foo(cookie);
+      Date d1 = new Date();
+      Date d2;
+      d2 = d1;
+      d2 = new Date();
+      d = d1;
+      d = new Date();
+      return new Date();
+    }
+
+    class JavaNet {
+      Cookie httpCookie(HttpServletResponse response) {
+        Cookie cookie = new Cookie("name", "value"); // Noncompliant
+        response.addCookie(new Cookie("name", "value")); // Noncompliant
+        return new Cookie("name", "value"); // Noncompliant
+      }
+    }
+  }
+}
diff --git a/java-checks-test-sources/default/src/main/java/checks/serialization/SerializableObjectInSessionCheck.java b/java-checks-test-sources/default/src/main/java/checks/serialization/SerializableObjectInSessionCheck.java
@@ -98,6 +98,12 @@ void foo(HttpServletRequest request) {
     session.setAttribute("name",  notSerializableClass); // Noncompliant {{Make "Class" and its parameters serializable or don't store it in the session.}}
   }
 
+  // Make sure we also cover Jakarta based on one example
+  void jakarta(jakarta.servlet.http.HttpServletRequest request) {
+    var session = request.getSession();
+    session.setAttribute("address", new Address()); // Noncompliant
+  }
+
   public class Address {
   }
   public class Person {
diff --git a/java-checks/src/main/java/org/sonar/java/checks/ServletInstanceFieldCheck.java b/java-checks/src/main/java/org/sonar/java/checks/ServletInstanceFieldCheck.java
diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/SecureCookieCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/SecureCookieCheck.java
diff --git a/java-checks/src/main/java/org/sonar/java/checks/serialization/SerializableObjectInSessionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/serialization/SerializableObjectInSessionCheck.java
diff --git a/java-checks/src/test/java/org/sonar/java/checks/ServletInstanceFieldCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/ServletInstanceFieldCheckTest.java
diff --git a/java-checks/src/test/java/org/sonar/java/checks/security/SecureCookieCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/security/SecureCookieCheckTest.java
diff --git a/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SanityTest.java b/sonar-java-plugin/src/test/java/org/sonar/plugins/java/SanityTest.java

Original file line number	Diff line number	Diff line change
`@@ -181,7 +181,7 @@ public void javaCheckTestSources() throws Exception {`
`181`	`181`	`}`
`182`	`182`
`183`	`183`	`// store new unexpected diffs in JSON files - serializable`
`184`		`- Files.createDirectory(pathFor(TARGET_ACTUAL + "autoscan-diffs/"));`
	`184`	`+ Files.createDirectories(pathFor(TARGET_ACTUAL + "autoscan-diffs/"));`
`185`	`185`	`for (var newDiff : newDiffs) {`
`186`	`186`	`if (!newDiff.equals(knownDiffs.get(newDiff.ruleKey))) {`
`187`	`187`	`Files.writeString(pathFor(TARGET_ACTUAL + "autoscan-diffs/diff_" + newDiff.ruleKey + ".json"), GSON.toJson(newDiff));`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"ruleKey": "S1874",`
`3`	`3`	`"hasTruePositives": true,`
`4`		`- "falseNegatives": 93,`
	`4`	`+ "falseNegatives": 111,`
`5`	`5`	`"falsePositives": 0`
`6`	`6`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"ruleKey": "S1948",`
`3`	`3`	`"hasTruePositives": true,`
`4`		`- "falseNegatives": 0,`
	`4`	`+ "falseNegatives": 1,`
`5`	`5`	`"falsePositives": 0`
`6`	`6`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"ruleKey": "S2092",`
`3`	`3`	`"hasTruePositives": true,`
`4`		`- "falseNegatives": 42,`
	`4`	`+ "falseNegatives": 93,`
`5`	`5`	`"falsePositives": 0`
`6`	`6`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"ruleKey": "S2160",`
`3`	`3`	`"hasTruePositives": true,`
`4`		`- "falseNegatives": 1,`
	`4`	`+ "falseNegatives": 2,`
`5`	`5`	`"falsePositives": 0`
`6`	`6`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"ruleKey": "S2226",`
`3`	`3`	`"hasTruePositives": false,`
`4`		`- "falseNegatives": 5,`
	`4`	`+ "falseNegatives": 9,`
`5`	`5`	`"falsePositives": 0`
`6`	`6`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"ruleKey": "S2441",`
`3`	`3`	`"hasTruePositives": true,`
`4`		`- "falseNegatives": 0,`
	`4`	`+ "falseNegatives": 1,`
`5`	`5`	`"falsePositives": 0`
`6`	`6`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"ruleKey": "S3330",`
`3`	`3`	`"hasTruePositives": true,`
`4`		`- "falseNegatives": 51,`
	`4`	`+ "falseNegatives": 77,`
`5`	`5`	`"falsePositives": 0`
`6`	`6`	`}`