Merge pull request #1136 from HubSpot/fix-npe-lsong-12-11

Fix NPE as a map key can be null.
HubSpot · Dec 11, 2023 · a525013 · a525013
2 parents ddd35c0 + dcfdc7f
commit a525013
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 6 deletions.
diff --git a/src/main/java/com/hubspot/jinjava/el/ext/CollectionMembershipOperator.java b/src/main/java/com/hubspot/jinjava/el/ext/CollectionMembershipOperator.java
@@ -9,6 +9,7 @@
 import de.odysseus.el.tree.impl.ast.AstBinary.SimpleOperator;
 import de.odysseus.el.tree.impl.ast.AstNode;
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Objects;
@@ -47,14 +48,29 @@ public Object apply(TypeConverter converter, Object o1, Object o2) {
 
     if (Map.class.isAssignableFrom(o2.getClass())) {
       Map map = (Map) o2;
-      if (!map.isEmpty()) {
-        try {
-          Class<?> keyClass = map.keySet().iterator().next().getClass();
-          return map.containsKey(converter.convert(o1, keyClass));
-        } catch (ELException | NoSuchElementException e) {
-          return Boolean.FALSE;
+      if (map.isEmpty()) {
+        return Boolean.FALSE;
+      }
+      Iterator iterator = map.keySet().iterator();
+      Object key = iterator.next();
+      if (key == null) {
+        if (o1 == null) {
+          return Boolean.TRUE;
+        } else {
+          if (iterator.hasNext()) {
+            // Must be non-null this time.
+            key = iterator.next();
+          } else {
+            return Boolean.FALSE;
+          }
         }
       }
+      try {
+        Class<?> keyClass = key.getClass();
+        return map.containsKey(converter.convert(o1, keyClass));
+      } catch (ELException | NoSuchElementException e) {
+        return Boolean.FALSE;
+      }
     }
 
     return Boolean.FALSE;

diff --git a/src/test/java/com/hubspot/jinjava/el/ext/CollectionMembershipOperatorTest.java b/src/test/java/com/hubspot/jinjava/el/ext/CollectionMembershipOperatorTest.java
@@ -4,6 +4,8 @@
 
 import com.hubspot.jinjava.Jinjava;
 import com.hubspot.jinjava.interpret.JinjavaInterpreter;
+import java.util.HashMap;
+import java.util.Map;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -35,4 +37,15 @@ public void itChecksIfDictionaryContainsKey() {
     assertThat(interpreter.resolveELExpression("'c' in {'a': 1, 'b': 2}", -1))
       .isEqualTo(false);
   }
+
+  @Test
+  public void itChecksIfDictionaryContainsNullKey() {
+    Map<String, Object> map = new HashMap();
+    map.put(null, "null");
+    map.put("a", 1);
+    interpreter.getContext().put("map", map);
+    assertThat(interpreter.resolveELExpression("'a' in map", -1)).isEqualTo(true);
+    assertThat(interpreter.resolveELExpression("null in map", -1)).isEqualTo(true);
+    assertThat(interpreter.resolveELExpression("'b' in map", -1)).isEqualTo(false);
+  }
 }