Merge pull request apache#3030 from RuiLi8080/STORM-3411

[STORM-3411] add parent dir name to downloaded worker log file
hecsalazar · Jul 8, 2019 · 80b2384 · 80b2384
2 parents 727caf6 + 4990c86
commit 80b2384
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 29 deletions.
diff --git a/.../src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogDownloadHandler.java b/.../src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogDownloadHandler.java
@@ -49,26 +49,28 @@ public LogviewerLogDownloadHandler(String logRoot, String daemonLogRoot, WorkerL
     /**
      * Download an worker log.
      *
+     * @param host host address
      * @param fileName file to download
      * @param user username
      * @return a Response which lets browsers download that file.
-     * @see {@link LogFileDownloader#downloadFile(String, String, boolean)}
+     * @see {@link LogFileDownloader#downloadFile(String, String, String, boolean)}
      */
-    public Response downloadLogFile(String fileName, String user) throws IOException {
+    public Response downloadLogFile(String host, String fileName, String user) throws IOException {
         workerLogs.setLogFilePermission(fileName);
-        return logFileDownloadHelper.downloadFile(fileName, user, false);
+        return logFileDownloadHelper.downloadFile(host, fileName, user, false);
     }
 
     /**
      * Download a daemon log.
      *
+     * @param host host address
      * @param fileName file to download
      * @param user username
      * @return a Response which lets browsers download that file.
-     * @see {@link LogFileDownloader#downloadFile(String, String, boolean)}
+     * @see {@link LogFileDownloader#downloadFile(String, String, String, boolean)}
      */
-    public Response downloadDaemonLogFile(String fileName, String user) throws IOException {
-        return logFileDownloadHelper.downloadFile(fileName, user, true);
+    public Response downloadDaemonLogFile(String host, String fileName, String user) throws IOException {
+        return logFileDownloadHelper.downloadFile(host, fileName, user, true);
     }
 
 }
diff --git a/...bapp/src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerProfileHandler.java b/...bapp/src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerProfileHandler.java
@@ -102,14 +102,15 @@ public Response listDumpFiles(String topologyId, String hostPort, String user) t
     /**
      * Download a dump file.
      *
+     * @param host host address
      * @param topologyId topology ID
      * @param hostPort host and port of worker
      * @param fileName dump file name
      * @param user username
      * @return a Response which lets browsers download that file.
-     * @see {@link org.apache.storm.daemon.logviewer.utils.LogFileDownloader#downloadFile(String, String, boolean)}
+     * @see {@link org.apache.storm.daemon.logviewer.utils.LogFileDownloader#downloadFile(String, String, String, boolean)}
      */
-    public Response downloadDumpFile(String topologyId, String hostPort, String fileName, String user) throws IOException {
+    public Response downloadDumpFile(String host, String topologyId, String hostPort, String fileName, String user) throws IOException {
         String portStr = hostPort.split(":")[1];
         Path rawFile = logRoot.resolve(topologyId).resolve(portStr).resolve(fileName);
         Path absFile = rawFile.toAbsolutePath().normalize();
@@ -121,7 +122,7 @@ public Response downloadDumpFile(String topologyId, String hostPort, String file
         if (absFile.toFile().exists()) {
             String workerFileRelativePath = String.join(File.separator, topologyId, portStr, WORKER_LOG_FILENAME);
             if (resourceAuthorizer.isUserAllowedToAccessFile(user, workerFileRelativePath)) {
-                return LogviewerResponseBuilder.buildDownloadFile(absFile.toFile(), numFileDownloadExceptions);
+                return LogviewerResponseBuilder.buildDownloadFile(host, absFile.toFile(), numFileDownloadExceptions);
             } else {
                 return LogviewerResponseBuilder.buildResponseUnauthorizedUser(user);
             }

diff --git a/storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/utils/LogFileDownloader.java b/storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/utils/LogFileDownloader.java
@@ -21,7 +21,6 @@
 import com.codahale.metrics.Histogram;
 import com.codahale.metrics.Meter;
 
-import java.io.File;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -59,12 +58,13 @@ public LogFileDownloader(String logRoot, String daemonLogRoot, ResourceAuthorize
     /**
      * Checks authorization for the log file and download.
      *
+     * @param host host address
      * @param fileName file to download
      * @param user username
      * @param isDaemon true if the file is a daemon log, false if the file is an worker log
      * @return a Response which lets browsers download that file.
      */
-    public Response downloadFile(String fileName, String user, boolean isDaemon) throws IOException {
+    public Response downloadFile(String host, String fileName, String user, boolean isDaemon) throws IOException {
         Path rootDir = isDaemon ? daemonLogRoot : logRoot;
         Path rawFile = rootDir.resolve(fileName);
         Path file = rawFile.toAbsolutePath().normalize();
@@ -80,7 +80,7 @@ public Response downloadFile(String fileName, String user, boolean isDaemon) thr
         if (file.toFile().exists()) {
             if (isDaemon || resourceAuthorizer.isUserAllowedToAccessFile(user, fileName)) {
                 fileDownloadSizeDistMb.update(Math.round((double) file.toFile().length() / FileUtils.ONE_MB));
-                return LogviewerResponseBuilder.buildDownloadFile(file.toFile(), numFileDownloadExceptions);
+                return LogviewerResponseBuilder.buildDownloadFile(host, file.toFile(), numFileDownloadExceptions);
             } else {
                 return LogviewerResponseBuilder.buildResponseUnauthorizedUser(user);
             }

diff --git a/...ebapp/src/main/java/org/apache/storm/daemon/logviewer/utils/LogviewerResponseBuilder.java b/...ebapp/src/main/java/org/apache/storm/daemon/logviewer/utils/LogviewerResponseBuilder.java
@@ -75,16 +75,25 @@ public static Response buildSuccessJsonResponse(Object entity, String callback,
     /**
      * Build a Response object representing download a file.
      *
+     * @param host host address
      * @param file file to download
      */
-    public static Response buildDownloadFile(File file, Meter numFileDownloadExceptions) throws IOException {
+    public static Response buildDownloadFile(String host, File file, Meter numFileDownloadExceptions) throws IOException {
+        String fname;
+        try {
+            String topoInfo = file.getParentFile().getParentFile().getName();
+            String port = file.getParentFile().getName();
+            fname = String.format("%s-%s-%s-%s", host, port, topoInfo, file.getName());
+        } catch (NullPointerException e) {
+            fname = file.getName();
+        }
         try {
             // do not close this InputStream in method: it will be used from jetty server
             InputStream is = Files.newInputStream(file.toPath());
             return Response.status(OK)
                     .entity(wrapWithStreamingOutput(is))
                     .type(MediaType.APPLICATION_OCTET_STREAM_TYPE)
-                    .header("Content-Disposition", "attachment; filename=\"" + file.getName() + "\"")
+                    .header("Content-Disposition", "attachment; filename=\"" + fname + "\"")
                     .build();
         } catch (IOException e) {
             numFileDownloadExceptions.mark();

diff --git a/storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/webapp/LogviewerResource.java b/storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/webapp/LogviewerResource.java
@@ -22,6 +22,7 @@
 import com.codahale.metrics.Timer;
 
 import java.io.IOException;
+import java.net.InetAddress;
 import java.util.Map;
 
 import javax.servlet.http.HttpServletRequest;
@@ -229,9 +230,11 @@ public Response listDumpFiles(@PathParam("topo-id") String topologyId, @PathPara
     @Path("/dumps/{topo-id}/{host-port}/{filename}")
     public Response downloadDumpFile(@PathParam("topo-id") String topologyId, @PathParam("host-port") String hostPort,
                                      @PathParam("filename") String fileName, @Context HttpServletRequest request) throws IOException {
+
         String user = httpCredsHandler.getUserName(request);
         try {
-            return profileHandler.downloadDumpFile(topologyId, hostPort, fileName, user);
+            String host = InetAddress.getLocalHost().getCanonicalHostName();
+            return profileHandler.downloadDumpFile(host, topologyId, hostPort, fileName, user);
         } catch (IOException e) {
             numDownloadDumpExceptions.mark();
             throw e;
@@ -245,12 +248,12 @@ public Response downloadDumpFile(@PathParam("topo-id") String topologyId, @PathP
     @Path("/download")
     public Response downloadLogFile(@Context HttpServletRequest request) throws IOException {
         meterDownloadLogFileHttpRequests.mark();
-
         String user = httpCredsHandler.getUserName(request);
         String file = request.getParameter("file");
         String decodedFileName = Utils.urlDecodeUtf8(file);
         try {
-            return logDownloadHandler.downloadLogFile(decodedFileName, user);
+            String host = InetAddress.getLocalHost().getCanonicalHostName();
+            return logDownloadHandler.downloadLogFile(host, decodedFileName, user);
         } catch (IOException e) {
             numDownloadLogExceptions.mark();
             throw e;
@@ -264,12 +267,12 @@ public Response downloadLogFile(@Context HttpServletRequest request) throws IOEx
     @Path("/daemondownload")
     public Response downloadDaemonLogFile(@Context HttpServletRequest request) throws IOException {
         meterDownloadLogDaemonFileHttpRequests.mark();
-
         String user = httpCredsHandler.getUserName(request);
         String file = request.getParameter("file");
         String decodedFileName = Utils.urlDecodeUtf8(file);
         try {
-            return logDownloadHandler.downloadDaemonLogFile(decodedFileName, user);
+            String host = InetAddress.getLocalHost().getCanonicalHostName();
+            return logDownloadHandler.downloadDaemonLogFile(host, decodedFileName, user);
         } catch (IOException e) {
             numDownloadDaemonLogExceptions.mark();
             throw e;

diff --git a/.../test/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogDownloadHandlerTest.java b/.../test/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogDownloadHandlerTest.java
@@ -45,8 +45,8 @@ public void testDownloadLogFile() throws IOException {
 
             LogviewerLogDownloadHandler handler = createHandlerTraversalTests(rootPath.getFile().toPath());
 
-            Response topoAResponse = handler.downloadLogFile("topoA/1111/worker.log", "user");
-            Response topoBResponse = handler.downloadLogFile("topoB/1111/worker.log", "user");
+            Response topoAResponse = handler.downloadLogFile("host", "topoA/1111/worker.log", "user");
+            Response topoBResponse = handler.downloadLogFile("host", "topoB/1111/worker.log", "user");
 
             Utils.forceDelete(rootPath.toString());
 
@@ -63,7 +63,7 @@ public void testDownloadLogFileTraversal() throws IOException {
 
             LogviewerLogDownloadHandler handler = createHandlerTraversalTests(rootPath.getFile().toPath());
 
-            Response topoAResponse = handler.downloadLogFile("../nimbus.log", "user");
+            Response topoAResponse = handler.downloadLogFile("host","../nimbus.log", "user");
 
             Utils.forceDelete(rootPath.toString());
 
@@ -77,7 +77,7 @@ public void testDownloadDaemonLogFile() throws IOException {
 
             LogviewerLogDownloadHandler handler = createHandlerTraversalTests(rootPath.getFile().toPath());
 
-            Response response = handler.downloadDaemonLogFile("nimbus.log", "user");
+            Response response = handler.downloadDaemonLogFile("host","nimbus.log", "user");
 
             Utils.forceDelete(rootPath.toString());
 
@@ -92,7 +92,7 @@ public void testDownloadDaemonLogFilePathIntoWorkerLogs() throws IOException {
 
             LogviewerLogDownloadHandler handler = createHandlerTraversalTests(rootPath.getFile().toPath());
 
-            Response response = handler.downloadDaemonLogFile("workers-artifacts/topoA/1111/worker.log", "user");
+            Response response = handler.downloadDaemonLogFile("host","workers-artifacts/topoA/1111/worker.log", "user");
 
             Utils.forceDelete(rootPath.toString());
 
@@ -106,7 +106,7 @@ public void testDownloadDaemonLogFilePathOutsideLogRoot() throws IOException {
 
             LogviewerLogDownloadHandler handler = createHandlerTraversalTests(rootPath.getFile().toPath());
 
-            Response response = handler.downloadDaemonLogFile("../evil.sh", "user");
+            Response response = handler.downloadDaemonLogFile("host","../evil.sh", "user");
 
             Utils.forceDelete(rootPath.toString());
 

diff --git a/.../src/test/java/org/apache/storm/daemon/logviewer/handler/LogviewerProfileHandlerTest.java b/.../src/test/java/org/apache/storm/daemon/logviewer/handler/LogviewerProfileHandlerTest.java
@@ -98,8 +98,8 @@ public void testDownloadDumpFile() throws IOException {
 
             LogviewerProfileHandler handler = createHandlerTraversalTests(rootPath.getFile().toPath());
 
-            Response topoAResponse = handler.downloadDumpFile("topoA", "localhost:1111", "worker.jfr", "user");
-            Response topoBResponse = handler.downloadDumpFile("topoB", "localhost:1111", "worker.txt", "user");
+            Response topoAResponse = handler.downloadDumpFile("host","topoA", "localhost:1111", "worker.jfr", "user");
+            Response topoBResponse = handler.downloadDumpFile("host","topoB", "localhost:1111", "worker.txt", "user");
 
             Utils.forceDelete(rootPath.toString());
 
@@ -116,7 +116,7 @@ public void testDownloadDumpFileTraversalInTopoId() throws IOException {
 
             LogviewerProfileHandler handler = createHandlerTraversalTests(rootPath.getFile().toPath());
 
-            Response topoAResponse = handler.downloadDumpFile("../../", "localhost:logs", "daemon-dump.bin", "user");
+            Response topoAResponse = handler.downloadDumpFile("host","../../", "localhost:logs", "daemon-dump.bin", "user");
 
             Utils.forceDelete(rootPath.toString());
 
@@ -130,7 +130,7 @@ public void testDownloadDumpFileTraversalInPort() throws IOException {
 
             LogviewerProfileHandler handler = createHandlerTraversalTests(rootPath.getFile().toPath());
 
-            Response topoAResponse = handler.downloadDumpFile("../", "localhost:../logs", "daemon-dump.bin", "user");
+            Response topoAResponse = handler.downloadDumpFile("host","../", "localhost:../logs", "daemon-dump.bin", "user");
 
             Utils.forceDelete(rootPath.toString());