This is an automated email from the ASF dual-hosted git repository.

xxyu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kylin.git


The following commit(s) were added to refs/heads/master by this push:
     new 59b0bf3  KYLIN-4617 Check whether project/jobid exists before download 
diagnosis package
59b0bf3 is described below

commit 59b0bf32921afea2f85035a79744756a3602039a
Author: Zhichao Zhang <441586...@qq.com>
AuthorDate: Thu Jul 2 11:44:54 2020 +0800

    KYLIN-4617 Check whether project/jobid exists before download diagnosis 
package
    
    When download diagnosis package, it's better to check whether project/jobid 
exists, if not, it raise a BadRequestException with XXX doesn't exist instead 
of NPE.
---
 .../kylin/common/util/CliCommandExecutor.java      |  2 +-
 .../kylin/rest/controller/DiagnosisController.java |  5 ++---
 .../java/org/apache/kylin/rest/msg/CnMessage.java  |  8 +++++++
 .../java/org/apache/kylin/rest/msg/Message.java    |  8 +++++++
 .../kylin/rest/service/DiagnosisService.java       | 22 +++++++++++++++++--
 .../org/apache/kylin/rest/util/AclEvaluate.java    | 25 ++++++++++++++++++----
 6 files changed, 60 insertions(+), 10 deletions(-)

diff --git 
a/core-common/src/main/java/org/apache/kylin/common/util/CliCommandExecutor.java
 
b/core-common/src/main/java/org/apache/kylin/common/util/CliCommandExecutor.java
index bb08b4c..74ea1f9 100644
--- 
a/core-common/src/main/java/org/apache/kylin/common/util/CliCommandExecutor.java
+++ 
b/core-common/src/main/java/org/apache/kylin/common/util/CliCommandExecutor.java
@@ -163,7 +163,7 @@ public class CliCommandExecutor {
         }
     }
 
-    public static final String COMMAND_BLOCK_LIST = "[ 
&`>|{}()$;\\#~!+*\\\\]+";
+    public static final String COMMAND_BLOCK_LIST = "[ 
&`>|{}()$;\\-#~!+*\\\\]+";
     public static final String COMMAND_WHITE_LIST = "[^\\w%,@/:=?.\"\\[\\]]";
     public static final String HIVE_BLOCK_LIST = "[ <>()$;\\-#!+*\"'/=%@]+";
 
diff --git 
a/server-base/src/main/java/org/apache/kylin/rest/controller/DiagnosisController.java
 
b/server-base/src/main/java/org/apache/kylin/rest/controller/DiagnosisController.java
index 6bb16fe..190ee43 100644
--- 
a/server-base/src/main/java/org/apache/kylin/rest/controller/DiagnosisController.java
+++ 
b/server-base/src/main/java/org/apache/kylin/rest/controller/DiagnosisController.java
@@ -26,7 +26,6 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.apache.kylin.common.persistence.AutoDeleteDirectory;
-import org.apache.kylin.common.util.CliCommandExecutor;
 import org.apache.kylin.metadata.badquery.BadQueryEntry;
 import org.apache.kylin.metadata.badquery.BadQueryHistory;
 import org.apache.kylin.rest.exception.InternalErrorException;
@@ -80,7 +79,7 @@ public class DiagnosisController extends BasicController {
     public void dumpProjectDiagnosisInfo(@PathVariable String project, final 
HttpServletRequest request,
             final HttpServletResponse response) {
         try (AutoDeleteDirectory diagDir = new 
AutoDeleteDirectory("diag_project", "")) {
-            String filePath = 
dgService.dumpProjectDiagnosisInfo(CliCommandExecutor.checkParameter(project), 
diagDir.getFile());
+            String filePath = dgService.dumpProjectDiagnosisInfo(project, 
diagDir.getFile());
             setDownloadResponse(filePath, response);
         } catch (IOException e) {
             throw new InternalErrorException("Failed to dump project diagnosis 
info. " + e.getMessage(), e);
@@ -96,7 +95,7 @@ public class DiagnosisController extends BasicController {
     public void dumpJobDiagnosisInfo(@PathVariable String jobId, final 
HttpServletRequest request,
             final HttpServletResponse response) {
         try (AutoDeleteDirectory diagDir = new AutoDeleteDirectory("diag_job", 
"")) {
-            String filePath = 
dgService.dumpJobDiagnosisInfo(CliCommandExecutor.checkParameter(jobId), 
diagDir.getFile());
+            String filePath = dgService.dumpJobDiagnosisInfo(jobId, 
diagDir.getFile());
             setDownloadResponse(filePath, response);
         } catch (IOException e) {
             throw new InternalErrorException("Failed to dump job diagnosis 
info. " + e.getMessage(), e);
diff --git a/server-base/src/main/java/org/apache/kylin/rest/msg/CnMessage.java 
b/server-base/src/main/java/org/apache/kylin/rest/msg/CnMessage.java
index 4424ef9..cdf3cbb 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/msg/CnMessage.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/msg/CnMessage.java
@@ -404,6 +404,14 @@ public class CnMessage extends Message {
         return "找不到诊断包, 路径: %s";
     }
 
+    public String getDIAG_PROJECT_NOT_FOUND() {
+        return "找不到项目: %s.";
+    }
+
+    public String getDIAG_JOBID_NOT_FOUND() {
+        return "找不到任务ID: %s.";
+    }
+
     // Encoding
     public String getVALID_ENCODING_NOT_AVAILABLE() {
         return "无法为数据类型: %s 提供合法的编码";
diff --git a/server-base/src/main/java/org/apache/kylin/rest/msg/Message.java 
b/server-base/src/main/java/org/apache/kylin/rest/msg/Message.java
index ea23761..dbd637c 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/msg/Message.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/msg/Message.java
@@ -417,6 +417,14 @@ public class Message {
         return "Diagnosis package not found in directory: %s.";
     }
 
+    public String getDIAG_PROJECT_NOT_FOUND() {
+        return "Can not find project: %s.";
+    }
+
+    public String getDIAG_JOBID_NOT_FOUND() {
+        return "Can not find job id: %s.";
+    }
+
     // Encoding
     public String getVALID_ENCODING_NOT_AVAILABLE() {
         return "Can not provide valid encodings for datatype: %s.";
diff --git 
a/server-base/src/main/java/org/apache/kylin/rest/service/DiagnosisService.java 
b/server-base/src/main/java/org/apache/kylin/rest/service/DiagnosisService.java
index 7eb96c8..66b7ba0 100644
--- 
a/server-base/src/main/java/org/apache/kylin/rest/service/DiagnosisService.java
+++ 
b/server-base/src/main/java/org/apache/kylin/rest/service/DiagnosisService.java
@@ -30,12 +30,16 @@ import org.apache.commons.lang3.StringUtils;
 import org.apache.kylin.common.KylinConfig;
 import org.apache.kylin.common.util.CliCommandExecutor;
 import org.apache.kylin.common.util.Pair;
+import org.apache.kylin.job.JobInstance;
 import org.apache.kylin.metadata.badquery.BadQueryEntry;
 import org.apache.kylin.metadata.badquery.BadQueryHistory;
+import org.apache.kylin.metadata.project.ProjectInstance;
+import org.apache.kylin.metadata.project.ProjectManager;
 import org.apache.kylin.rest.exception.BadRequestException;
 import org.apache.kylin.rest.msg.Message;
 import org.apache.kylin.rest.msg.MsgPicker;
 import org.apache.kylin.rest.util.AclEvaluate;
+import org.apache.kylin.rest.util.ValidateUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -81,14 +85,28 @@ public class DiagnosisService extends BasicService {
     }
 
     public String dumpProjectDiagnosisInfo(String project, File exportPath) 
throws IOException {
-        aclEvaluate.checkProjectOperationPermission(project);
+        Message msg = MsgPicker.getMsg();
+        ProjectInstance projectInstance =
+                ProjectManager.getInstance(KylinConfig.getInstanceFromEnv())
+                        
.getProject(ValidateUtil.convertStringToBeAlphanumericUnderscore(project));
+        if (null == projectInstance) {
+            throw new BadRequestException(
+                    String.format(Locale.ROOT, 
msg.getDIAG_PROJECT_NOT_FOUND(), project));
+        }
+        aclEvaluate.checkProjectOperationPermission(projectInstance);
         String[] args = { project, exportPath.getAbsolutePath() };
         runDiagnosisCLI(args);
         return getDiagnosisPackageName(exportPath);
     }
 
     public String dumpJobDiagnosisInfo(String jobId, File exportPath) throws 
IOException {
-        
aclEvaluate.checkProjectOperationPermission(jobService.getJobInstance(jobId));
+        Message msg = MsgPicker.getMsg();
+        JobInstance jobInstance = jobService.getJobInstance(jobId);
+        if (null == jobInstance) {
+            throw new BadRequestException(
+                    String.format(Locale.ROOT, msg.getDIAG_JOBID_NOT_FOUND(), 
jobId));
+        }
+        aclEvaluate.checkProjectOperationPermission(jobInstance);
         String[] args = { jobId, exportPath.getAbsolutePath() };
         runDiagnosisCLI(args);
         return getDiagnosisPackageName(exportPath);
diff --git 
a/server-base/src/main/java/org/apache/kylin/rest/util/AclEvaluate.java 
b/server-base/src/main/java/org/apache/kylin/rest/util/AclEvaluate.java
index dcd625a..3cfcfae 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/util/AclEvaluate.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/util/AclEvaluate.java
@@ -60,25 +60,42 @@ public class AclEvaluate {
         return getProjectInstance(projectName);
     }
 
+    public void checkProjectAdminPermission(ProjectInstance projectInstance) {
+        aclUtil.hasProjectAdminPermission(projectInstance);
+    }
+
+    //for raw project
+    public void checkProjectReadPermission(ProjectInstance projectInstance) {
+        aclUtil.hasProjectReadPermission(projectInstance);
+    }
+
+    public void checkProjectWritePermission(ProjectInstance projectInstance) {
+        aclUtil.hasProjectWritePermission(projectInstance);
+    }
+
+    public void checkProjectOperationPermission(ProjectInstance 
projectInstance) {
+        aclUtil.hasProjectOperationPermission(projectInstance);
+    }
+
     public void checkProjectAdminPermission(String projectName) {
         ProjectInstance projectInstance = getProjectInstance(projectName);
-        aclUtil.hasProjectAdminPermission(projectInstance);
+        checkProjectAdminPermission(projectInstance);
     }
 
     //for raw project
     public void checkProjectReadPermission(String projectName) {
         ProjectInstance projectInstance = getProjectInstance(projectName);
-        aclUtil.hasProjectReadPermission(projectInstance);
+        checkProjectReadPermission(projectInstance);
     }
 
     public void checkProjectWritePermission(String projectName) {
         ProjectInstance projectInstance = getProjectInstance(projectName);
-        aclUtil.hasProjectWritePermission(projectInstance);
+        checkProjectWritePermission(projectInstance);
     }
 
     public void checkProjectOperationPermission(String projectName) {
         ProjectInstance projectInstance = getProjectInstance(projectName);
-        aclUtil.hasProjectOperationPermission(projectInstance);
+        checkProjectOperationPermission(projectInstance);
     }
 
     //for cube acl entity

Reply via email to