Fix NPE when substituting blank variables

The substitution parser attempted to deref a null pointer when a given variable had a blank value. c.q.l.core.subst.Parser#T() was missing a null-check. This was discovered in logback-android's AndroidManifestPropertiesUtil.
qos-ch · Feb 7, 2014 · 9ac1e28 · 9ac1e28
1 parent cef354f
commit 9ac1e28
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 4 deletions.
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
@@ -71,6 +71,9 @@ private Node Eopt() throws ScanException {
   // T = LITERAL | '${' V '}'
   private Node T() throws ScanException {
     Token t = peekAtCurentToken();
+    if (t == null) {
+      return null;
+    }
 
     switch (t.type) {
       case LITERAL:

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
@@ -13,10 +13,13 @@
  */
 package ch.qos.logback.core.subst;
 
-import ch.qos.logback.core.spi.ScanException;
+import static org.junit.Assert.assertEquals;
+
+import java.util.ArrayList;
+
 import org.junit.Test;
 
-import static org.junit.Assert.assertEquals;
+import ch.qos.logback.core.spi.ScanException;
 
 /**
  * Created with IntelliJ IDEA.
@@ -176,6 +179,15 @@ public void defaultSeparatorOutsideOfAVariable() throws ScanException {
     assertEquals(witness, node);
   }
 
+  @Test
+  public void emptyTokenListDoesNotThrowNullPointerException() throws ScanException {
+    // An empty token list would be returned from Tokenizer.tokenize()
+    // if it were constructed with an empty string. The parser should
+    // be able to handle this.
+    Parser parser = new Parser(new ArrayList<Token>());
+    parser.parse();
+  }
+
   private void dump(Node node) {
     while (node != null) {
       System.out.println(node.toString());

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
@@ -22,10 +22,11 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import ch.qos.logback.core.Context;
 import ch.qos.logback.core.ContextBase;
-import org.junit.rules.ExpectedException;
+import ch.qos.logback.core.joran.spi.JoranException;
 
 
 public class OptionHelperTest  {
@@ -241,12 +242,16 @@ public void defaultValueReferencingAVariable() {
     assertEquals("k1", result);
   }
 
-
   @Test
   public void jackrabbit_standalone() {
     String r = OptionHelper.substVars("${jackrabbit.log:-${repo:-jackrabbit}/log/jackrabbit.log}", context);
     assertEquals("jackrabbit/log/jackrabbit.log", r);
   }
 
+  @Test
+  public void doesNotThrowNullPointerExceptionForEmptyVariable() throws JoranException {
+    context.putProperty("var", "");
+    OptionHelper.substVars("${var}", context);
+  }
 
 }