SONARJAVA-4741 FP on S6857 for special default values in property pla…

…ceholders (#4766)
SonarSource · Apr 15, 2024 · 3f65e31 · 3f65e31
1 parent a7b0c31
commit 3f65e31
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 38 deletions.
diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/SpelExpressionCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/spring/SpelExpressionCheckSample.java
@@ -60,55 +60,64 @@ public class SpelExpressionCheckSample {
   private String region4;
 
   @Value("${user.region:defaultRegion}") // Compliant
-  private String multi1;
+  private String default1;
 
   @Value("${user.region::defaultRegion}") // Noncompliant {{Correct this malformed property placeholder.}}
-  private String multi2;
+  private String default2;
 
   @Value("${:user.region:defaultRegion}") // Noncompliant {{Correct this malformed property placeholder.}}
-  private String multi3;
+  private String default3;
 
-  @Value("${user.region:defaultRegion:}") // Noncompliant {{Correct this malformed property placeholder.}}
-  private String multi4;
+  @Value("${user.region:defaultRegion:}") // Noncompliant
+  private String default4;
 
   @Value("${  user.region  : defaultRegion  }") // Compliant
-  private String multi5;
+  private String default5;
 
   @Value("${user.region:#{null}}") // Compliant
-  private String multi6;
+  private String default6;
 
   @Value("${user.region:#{  null  }}") // Compliant
-  private String multi7;
+  private String default7;
 
   @Value("${user.region:#{  null + 3 }}") // Compliant
-  private String multi8;
+  private String default8;
 
   @Value("${user.region:#{  null + * 3 }}") // Noncompliant [[sc=25;ec=41]] {{Correct this malformed SpEL expression.}}
-  private String multi9;
+  private String default9;
 
   @Value("${user.region:#{'D'+'E'}}") // Compliant
-  private String multi10;
+  private String default10;
 
-  @Value("${user.region:#{null}:#{null}:foo.bar}") // Compliant
-  private String multi11;
+  @Value("${user.region:#{null}:#{null}:foo.bar}") // Noncompliant
+  private String default11;
 
-  @Value("${user.region:#{null}:#{4**4}:foo.bar}") // Noncompliant [[sc=33;ec=40]] {{Correct this malformed SpEL expression.}}
-  private String multi12;
+  @Value("${user.region:#{null}:#{4**4}:foo.bar}") // Noncompliant [[sc=11;ec=49]] {{Correct this malformed property placeholder.}}
+  private String default12;
 
-  @Value("${user.region:#{null}:#{4*4}:foo.bar}") // Compliant
-  private String multi13;
+  @Value("${user.region:#{4**4}:#{null}:foo.bar}") // Noncompliant [[sc=25;ec=32]] {{Correct this malformed SpEL expression.}}
+  private String default13;
 
-  @Value("${user.region:#{null}:#{4*4}:foo..bar}") // Noncompliant
-  private String multi14;
-
-  @Value("${user.region:#{4**4}:#{4**4}:foo.bar}") // Noncompliant
-  private String multi15;
+  @Value("${user.2region:default-region}") // Compliant
+  private String default14;
 
   @Value("${:defaultRegion}") // Noncompliant
-  private String multi16;
+  private String default15;
 
-  @Value("${user.2region:default-region}") // Compliant
-  private String multi17;
+  @Value("${user.region:}") // Compliant
+  private String emptyDefaultValue;
+
+  @Value("${foo.bar:0 0 * 8 b c}") // Compliant
+  private String defaultValueStringContents;
+
+  @Value("${server.error.path:${error.path}}") // Compliant
+  private String nestedPropertyValue1;
+
+  @Value("${server.error.path:${error.path}    }") // Compliant
+  private String checkTrimEMptyEnd;
+
+  @Value("${server.error.path:${error.path:defaultErrorValue}}") // Compliant
+  private String nestedPropertyValue2;
 
   @Value("#{'${listOfValues}' split(',')}") // Noncompliant [[sc=11;ec=42]] {{Correct this malformed SpEL expression.}}
   private List<String> valuesListNc;

diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpelExpressionCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpelExpressionCheck.java
@@ -238,22 +238,14 @@ private static void parseValidPropertyPlaceholder(String placeholder, int startC
   }
 
   private static boolean isValidPropertyPlaceholder(String placeholder, int startColumn) throws SyntaxError {
-    var startIndex = 0;
-    var endIndex = placeholder.indexOf(':');
-
-    while (endIndex != -1) {
-      var segment = placeholder.substring(startIndex, endIndex);
-      if (!isValidPropertyPlaceholderSegment(segment, startColumn + startIndex)) {
-        return false;
-      }
-      startIndex = endIndex + 1;
-      endIndex = placeholder.indexOf(':', startIndex);
+    var segments = placeholder.split(":",2);
+    if (!isValidPropertyPlaceholderFirstSegment(segments[0], startColumn)) {
+      return false;
     }
-    var segment = placeholder.substring(startIndex);
-    return isValidPropertyPlaceholderSegment(segment, startColumn + startIndex);
+    return segments.length < 2 || (isValidPropertyPlaceholderDefaultSegment(segments[1], startColumn + segments[0].length() + 1));
   }
 
-  private static boolean isValidPropertyPlaceholderSegment(String segment, int startColumn) throws SyntaxError {
+  private static boolean isValidPropertyPlaceholderFirstSegment(String segment, int startColumn) throws SyntaxError {
     var stripped = segment.stripLeading();
     startColumn += segment.length() - stripped.length();
     stripped = stripped.stripTrailing();
@@ -266,6 +258,29 @@ private static boolean isValidPropertyPlaceholderSegment(String segment, int sta
     }
   }
 
+  private static boolean isValidPropertyPlaceholderDefaultSegment(String segment, int startColumn) throws SyntaxError {
+    var stripped = segment.stripLeading();
+    startColumn += segment.length() - stripped.length();
+    stripped = stripped.stripTrailing();
+
+    var contentsParser = getContentsParser(stripped);
+    if (contentsParser != null) {
+      var endIndex = parseDelimitersAndContents(stripped, 1, startColumn + 2, contentsParser);
+      return endIndex == segment.stripTrailing().length();
+    }
+    return segment.indexOf(':') < 0;
+  }
+
+  private static ObjIntConsumer<String> getContentsParser(String contents) {
+    if (contents.startsWith("${")) {
+      return SpelExpressionCheck::parseValidPropertyPlaceholder;
+    }
+    if (contents.startsWith("#{")) {
+      return SpelExpressionCheck::parseValidSpelExpression;
+    }
+    return null;
+  }
+
   private static void parseValidSpelExpression(String expressionString, int startColumn) throws SyntaxError {
     if (!isValidSpelExpression(expressionString)) {
       // +3 because of delimiter `${` and `}`