fix LOGBACK-1712

Signed-off-by: Ceki Gulcu <[email protected]>
qos-ch · Dec 13, 2022 · 93443be · 93443be
1 parent 23301c2
commit 93443be
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 10 deletions.
diff --git a/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/Parser.java b/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/Parser.java
@@ -27,7 +27,7 @@
 import ch.qos.logback.core.spi.ContextAwareBase;
 import ch.qos.logback.core.spi.ScanException;
 
-// ~=lamda
+// ~=lambda
 // E = TE|T
 
 // Left factorization

diff --git a/logback-core/src/main/java/ch/qos/logback/core/subst/NodeToStringTransformer.java b/logback-core/src/main/java/ch/qos/logback/core/subst/NodeToStringTransformer.java
@@ -97,13 +97,15 @@ private void handleVariable(Node n, StringBuilder stringBuilder, Stack<Node> cyc
         String key = keyBuffer.toString();
         String value = lookupKey(key);
 
+        // empty values are considered valid
         if (value != null) {
             Node innerNode = tokenizeAndParseString(value);
             compileNode(innerNode, stringBuilder, cycleCheckStack);
             cycleCheckStack.pop();
             return;
         }
 
+        // empty default literal is a valid value
         if (n.defaultPart == null) {
             stringBuilder.append(key + CoreConstants.UNDEFINED_PROPERTY_SUFFIX);
             cycleCheckStack.pop();

diff --git a/logback-core/src/main/java/ch/qos/logback/core/subst/Parser.java b/logback-core/src/main/java/ch/qos/logback/core/subst/Parser.java
@@ -27,6 +27,11 @@
 // V = E|E :- E
 //   = E(':-'E|~)
 
+// new definition
+
+// V = E | E :- Eopt
+//   = E (~| :- Eopt)
+
 /**
  * Parse a token list returning a node chain.
  *
@@ -107,21 +112,25 @@ private Node makeNewLiteralNode(String s) {
         return new Node(Node.Type.LITERAL, s);
     }
 
-    // V = E(':='E|~)
+    // V = E (~| :- Eopt)
     private Node V() throws ScanException {
         Node e = E();
         Node variable = new Node(Node.Type.VARIABLE, e);
         Token t = peekAtCurentToken();
         if (isDefaultToken(t)) {
             advanceTokenPointer();
-            Node def = E();
-            variable.defaultPart = def;
+            Node def = Eopt();
+            if(def != null) {
+                variable.defaultPart = def;
+            } else {
+                variable.defaultPart = makeNewLiteralNode(CoreConstants.EMPTY_STRING);
+            }
         }
         return variable;
     }
 
 
-    // C = E(':='E|~)
+    // C = E(':-'E|~)
     private Node C() throws ScanException {
         Node e0 = E();
         Token t = peekAtCurentToken();

diff --git a/logback-core/src/test/java/ch/qos/logback/core/subst/NodeToStringTransformerTest.java b/logback-core/src/test/java/ch/qos/logback/core/subst/NodeToStringTransformerTest.java
@@ -175,4 +175,22 @@ public void LOGBACK_1101() throws ScanException {
         NodeToStringTransformer nodeToStringTransformer = new NodeToStringTransformer(node, propertyContainer0);
         Assertions.assertEquals("a: {y}", nodeToStringTransformer.transform());
     }
+
+    @Test
+    public void definedAsEmpty() throws ScanException {
+        propertyContainer0.putProperty("empty", "");
+        String input = "a=${empty}";
+        Node node = makeNode(input);
+        NodeToStringTransformer nodeToStringTransformer = new NodeToStringTransformer(node, propertyContainer0);
+        Assertions.assertEquals("a=", nodeToStringTransformer.transform());
+    }
+
+    @Test
+    public void emptyDefault() throws ScanException {
+        propertyContainer0.putProperty("empty", "");
+        String input = "a=${undef:-${empty}}";
+        Node node = makeNode(input);
+        NodeToStringTransformer nodeToStringTransformer = new NodeToStringTransformer(node, propertyContainer0);
+        Assertions.assertEquals("a=", nodeToStringTransformer.transform());
+    }
 }
diff --git a/logback-core/src/test/java/ch/qos/logback/core/subst/ParserTest.java b/logback-core/src/test/java/ch/qos/logback/core/subst/ParserTest.java
@@ -22,8 +22,7 @@
 import static org.junit.jupiter.api.Assertions.fail;
 
 /**
- * Created with IntelliJ IDEA. User: ceki Date: 05.08.12 Time: 00:15 To change
- * this template use File | Settings | File Templates.
+ *
  */
 public class ParserTest {
 
@@ -165,6 +164,17 @@ public void withDefault() throws ScanException {
         assertEquals(witness, node);
     }
 
+    @Test
+    public void withEmptryDefault() throws ScanException {
+        Tokenizer tokenizer = new Tokenizer("${b:-}");
+        Parser parser = new Parser(tokenizer.tokenize());
+        Node node = parser.parse();
+        Node witness = new Node(Node.Type.VARIABLE, new Node(Node.Type.LITERAL, "b"));
+        witness.defaultPart = new Node(Node.Type.LITERAL, "");
+        assertEquals(witness, node);
+    }
+
+
     @Test
     public void defaultSeparatorOutsideOfAVariable() throws ScanException {
         Tokenizer tokenizer = new Tokenizer("{a:-b}");

diff --git a/logback-core/src/test/java/ch/qos/logback/core/util/OptionHelperTest.java b/logback-core/src/test/java/ch/qos/logback/core/util/OptionHelperTest.java
@@ -23,7 +23,9 @@
 import java.util.HashMap;
 import java.util.Map;
 
+import ch.qos.logback.core.testUtil.RandomUtil;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import ch.qos.logback.core.Context;
@@ -39,6 +41,8 @@ public class OptionHelperTest {
     Context context = new ContextBase();
     Map<String, String> secondaryMap;
 
+    int diff = RandomUtil.getPositiveInt();
+
     @BeforeEach
     public void setUp() throws Exception {
         secondaryMap = new HashMap<String, String>();
@@ -308,9 +312,29 @@ public void jackrabbit_standalone() throws ScanException {
     }
 
     @Test
-    public void doesNotThrowNullPointerExceptionForEmptyVariable() throws JoranException, ScanException {
-        context.putProperty("var", "");
-        OptionHelper.substVars("${var}", context);
+    public void emptyVariableIsAccepted() throws JoranException, ScanException {
+        String varName = "var"+diff;
+        context.putProperty(varName, "");
+        String r = OptionHelper.substVars("x ${"+varName+"} b", context);
+        assertEquals("x  b", r);
+    }
+
+    // https://jira.qos.ch/browse/LOGBACK-1012
+    // conflicts with the idea that variables assigned the empty string are valid
+    @Disabled
+    @Test
+    public void defaultExpansionForEmptyVariables() throws JoranException, ScanException {
+        String varName = "var"+diff;
+        context.putProperty(varName, "");
+
+        String r = OptionHelper.substVars("x ${"+varName+":-def} b", context);
+        assertEquals("x def b", r);
+    }
+
+    @Test
+    public void emptyDefault() throws ScanException {
+        String r = OptionHelper.substVars("a${undefinedX:-}b", context);
+        assertEquals("ab", r);
     }
 
     @Test
@@ -332,6 +356,9 @@ public void trailingColon_LOGBACK_1140() throws ScanException {
         assertEquals(prefix + suffix, r);
     }
 
+
+
+
     @Test
     public void curlyBraces_LOGBACK_1101() throws ScanException {
         {