[
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: [email protected]
For additional commands, e-mail: [email protected]
