[ https://issues.apache.org/jira/browse/HADOOP-19079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17833689#comment-17833689 ]
ASF GitHub Bot commented on HADOOP-19079: ----------------------------------------- steveloughran commented on code in PR #6557: URL: https://github.com/apache/hadoop/pull/6557#discussion_r1550188734 ########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/LambdaTestUtils.java: ########## @@ -382,7 +383,7 @@ public static <T, E extends Throwable> E intercept( Callable<T> eval) throws Exception { return intercept(clazz, - null, + (String) null, Review Comment: ooh, I worry if there are going to be link problems by anything not in our codebase using the method (gcs etc). Hope not. ########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestHttpExceptionUtils.java: ########## @@ -87,35 +92,34 @@ public void testValidateResponseOK() throws IOException { 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); + 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)); - } + Collection<String> expectedValues = Review Comment: Arrays.asList() is easier ########## hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/LambdaTestUtils.java: ########## @@ -508,6 +509,61 @@ public static <T, E extends Throwable> E intercept( return ex; } + /** + * Intercept an exception; throw an {@code AssertionError} if one not raised. + * The caught exception is rethrown if it is of the wrong class or + * does not contain the text defined in {@code contained}. + * <p> + * Example: expect deleting a nonexistent file to raise a + * {@code FileNotFoundException} with the {@code toString()} value + * containing the text {@code "missing"}. + * <pre> + * FileNotFoundException ioe = intercept(FileNotFoundException.class, + * "missing", + * "path should not be found", + * () -> { + * filesystem.delete(new Path("/missing"), false); + * }); + * </pre> + * + * @param clazz class of exception; the raised exception must be this class + * <i>or a subclass</i>. + * @param contains strings which must be in the {@code toString()} value + * of the exception (order does not matter) + * @param message any message tho include in exception/log messages + * @param eval expression to eval + * @param <T> return type of expression + * @param <E> exception class + * @return the caught exception if it was of the expected type and contents + * @throws Exception any other exception raised + * @throws AssertionError if the evaluation call didn't raise an exception. + * The error includes the {@code toString()} value of the result, if this + * can be determined. + * @see GenericTestUtils#assertExceptionContains(String, Throwable) + */ + public static <T, E extends Throwable> E intercept( + Class<E> clazz, + Collection<String> contains, + String message, + Callable<T> eval) + throws Exception { + E ex; + try { + T result = eval.call(); + throw new AssertionError(message + ": " + robustToString(result)); + } catch (Throwable e) { + if (!clazz.isAssignableFrom(e.getClass())) { + throw e; + } else { + ex = (E) e; + } + } + for (String contained : contains) { Review Comment: skip if contains==null > check that class that is loaded is really an exception > ------------------------------------------------------ > > Key: HADOOP-19079 > URL: https://issues.apache.org/jira/browse/HADOOP-19079 > Project: Hadoop Common > Issue Type: Task > Components: common, security > Reporter: PJ Fanning > Priority: Major > Labels: pull-request-available > > It can be dangerous taking class names as inputs from HTTP messages even if > we control the source. Issue is in HttpExceptionUtils in hadoop-common > (validateResponse method). > I can provide a PR that will highlight the issue. -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org