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 · e7c8c3f · e7c8c3f
1 parent 30e62a5
commit e7c8c3f
Show file tree

Hide file tree

Showing 2 changed files with 44 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
@@ -53,7 +53,6 @@ public SCatch(Location location, String type, String name, SBlock block) {
         this.block = block;
     }
 
-    @Override
     void extractVariables(Set<String> variables) {
         variables.add(name);
 
@@ -109,7 +108,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;}"
+        ));
+    }
 }