HADOOP-19079. Check class is an exception class before constructing an instance

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/HttpExceptionUtils.java

@@ -26,7 +26,9 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Writer;
-import java.lang.reflect.Constructor;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
 import java.net.HttpURLConnection;
 import java.util.Collections;
 import java.util.LinkedHashMap;
@@ -54,6 +56,10 @@ public class HttpExceptionUtils {
 
   private static final String ENTER = System.getProperty("line.separator");
 
+  private static final MethodHandles.Lookup PUBLIC_LOOKUP = MethodHandles.publicLookup();
+  private static final MethodType EXCEPTION_CONSTRUCTOR_TYPE =
+          MethodType.methodType(void.class, String.class);
+
   /**
    * Creates a HTTP servlet response serializing the exception in it as JSON.
    *
@@ -150,9 +156,12 @@ public static void validateResponse(HttpURLConnection conn,
           try {
             ClassLoader cl = HttpExceptionUtils.class.getClassLoader();
             Class klass = cl.loadClass(exClass);
-            Constructor constr = klass.getConstructor(String.class);
-            toThrow = (Exception) constr.newInstance(exMsg);
-          } catch (Exception ex) {
+            Preconditions.checkState(Exception.class.isAssignableFrom(klass),
+                "Class [%s] is not a subclass of Exception", klass);
+            MethodHandle methodHandle = PUBLIC_LOOKUP.findConstructor(
+                    klass, EXCEPTION_CONSTRUCTOR_TYPE);
+            toThrow = (Exception) methodHandle.invoke(exMsg);
+          } catch (Throwable t) {
             toThrow = new IOException(String.format(
                 "HTTP status [%d], exception [%s], message [%s], URL [%s]",
                 conn.getResponseCode(), exClass, exMsg, conn.getURL()));

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/impl/prefetch/TestBlockCache.java

@@ -54,7 +54,7 @@ public void testArgChecks() throws Exception {
         () -> cache.put(42, null, null, null));
 
 
-    intercept(NullPointerException.class, null,
+    intercept(NullPointerException.class,
         () -> new SingleFilePerBlockCache(null, 2, null));
 
   }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/LambdaTestUtils.java

@@ -528,7 +528,7 @@ public static <E extends Throwable> E intercept(
       throws Exception {
     return intercept(clazz, contained,
         "Expecting " + clazz.getName()
-        + (contained != null? (" with text " + contained) : "")
+        + (contained != null ? (" with text " + contained) : "")
         + " but got ",
         () -> {
           eval.call();
@@ -1037,3 +1037,4 @@ public Void run() throws Exception {
     }
   }
 }
+

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestHttpExceptionUtils.java

@@ -18,6 +18,7 @@
 package org.apache.hadoop.util;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hadoop.test.LambdaTestUtils;
 import org.junit.Assert;
 import org.junit.Test;
 import org.mockito.Mockito;
@@ -31,6 +32,7 @@
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.net.HttpURLConnection;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
@@ -82,40 +84,35 @@ public void testCreateJerseyException() throws IOException {
   @Test
   public void testValidateResponseOK() throws IOException {
     HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
-    Mockito.when(conn.getResponseCode()).thenReturn(
-        HttpURLConnection.HTTP_CREATED);
+    Mockito.when(conn.getResponseCode()).thenReturn(HttpURLConnection.HTTP_CREATED);
     HttpExceptionUtils.validateResponse(conn, HttpURLConnection.HTTP_CREATED);
   }
 
-  @Test(expected = IOException.class)
-  public void testValidateResponseFailNoErrorMessage() throws IOException {
+  @Test
+  public void testValidateResponseFailNoErrorMessage() throws Exception {
     HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
-    Mockito.when(conn.getResponseCode()).thenReturn(
-        HttpURLConnection.HTTP_BAD_REQUEST);
-    HttpExceptionUtils.validateResponse(conn, HttpURLConnection.HTTP_CREATED);
+    Mockito.when(conn.getResponseCode()).thenReturn(HttpURLConnection.HTTP_BAD_REQUEST);
+    LambdaTestUtils.intercept(IOException.class,
+        () -> HttpExceptionUtils.validateResponse(conn, HttpURLConnection.HTTP_CREATED));
   }
 
   @Test
-  public void testValidateResponseNonJsonErrorMessage() throws IOException {
+  public void testValidateResponseNonJsonErrorMessage() throws Exception {
     String msg = "stream";
-    InputStream is = new ByteArrayInputStream(msg.getBytes());
+    InputStream is = new ByteArrayInputStream(msg.getBytes(StandardCharsets.UTF_8));
     HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
     Mockito.when(conn.getErrorStream()).thenReturn(is);
     Mockito.when(conn.getResponseMessage()).thenReturn("msg");
-    Mockito.when(conn.getResponseCode()).thenReturn(
-        HttpURLConnection.HTTP_BAD_REQUEST);
-    try {
-      HttpExceptionUtils.validateResponse(conn, HttpURLConnection.HTTP_CREATED);
-      Assert.fail();
-    } catch (IOException ex) {
-      Assert.assertTrue(ex.getMessage().contains("msg"));
-      Assert.assertTrue(ex.getMessage().contains("" +
-          HttpURLConnection.HTTP_BAD_REQUEST));
-    }
+    Mockito.when(conn.getResponseCode()).thenReturn(HttpURLConnection.HTTP_BAD_REQUEST);
+    IOException ex = LambdaTestUtils.intercept(IOException.class,
+        () -> HttpExceptionUtils.validateResponse(conn, HttpURLConnection.HTTP_CREATED));
+    assertContains(Integer.toString(HttpURLConnection.HTTP_BAD_REQUEST), ex.getMessage());
+    assertContains("msg", ex.getMessage());
+    assertContains("com.fasterxml.jackson.core.JsonParseException", ex.getMessage());
   }
 
   @Test
-  public void testValidateResponseJsonErrorKnownException() throws IOException {
+  public void testValidateResponseJsonErrorKnownException() throws Exception {
     Map<String, Object> json = new HashMap<String, Object>();
     json.put(HttpExceptionUtils.ERROR_EXCEPTION_JSON, IllegalStateException.class.getSimpleName());
     json.put(HttpExceptionUtils.ERROR_CLASSNAME_JSON, IllegalStateException.class.getName());
@@ -124,23 +121,19 @@ public void testValidateResponseJsonErrorKnownException() throws IOException {
     response.put(HttpExceptionUtils.ERROR_JSON, json);
     ObjectMapper jsonMapper = new ObjectMapper();
     String msg = jsonMapper.writeValueAsString(response);
-    InputStream is = new ByteArrayInputStream(msg.getBytes());
+    InputStream is = new ByteArrayInputStream(msg.getBytes(StandardCharsets.UTF_8));
     HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
     Mockito.when(conn.getErrorStream()).thenReturn(is);
     Mockito.when(conn.getResponseMessage()).thenReturn("msg");
-    Mockito.when(conn.getResponseCode()).thenReturn(
-        HttpURLConnection.HTTP_BAD_REQUEST);
-    try {
-      HttpExceptionUtils.validateResponse(conn, HttpURLConnection.HTTP_CREATED);
-      Assert.fail();
-    } catch (IllegalStateException ex) {
-      Assert.assertEquals("EX", ex.getMessage());
-    }
+    Mockito.when(conn.getResponseCode()).thenReturn(HttpURLConnection.HTTP_BAD_REQUEST);
+    LambdaTestUtils.intercept(IllegalStateException.class,
+        "EX",
+        () -> HttpExceptionUtils.validateResponse(conn, HttpURLConnection.HTTP_CREATED));
   }
 
   @Test
   public void testValidateResponseJsonErrorUnknownException()
-      throws IOException {
+      throws Exception {
     Map<String, Object> json = new HashMap<String, Object>();
     json.put(HttpExceptionUtils.ERROR_EXCEPTION_JSON, "FooException");
     json.put(HttpExceptionUtils.ERROR_CLASSNAME_JSON, "foo.FooException");
@@ -149,19 +142,44 @@ public void testValidateResponseJsonErrorUnknownException()
     response.put(HttpExceptionUtils.ERROR_JSON, json);
     ObjectMapper jsonMapper = new ObjectMapper();
     String msg = jsonMapper.writeValueAsString(response);
-    InputStream is = new ByteArrayInputStream(msg.getBytes());
+    InputStream is = new ByteArrayInputStream(msg.getBytes(StandardCharsets.UTF_8));
+    HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
+    Mockito.when(conn.getErrorStream()).thenReturn(is);
+    Mockito.when(conn.getResponseMessage()).thenReturn("msg");
+    Mockito.when(conn.getResponseCode()).thenReturn(HttpURLConnection.HTTP_BAD_REQUEST);
+    IOException ex = LambdaTestUtils.intercept(IOException.class,
+        () -> HttpExceptionUtils.validateResponse(conn, HttpURLConnection.HTTP_CREATED));
+    assertContains(Integer.toString(HttpURLConnection.HTTP_BAD_REQUEST), ex.getMessage());
+    assertContains("foo.FooException", ex.getMessage());
+    assertContains("EX", ex.getMessage());
+  }
+
+  @Test
+  public void testValidateResponseJsonErrorNonException() throws Exception {
+    Map<String, Object> json = new HashMap<String, Object>();
+    json.put(HttpExceptionUtils.ERROR_EXCEPTION_JSON, "invalid");
+    // test case where the exception classname is not a valid exception class
+    json.put(HttpExceptionUtils.ERROR_CLASSNAME_JSON, String.class.getName());
+    json.put(HttpExceptionUtils.ERROR_MESSAGE_JSON, "EX");
+    Map<String, Object> response = new HashMap<String, Object>();
+    response.put(HttpExceptionUtils.ERROR_JSON, json);
+    ObjectMapper jsonMapper = new ObjectMapper();
+    String msg = jsonMapper.writeValueAsString(response);
+    InputStream is = new ByteArrayInputStream(msg.getBytes(StandardCharsets.UTF_8));
     HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
     Mockito.when(conn.getErrorStream()).thenReturn(is);
     Mockito.when(conn.getResponseMessage()).thenReturn("msg");
-    Mockito.when(conn.getResponseCode()).thenReturn(
-        HttpURLConnection.HTTP_BAD_REQUEST);
-    try {
-      HttpExceptionUtils.validateResponse(conn, HttpURLConnection.HTTP_CREATED);
-      Assert.fail();
-    } catch (IOException ex) {
-      Assert.assertTrue(ex.getMessage().contains("EX"));
-      Assert.assertTrue(ex.getMessage().contains("foo.FooException"));
-    }
+    Mockito.when(conn.getResponseCode()).thenReturn(HttpURLConnection.HTTP_BAD_REQUEST);
+    IOException ex = LambdaTestUtils.intercept(IOException.class,
+        () -> HttpExceptionUtils.validateResponse(conn, HttpURLConnection.HTTP_CREATED));
+    assertContains(Integer.toString(HttpURLConnection.HTTP_BAD_REQUEST), ex.getMessage());
+    assertContains("java.lang.String", ex.getMessage());
+    assertContains("EX", ex.getMessage());
+  }
+
+  private static void assertContains(String expected, String actual) {
+    Assert.assertTrue("Expected: " + expected + ", Actual: " + actual,
+        actual.contains(expected));
   }
 
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestPreconditions.java

@@ -73,7 +73,6 @@ public void testCheckNotNullFailure() throws Exception {
 
     // failure with Null message
     LambdaTestUtils.intercept(NullPointerException.class,
-        null,
         () -> Preconditions.checkNotNull(null, errorMessage));
 
     // failure with message format
@@ -162,7 +161,6 @@ public void testCheckArgumentWithFailure() throws Exception {
     errorMessage = null;
     // failure with Null message
     LambdaTestUtils.intercept(IllegalArgumentException.class,
-        null,
         () -> Preconditions.checkArgument(false, errorMessage));
     // failure with message
     errorMessage = EXPECTED_ERROR_MSG;
@@ -200,7 +198,6 @@ public void testCheckArgumentWithFailure() throws Exception {
     // failure with Null supplier
     final Supplier<String> nullSupplier = null;
     LambdaTestUtils.intercept(IllegalArgumentException.class,
-        null,
         () -> Preconditions.checkArgument(false, nullSupplier));
 
     // ignore illegal format in supplier
@@ -262,7 +259,6 @@ public void testCheckStateWithFailure() throws Exception {
     errorMessage = null;
     // failure with Null message
     LambdaTestUtils.intercept(IllegalStateException.class,
-        null,
         () -> Preconditions.checkState(false, errorMessage));
     // failure with message
     errorMessage = EXPECTED_ERROR_MSG;
@@ -300,7 +296,6 @@ public void testCheckStateWithFailure() throws Exception {
     // failure with Null supplier
     final Supplier<String> nullSupplier = null;
     LambdaTestUtils.intercept(IllegalStateException.class,
-        null,
         () -> Preconditions.checkState(false, nullSupplier));
 
     // ignore illegal format in supplier