From 93443be2c01b4827d36929e40f437202f659bba9 Mon Sep 17 00:00:00 2001 From: Ceki Gulcu Date: Sun, 11 Dec 2022 22:17:42 +0100 Subject: [PATCH] fix LOGBACK-1712 Signed-off-by: Ceki Gulcu --- .../logback/core/pattern/parser/Parser.java | 2 +- .../core/subst/NodeToStringTransformer.java | 2 ++ .../ch/qos/logback/core/subst/Parser.java | 17 +++++++--- .../subst/NodeToStringTransformerTest.java | 18 ++++++++++ .../ch/qos/logback/core/subst/ParserTest.java | 14 ++++++-- .../logback/core/util/OptionHelperTest.java | 33 +++++++++++++++++-- 6 files changed, 76 insertions(+), 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 index 93c97bae5c..39f0d74c18 100644 --- 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 index a6e715c3ab..28f1af61cc 100755 --- 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,6 +97,7 @@ private void handleVariable(Node n, StringBuilder stringBuilder, Stack 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); @@ -104,6 +105,7 @@ private void handleVariable(Node n, StringBuilder stringBuilder, Stack cyc 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 index 8c2f17ea7c..8a38bb1293 100755 --- 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 index 98f7a4dcfc..b9e3403d48 100644 --- 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 index 16da82df5d..70c1d5a7f4 100644 --- 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 index ac5897a88a..4c1e97dd20 100755 --- 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 secondaryMap; + int diff = RandomUtil.getPositiveInt(); + @BeforeEach public void setUp() throws Exception { secondaryMap = new HashMap(); @@ -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 { {