Implement a Regex based Check on Expressions (#1776)

Co-authored-by: Kevin Davis <kdavisk6@gmail.com>

core/src/main/java/feign/template/Expressions.java

@@ -15,7 +15,6 @@ package feign.template;
 
 import feign.Param.Expander;
 import feign.Util;
-import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.regex.Matcher;
@@ -23,8 +22,43 @@ import java.util.regex.Pattern;
 
 public final class Expressions {
 
-  private static final String PATH_STYLE_MODIFIER = ";";
-  private static final Pattern EXPRESSION_PATTERN = Pattern.compile("^([+#./;?&]?)(.*)$");
+  private static final String PATH_STYLE_OPERATOR = ";";
+  /**
+   * Literals may be present and preceded the expression.
+   *
+   * The expression part must start with a '{' and end with a '}'. The contents of the expression
+   * may start with an RFC Operator or the operators reserved by the rfc: Level 2 Operators: '+' and
+   * '#' Level 3 Operators: '.' and '/' and ';' and '?' and '&' Reserved Operators: '=' and ',' and
+   * '!' and '@' and '|'
+   *
+   * The RFC specifies that '{' or '}' or '(' or ')' or'$' is are illegal characters. Feign does not
+   * honor this portion of the RFC Expressions allow '$' characters for Collection expansions, and
+   * all other characters are legal as a regular expression may be passed as a Value Modifier in
+   * Feign
+   *
+   * This is not a complete implementation of the rfc
+   *
+   * <a href="https://www.rfc-editor.org/rfc/rfc6570#section-2.2>RFC 6570 Expressions</a>
+   */
+  private static final Pattern EXPRESSION_PATTERN =
+      Pattern.compile("^(\\{([+#./;?&=,!@|]?)(.+)})$");
+
+  // Partially From:
+  // https://stackoverflow.com/questions/29494608/regex-for-uri-templates-rfc-6570-wanted -- I
+  // suspect much of the codebase could be refactored around the example regex there
+  /**
+   * A pattern for matching possible variable names.
+   *
+   * This pattern accepts characters allowed in RFC 6570 Section 2.3 It also allows the characters
+   * feign has allowed in the past "[]-$"
+   *
+   * The RFC specifies that a variable name followed by a ':' should be a max-length specification.
+   * Feign deviates from the rfc in that the ':' value modifier is used to mark a regular
+   * expression.
+   *
+   */
+  private static final Pattern VARIABLE_LIST_PATTERN = Pattern.compile(
+      "(([\\w-\\[\\]$]|%[0-9A-Fa-f]{2})(\\.?([\\w-\\[\\]$]|%[0-9A-Fa-f]{2}))*(:.*|\\*)?)(,(([\\w-\\[\\]$]|%[0-9A-Fa-f]{2})(\\.?([\\w-\\[\\]$]|%[0-9A-Fa-f]{2}))*(:.*|\\*)?))*");
 
   public static Expression create(final String value) {
 
@@ -37,14 +71,14 @@ public final class Expressions {
     /* create a new regular expression matcher for the expression */
     String variableName = null;
     String variablePattern = null;
-    String modifier = null;
-    Matcher matcher = EXPRESSION_PATTERN.matcher(expression);
+    String operator = null;
+    Matcher matcher = EXPRESSION_PATTERN.matcher(value);
     if (matcher.matches()) {
-      /* grab the modifier */
-      modifier = matcher.group(1).trim();
+      /* grab the operator */
+      operator = matcher.group(2).trim();
 
       /* we have a valid variable expression, extract the name from the first group */
-      variableName = matcher.group(2).trim();
+      variableName = matcher.group(3).trim();
       if (variableName.contains(":")) {
         /* split on the colon */
         String[] parts = variableName.split(":");
@@ -59,13 +93,15 @@ public final class Expressions {
       }
     }
 
-    /* check for a modifier */
-    if (PATH_STYLE_MODIFIER.equalsIgnoreCase(modifier)) {
+    /* check for an operator */
+    if (PATH_STYLE_OPERATOR.equalsIgnoreCase(operator)) {
       return new PathStyleExpression(variableName, variablePattern);
     }
 
     /* default to simple */
-    return new SimpleExpression(variableName, variablePattern);
+    return SimpleExpression.isSimpleExpression(value)
+        ? new SimpleExpression(variableName, variablePattern)
+        : null; // Return null if it can't be validated as a Simple Expression -- Probably a Literal
   }
 
   private static String stripBraces(String expression) {
@@ -180,6 +216,13 @@ public final class Expressions {
       }
       return result.toString();
     }
+
+    protected static boolean isSimpleExpression(String expressionCandidate) {
+      final Matcher matcher = EXPRESSION_PATTERN.matcher(expressionCandidate);
+      return matcher.matches()
+          && matcher.group(2).isEmpty() // Simple Expressions do not support any special operators
+          && VARIABLE_LIST_PATTERN.matcher(matcher.group(3)).matches();
+    }
   }
 
   public static class PathStyleExpression extends SimpleExpression implements Expander {

core/src/test/java/feign/RequestTemplateTest.java

@@ -206,6 +206,31 @@ public class RequestTemplateTest {
         .hasHeaders(entry("Encoded", Collections.singletonList("{{{{dont_expand_me}}")));
   }
 
+  @Test
+  public void resolveTemplateWithHeaderWithJson() {
+    String json = "{ \"string\": \"val\", \"string2\": \"this should not be truncated\"}";
+    RequestTemplate template = new RequestTemplate().method(HttpMethod.GET)
+        .header("A-Header", "{aHeader}");
+
+    template = template.resolve(mapOf("aHeader", json));
+
+    assertThat(template)
+        .hasHeaders(entry("A-Header", Collections.singletonList(json)));
+  }
+
+  @Test
+  public void resolveTemplateWithHeaderWithNestedJson() {
+    String json =
+        "{ \"string\": \"val\", \"string2\": \"this should not be truncated\", \"property\": {\"nested\": true}}";
+    RequestTemplate template = new RequestTemplate().method(HttpMethod.GET)
+        .header("A-Header", "{aHeader}");
+
+    template = template.resolve(mapOf("aHeader", json));
+
+    assertThat(template)
+        .hasHeaders(entry("A-Header", Collections.singletonList(json)));
+  }
+
   /**
    * This ensures we don't mess up vnd types
    */

core/src/test/java/feign/template/HeaderTemplateTest.java

@@ -113,4 +113,17 @@ public class HeaderTemplateTest {
         headerTemplate.expand(
             Collections.singletonMap("expires", "Wed, 4 Jul 2001 12:08:56 -0700")));
   }
+
+  @Test
+  public void it_should_support_json_literal_values() {
+    HeaderTemplate headerTemplate =
+        HeaderTemplate.create("CustomHeader", Collections.singletonList("{jsonParam}"));
+
+    assertEquals("{\"string\": \"val\", \"string2\": \"this should not be truncated\"}",
+        headerTemplate.expand(
+            Collections.singletonMap(
+                "jsonParam",
+                "{\"string\": \"val\", \"string2\": \"this should not be truncated\"}")));
+
+  }
 }