From 4a8d4b9a180968a98777d11c378b7e1b767b9a66 Mon Sep 17 00:00:00 2001 From: Connor Taffe Date: Tue, 10 Dec 2019 10:54:27 -0600 Subject: [PATCH 1/7] Present friendlier error for non-static main - When executing a jar with RunJar, first check whether the method is static and if not throw an error. This prevents a cryptic NullPointerException from being raised within Method.invoke's implementation --- .../src/main/java/org/apache/hadoop/util/RunJar.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/RunJar.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/RunJar.java index 50126002b7be7..ea1cc40a96a68 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/RunJar.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/RunJar.java @@ -24,6 +24,7 @@ import java.io.OutputStream; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; @@ -315,6 +316,11 @@ public void run() { Thread.currentThread().setContextClassLoader(loader); Class mainClass = Class.forName(mainClassName, true, loader); Method main = mainClass.getMethod("main", String[].class); + + if (!Modifier.isStatic(main.getModifiers())) { + throw new IOException("Method main must be static: " + mainClassName); + } + List newArgsSubList = Arrays.asList(args) .subList(firstArg, args.length); String[] newArgs = newArgsSubList From 869f8dc61fe1ac94a9018bf46fd9cb94ac9ffbdb Mon Sep 17 00:00:00 2001 From: Connor Taffe Date: Tue, 10 Dec 2019 13:58:38 -0600 Subject: [PATCH 2/7] Test that exception is thrown when encountering non-static main --- .../org/apache/hadoop/util/NonStaticMain.java | 23 +++++++++++++ .../org/apache/hadoop/util/TestRunJar.java | 33 ++++++++++++++++++- 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/NonStaticMain.java diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/NonStaticMain.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/NonStaticMain.java new file mode 100644 index 0000000000000..a11e3312e5cf2 --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/NonStaticMain.java @@ -0,0 +1,23 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.util; + +public class NonStaticMain { + public void main(String[] args) { + } +} diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java index 1f7c71222fbfe..fd98ebe7cf274 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java @@ -297,4 +297,35 @@ public void testUnJar2() throws IOException { "would create file outside of", e); } } -} \ No newline at end of file + + /** + * Tests that RunJar errors appropriately for classes with non-static main + * methods. + */ + public void testRunNonStaticMain() throws Throwable { + RunJar runJar = spy(new RunJar()); + // enable the client classloader + when(runJar.useClientClassLoader()).thenReturn(true); + // set the system classes and blacklist the test main class and the test + // third class so they can be loaded by the application classloader + String mainCls = NonStaticMain.class.getName(); + String systemClasses = "-" + mainCls + "," + ApplicationClassLoader.SYSTEM_CLASSES_DEFAULT; + when(runJar.getSystemClasses()).thenReturn(systemClasses); + + // create the test jar + File testJar = JarFinder.makeClassLoaderTestJar(this.getClass(), TEST_ROOT_DIR, TEST_JAR_2_NAME, BUFF_SIZE, + mainCls); + // form the args + String[] args = new String[3]; + args[0] = testJar.getAbsolutePath(); + args[1] = mainCls; + + // run RunJar + try { + runJar.run(args); + fail("run should throw IOException."); + } catch (IOException e) { + GenericTestUtils.assertExceptionContains("Method main must be static", e); + } + } +} From fa56d95a96a7663348429e54ac8e7ff65f73d5dd Mon Sep 17 00:00:00 2001 From: Connor Taffe Date: Wed, 11 Dec 2019 17:17:01 -0600 Subject: [PATCH 3/7] Style, use LambdaTestUtils.intercept --- .../org/apache/hadoop/util/NonStaticMain.java | 1 + .../org/apache/hadoop/util/TestRunJar.java | 22 ++++++++----------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/NonStaticMain.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/NonStaticMain.java index a11e3312e5cf2..5ce50cc2bd8e6 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/NonStaticMain.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/NonStaticMain.java @@ -18,6 +18,7 @@ package org.apache.hadoop.util; public class NonStaticMain { + public void main(String[] args) { } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java index fd98ebe7cf274..cde1c284cf782 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.util; +import static org.apache.hadoop.test.LambdaTestUtils.intercept; import static org.apache.hadoop.util.RunJar.MATCH_ANY; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -57,7 +58,7 @@ public class TestRunJar { private static final int BUFF_SIZE = 2048; private File TEST_ROOT_DIR; - private static final String TEST_JAR_NAME="test-runjar.jar"; + private static final String TEST_JAR_NAME = "test-runjar.jar"; private static final String TEST_JAR_2_NAME = "test-runjar2.jar"; private static final long MOCKED_NOW = 1_460_389_972_000L; private static final long MOCKED_NOW_PLUS_TWO_SEC = MOCKED_NOW + 2_000; @@ -309,23 +310,18 @@ public void testRunNonStaticMain() throws Throwable { // set the system classes and blacklist the test main class and the test // third class so they can be loaded by the application classloader String mainCls = NonStaticMain.class.getName(); - String systemClasses = "-" + mainCls + "," + ApplicationClassLoader.SYSTEM_CLASSES_DEFAULT; + String systemClasses = "-" + mainCls + "," + + ApplicationClassLoader.SYSTEM_CLASSES_DEFAULT; when(runJar.getSystemClasses()).thenReturn(systemClasses); // create the test jar - File testJar = JarFinder.makeClassLoaderTestJar(this.getClass(), TEST_ROOT_DIR, TEST_JAR_2_NAME, BUFF_SIZE, - mainCls); + File testJar = JarFinder.makeClassLoaderTestJar(this.getClass(), + TEST_ROOT_DIR, TEST_JAR_2_NAME, BUFF_SIZE, mainCls); // form the args - String[] args = new String[3]; - args[0] = testJar.getAbsolutePath(); - args[1] = mainCls; + String[] args = new String[] { testJar.getAbsolutePath(), mainCls }; // run RunJar - try { - runJar.run(args); - fail("run should throw IOException."); - } catch (IOException e) { - GenericTestUtils.assertExceptionContains("Method main must be static", e); - } + IOException e = intercept(IOException.class, + "Method main must be static", () -> runJar.run(args)); } } From 2ab04aee74e82da83006beaca67824c707affbc7 Mon Sep 17 00:00:00 2001 From: Connor Taffe Date: Wed, 11 Dec 2019 21:20:31 -0600 Subject: [PATCH 4/7] Use try/catch instead of LambaTestUtils#intercept - Since RunJar#run throws Throwable, it doesn't wrap cleanly with Callable, which throws Exception. This is because IOException subclasses Throwable. Since LambdaTestUtils#intercept expects a Callable, it can't be used. --- .../src/test/java/org/apache/hadoop/util/TestRunJar.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java index cde1c284cf782..1225ad6876b53 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java @@ -17,7 +17,6 @@ */ package org.apache.hadoop.util; -import static org.apache.hadoop.test.LambdaTestUtils.intercept; import static org.apache.hadoop.util.RunJar.MATCH_ANY; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -321,7 +320,11 @@ public void testRunNonStaticMain() throws Throwable { String[] args = new String[] { testJar.getAbsolutePath(), mainCls }; // run RunJar - IOException e = intercept(IOException.class, - "Method main must be static", () -> runJar.run(args)); + try { + runJar.run(args); + fail("run should throw IOException."); + } catch (IOException e) { + GenericTestUtils.assertExceptionContains("Method main must be static", e); + } } } From 56166722a78dd1386a1e9cf7e2cfe66bf65e72ee Mon Sep 17 00:00:00 2001 From: Connor Taffe Date: Wed, 11 Dec 2019 21:24:36 -0600 Subject: [PATCH 5/7] Style - Fix style issues found by checkstyle --- .../test/java/org/apache/hadoop/util/NonStaticMain.java | 9 ++++++--- .../src/test/java/org/apache/hadoop/util/TestRunJar.java | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/NonStaticMain.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/NonStaticMain.java index 5ce50cc2bd8e6..d22915b3df494 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/NonStaticMain.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/NonStaticMain.java @@ -17,8 +17,11 @@ */ package org.apache.hadoop.util; +/** + * NonStaticMain is used to test classes which provide a main which + * isn't static. + */ public class NonStaticMain { - - public void main(String[] args) { - } + public void main(String[] args) { + } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java index 1225ad6876b53..923d954a424a7 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java @@ -317,7 +317,7 @@ public void testRunNonStaticMain() throws Throwable { File testJar = JarFinder.makeClassLoaderTestJar(this.getClass(), TEST_ROOT_DIR, TEST_JAR_2_NAME, BUFF_SIZE, mainCls); // form the args - String[] args = new String[] { testJar.getAbsolutePath(), mainCls }; + String[] args = new String[] {testJar.getAbsolutePath(), mainCls}; // run RunJar try { From 93b909ef70994036abc6b12b6cb565a30ae6efa6 Mon Sep 17 00:00:00 2001 From: Connor Taffe Date: Thu, 12 Dec 2019 09:42:00 -0600 Subject: [PATCH 6/7] Add missing @Test annotation --- .../src/test/java/org/apache/hadoop/util/TestRunJar.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java index 923d954a424a7..79f453ed7287d 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java @@ -302,6 +302,7 @@ public void testUnJar2() throws IOException { * Tests that RunJar errors appropriately for classes with non-static main * methods. */ + @Test public void testRunNonStaticMain() throws Throwable { RunJar runJar = spy(new RunJar()); // enable the client classloader From 415418faabba1712e964af9e20bfbe6708630b15 Mon Sep 17 00:00:00 2001 From: Connor Taffe Date: Thu, 12 Dec 2019 09:48:27 -0600 Subject: [PATCH 7/7] Fix comment --- .../src/test/java/org/apache/hadoop/util/TestRunJar.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java index 79f453ed7287d..c7e3fde3d7913 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java @@ -307,8 +307,8 @@ public void testRunNonStaticMain() throws Throwable { RunJar runJar = spy(new RunJar()); // enable the client classloader when(runJar.useClientClassLoader()).thenReturn(true); - // set the system classes and blacklist the test main class and the test - // third class so they can be loaded by the application classloader + // set the system classes and blacklist the test main class so it + // can be loaded by the application classloader String mainCls = NonStaticMain.class.getName(); String systemClasses = "-" + mainCls + "," + ApplicationClassLoader.SYSTEM_CLASSES_DEFAULT;