Fix bugs in Painless SCatch node (#45880)

This fixes two bugs: - A recently introduced bug where an NPE will be thrown if a catch block is empty. - A long-time bug where an NPE will be thrown if multiple catch blocks in a row are empty for the same try block.
elastic · Aug 23, 2019 · 45ad01a · 45ad01a
1 parent ceb8b9b
commit 45ad01a
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 2 deletions.
diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SCatch.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/SCatch.java
@@ -56,7 +56,9 @@ public SCatch(Location location, String type, String name, SBlock block) {
 
     @Override
     void storeSettings(CompilerSettings settings) {
-        block.storeSettings(settings);
+        if (block != null) {
+            block.storeSettings(settings);
+        }
     }
 
     @Override
@@ -115,7 +117,7 @@ void write(MethodWriter writer, Globals globals) {
 
         writer.visitTryCatchBlock(begin, end, jump, MethodWriter.getType(variable.clazz).getInternalName());
 
-        if (exception != null && !block.allEscape) {
+        if (exception != null && (block == null || !block.allEscape)) {
             writer.goTo(exception);
         }
     }

diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/TryCatchTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/TryCatchTests.java
@@ -55,4 +55,47 @@ public void testNoCatch() {
         });
         assertEquals("test", exception.getMessage());
     }
+
+    public void testNoCatchBlock() {
+        assertEquals(0, exec("try { return Integer.parseInt('f') } catch (NumberFormatException nfe) {} return 0;"));
+
+        assertEquals(0, exec("try { return Integer.parseInt('f') } " +
+                "catch (NumberFormatException nfe) {}" +
+                "catch (Exception e) {}" +
+                " return 0;"));
+
+        assertEquals(0, exec("try { throw new IllegalArgumentException('test') } " +
+                "catch (NumberFormatException nfe) {}" +
+                "catch (Exception e) {}" +
+                " return 0;"));
+
+        assertEquals(0, exec("try { throw new IllegalArgumentException('test') } " +
+                "catch (NumberFormatException nfe) {}" +
+                "catch (IllegalArgumentException iae) {}" +
+                "catch (Exception e) {}" +
+                " return 0;"));
+    }
+
+    public void testMultiCatch() {
+        assertEquals(1, exec(
+                "try { return Integer.parseInt('f') } " +
+                "catch (NumberFormatException nfe) {return 1;} " +
+                "catch (ArrayIndexOutOfBoundsException aioobe) {return 2;} " +
+                "catch (Exception e) {return 3;}"
+        ));
+
+        assertEquals(2, exec(
+                "try { return new int[] {}[0] } " +
+                "catch (NumberFormatException nfe) {return 1;} " +
+                "catch (ArrayIndexOutOfBoundsException aioobe) {return 2;} " +
+                "catch (Exception e) {return 3;}"
+        ));
+
+        assertEquals(3, exec(
+                "try { throw new IllegalArgumentException('test'); } " +
+                "catch (NumberFormatException nfe) {return 1;} " +
+                "catch (ArrayIndexOutOfBoundsException aioobe) {return 2;} " +
+                "catch (Exception e) {return 3;}"
+        ));
+    }
 }