YARN-5191. Renamed the newly added “download=true” option for getting…

… logs via NMWebServices and AHSWebServices to be a better "format" option. (Xuan Gong via vinodkv)
apache · Jun 9, 2016 · 9378d94 · 9378d94
1 parent 656c460
commit 9378d94
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 28 deletions.
diff --git a/...yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java b/...yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java
@@ -24,6 +24,7 @@
 import java.net.InetSocketAddress;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 import org.apache.hadoop.classification.InterfaceAudience.Private;
@@ -400,4 +401,21 @@ public static ApplicationId parseApplicationId(RecordFactory recordFactory,
     }
     return aid;
   }
+
+  public static String getSupportedLogContentType(String format) {
+    if (format.equalsIgnoreCase("text")) {
+      return "text/plain";
+    } else if (format.equalsIgnoreCase("octet-stream")) {
+      return "application/octet-stream";
+    }
+    return null;
+  }
+
+  public static String getDefaultLogContentType() {
+    return "text/plain";
+  }
+
+  public static List<String> listSupportedLogContentType() {
+    return Arrays.asList("text", "octet-stream");
+  }
 }
diff --git a/...n/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/AHSWebServices.java b/...n/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/AHSWebServices.java
@@ -66,6 +66,7 @@
 import org.apache.hadoop.yarn.util.Times;
 import org.apache.hadoop.yarn.util.timeline.TimelineUtils;
 import org.apache.hadoop.yarn.webapp.BadRequestException;
+import org.apache.hadoop.yarn.webapp.util.WebAppUtils;
 import com.google.common.base.Joiner;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
@@ -212,7 +213,7 @@ public Response getLogs(@Context HttpServletRequest req,
       @Context HttpServletResponse res,
       @PathParam("containerid") String containerIdStr,
       @PathParam("filename") String filename,
-      @QueryParam("download") String download,
+      @QueryParam("format") String format,
       @QueryParam("size") String size) {
     init(res);
     ContainerId containerId;
@@ -223,9 +224,6 @@ public Response getLogs(@Context HttpServletRequest req,
           "Invalid ContainerId: " + containerIdStr);
     }
 
-    boolean downloadFile = parseBooleanParam(download);
-
-
     final long length = parseLongParam(size);
 
     ApplicationId appId = containerId.getApplicationAttemptId()
@@ -236,7 +234,7 @@ public Response getLogs(@Context HttpServletRequest req,
     } catch (Exception ex) {
       // directly find logs from HDFS.
       return sendStreamOutputResponse(appId, null, null, containerIdStr,
-          filename, downloadFile, length);
+          filename, format, length);
     }
     String appOwner = appInfo.getUser();
 
@@ -250,7 +248,7 @@ public Response getLogs(@Context HttpServletRequest req,
       if (isFinishedState(appInfo.getAppState())) {
         // directly find logs from HDFS.
         return sendStreamOutputResponse(appId, appOwner, null, containerIdStr,
-            filename, downloadFile, length);
+            filename, format, length);
       }
       return createBadResponse(Status.INTERNAL_SERVER_ERROR,
           "Can not get ContainerInfo for the container: " + containerId);
@@ -270,7 +268,7 @@ public Response getLogs(@Context HttpServletRequest req,
       return response.build();
     } else if (isFinishedState(appInfo.getAppState())) {
       return sendStreamOutputResponse(appId, appOwner, nodeId,
-          containerIdStr, filename, downloadFile, length);
+          containerIdStr, filename, format, length);
     } else {
       return createBadResponse(Status.NOT_FOUND,
           "The application is not at Running or Finished State.");
@@ -293,13 +291,19 @@ private Response createBadResponse(Status status, String errMessage) {
     return response;
   }
 
-  private boolean parseBooleanParam(String param) {
-    return ("true").equalsIgnoreCase(param);
-  }
-
   private Response sendStreamOutputResponse(ApplicationId appId,
       String appOwner, String nodeId, String containerIdStr,
-      String fileName, boolean downloadFile, long bytes) {
+      String fileName, String format, long bytes) {
+    String contentType = WebAppUtils.getDefaultLogContentType();
+    if (format != null && !format.isEmpty()) {
+      contentType = WebAppUtils.getSupportedLogContentType(format);
+      if (contentType == null) {
+        String errorMessage = "The valid values for the parameter : format "
+            + "are " + WebAppUtils.listSupportedLogContentType();
+        return Response.status(Status.BAD_REQUEST).entity(errorMessage)
+            .build();
+      }
+    }
     StreamingOutput stream = null;
     try {
       stream = getStreamingOutput(appId, appOwner, nodeId,
@@ -313,9 +317,11 @@ private Response sendStreamOutputResponse(ApplicationId appId,
           "Can not get log for container: " + containerIdStr);
     }
     ResponseBuilder response = Response.ok(stream);
-    if (downloadFile) {
-      response.header("Content-Type", "application/octet-stream");
-    }
+    response.header("Content-Type", contentType);
+    // Sending the X-Content-Type-Options response header with the value
+    // nosniff will prevent Internet Explorer from MIME-sniffing a response
+    // away from the declared content-type.
+    response.header("X-Content-Type-Options", "nosniff");
     return response.build();
   }
 

diff --git a/...manager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/NMWebServices.java b/...manager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/NMWebServices.java
@@ -206,6 +206,10 @@ public ContainerInfo getNodeContainer(@javax.ws.rs.core.Context
    *    The container ID
    * @param filename
    *    The name of the log file
+   * @param format
+   *    The content type
+   * @param size
+   *    the size of the log file
    * @return
    *    The contents of the container's log file
    */
@@ -216,7 +220,7 @@ public ContainerInfo getNodeContainer(@javax.ws.rs.core.Context
   @Unstable
   public Response getLogs(@PathParam("containerid") String containerIdStr,
       @PathParam("filename") String filename,
-      @QueryParam("download") String download,
+      @QueryParam("format") String format,
       @QueryParam("size") String size) {
     ContainerId containerId;
     try {
@@ -234,8 +238,18 @@ public Response getLogs(@PathParam("containerid") String containerIdStr,
     } catch (YarnException ex) {
       return Response.serverError().entity(ex.getMessage()).build();
     }
-    boolean downloadFile = parseBooleanParam(download);
     final long bytes = parseLongParam(size);
+    String contentType = WebAppUtils.getDefaultLogContentType();
+    if (format != null && !format.isEmpty()) {
+      contentType = WebAppUtils.getSupportedLogContentType(format);
+      if (contentType == null) {
+        String errorMessage = "The valid values for the parameter : format "
+            + "are " + WebAppUtils.listSupportedLogContentType();
+        return Response.status(Status.BAD_REQUEST).entity(errorMessage)
+            .build();
+      }
+    }
+
     try {
       final FileInputStream fis = ContainerLogsUtils.openLogFileForRead(
           containerIdStr, logFile, nmContext);
@@ -288,22 +302,17 @@ public void write(OutputStream os) throws IOException,
         }
       };
       ResponseBuilder resp = Response.ok(stream);
-      if (downloadFile) {
-        resp.header("Content-Type", "application/octet-stream");
-      }
+      resp.header("Content-Type", contentType);
+      // Sending the X-Content-Type-Options response header with the value
+      // nosniff will prevent Internet Explorer from MIME-sniffing a response
+      // away from the declared content-type.
+      resp.header("X-Content-Type-Options", "nosniff");
       return resp.build();
     } catch (IOException ex) {
       return Response.serverError().entity(ex.getMessage()).build();
     }
   }
 
-  private boolean parseBooleanParam(String param) {
-    if (param != null) {
-      return ("true").equalsIgnoreCase(param);
-    }
-    return false;
-  }
-
   private long parseLongParam(String bytes) {
     if (bytes == null || bytes.isEmpty()) {
       return Long.MAX_VALUE;

diff --git a/...ger/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestNMWebServices.java b/...ger/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestNMWebServices.java
@@ -57,6 +57,7 @@
 import org.apache.hadoop.yarn.webapp.JerseyTestBase;
 import org.apache.hadoop.yarn.webapp.WebApp;
 import org.apache.hadoop.yarn.webapp.WebServicesTestUtils;
+import org.apache.hadoop.yarn.webapp.util.WebAppUtils;
 import org.codehaus.jettison.json.JSONException;
 import org.codehaus.jettison.json.JSONObject;
 import org.junit.AfterClass;
@@ -389,18 +390,30 @@ public void testContainerLogs() throws IOException {
         .queryParam("size", "-10000")
         .accept(MediaType.TEXT_PLAIN).get(ClientResponse.class);
     responseText = response.getEntity(String.class);
+    assertEquals("text/plain", response.getType().toString());
     assertEquals(fullTextSize, responseText.getBytes().length);
     assertEquals(logMessage, responseText);
 
     // ask and download it
     response = r.path("ws").path("v1").path("node").path("containerlogs")
-        .path(containerIdStr).path(filename).queryParam("download", "true")
+        .path(containerIdStr).path(filename)
+        .queryParam("format", "octet-stream")
         .accept(MediaType.TEXT_PLAIN).get(ClientResponse.class);
     responseText = response.getEntity(String.class);
     assertEquals(logMessage, responseText);
     assertEquals(200, response.getStatus());
     assertEquals("application/octet-stream", response.getType().toString());
 
+    // specify a invalid format value
+    response = r.path("ws").path("v1").path("node").path("containerlogs")
+        .path(containerIdStr).path(filename)
+        .queryParam("format", "123")
+        .accept(MediaType.TEXT_PLAIN).get(ClientResponse.class);
+    responseText = response.getEntity(String.class);
+    assertEquals("The valid values for the parameter : format are "
+        + WebAppUtils.listSupportedLogContentType(), responseText);
+    assertEquals(400, response.getStatus());
+
     // ask for file that doesn't exist
     response = r.path("ws").path("v1").path("node")
         .path("containerlogs").path(containerIdStr).path("uhhh")