github-code-scanning[bot] commented on code in PR #11931:
URL: https://github.com/apache/dolphinscheduler/pull/11931#discussion_r972568562
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -390,31 +368,29 @@
Collection<Long> codes =
CollectionUtils.subtract(postTaskCodes, taskNodeCodes);
if (CollectionUtils.isNotEmpty(codes)) {
logger.error("the task code is not exist");
- putMsg(result, Status.TASK_DEFINE_NOT_EXIST,
- StringUtils.join(codes, Constants.COMMA));
- return result;
+ throw new ServiceException(Status.TASK_DEFINE_NOT_EXIST,
StringUtils.join(codes, Constants.COMMA));
}
}
if (graphHasCycle(taskNodeList)) {
logger.error("process DAG has cycle");
- putMsg(result, Status.PROCESS_NODE_HAS_CYCLE);
- return result;
+ throw new ServiceException(Status.PROCESS_NODE_HAS_CYCLE);
}
// check whether the task relation json is normal
for (ProcessTaskRelationLog processTaskRelationLog :
taskRelationList) {
if (processTaskRelationLog.getPostTaskCode() == 0) {
logger.error("the post_task_code or post_task_version
can't be zero");
- putMsg(result, Status.CHECK_PROCESS_TASK_RELATION_ERROR);
- return result;
+ throw new
ServiceException(Status.CHECK_PROCESS_TASK_RELATION_ERROR);
}
}
- putMsg(result, Status.SUCCESS);
+ return taskRelationList;
+ } catch (ServiceException ex) {
+ throw ex;
} catch (Exception e) {
- result.put(Constants.STATUS,
Status.REQUEST_PARAMS_NOT_VALID_ERROR);
- result.put(Constants.MSG, e.getMessage());
+ logger.error("Check task relation list error, meet an unknown
exception, given taskRelationJson: {}",
+ taskRelationJson, e);
Review Comment:
## Logging should not be vulnerable to injection attacks
<!--SONAR_ISSUE_KEY:AYNENc3aS32ciPhy-rL8-->Change this code to not log
user-controlled data. <p>See more on <a
href="https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AYNENc3aS32ciPhy-rL8&open=AYNENc3aS32ciPhy-rL8&pullRequest=11931";>SonarCloud</a></p>
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/1289)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -317,66 +298,63 @@
logger.info("The task has not changed, so skip");
}
if (saveTaskResult == Constants.DEFINITION_FAILURE) {
- putMsg(result, Status.CREATE_TASK_DEFINITION_ERROR);
throw new ServiceException(Status.CREATE_TASK_DEFINITION_ERROR);
}
int insertVersion = processService.saveProcessDefine(loginUser,
processDefinition, Boolean.TRUE, Boolean.TRUE);
if (insertVersion == 0) {
- putMsg(result, Status.CREATE_PROCESS_DEFINITION_ERROR);
throw new ServiceException(Status.CREATE_PROCESS_DEFINITION_ERROR);
}
int insertResult = processService.saveTaskRelation(loginUser,
processDefinition.getProjectCode(),
processDefinition.getCode(),
insertVersion, taskRelationList, taskDefinitionLogs,
Boolean.TRUE);
- if (insertResult == Constants.EXIT_CODE_SUCCESS) {
- putMsg(result, Status.SUCCESS);
- result.put(Constants.DATA_LIST, processDefinition);
- } else {
- putMsg(result, Status.CREATE_PROCESS_TASK_RELATION_ERROR);
+ if (insertResult != Constants.EXIT_CODE_SUCCESS) {
throw new
ServiceException(Status.CREATE_PROCESS_TASK_RELATION_ERROR);
}
saveOtherRelation(loginUser, processDefinition, result,
otherParamsJson);
+
+ putMsg(result, Status.SUCCESS);
+ result.put(Constants.DATA_LIST, processDefinition);
return result;
}
- private Map<String, Object>
checkTaskDefinitionList(List<TaskDefinitionLog> taskDefinitionLogs,
- String
taskDefinitionJson) {
- Map<String, Object> result = new HashMap<>();
+ private List<TaskDefinitionLog> generateTaskDefinitionList(String
taskDefinitionJson) {
try {
- if (taskDefinitionLogs.isEmpty()) {
- logger.error("taskDefinitionJson invalid: {}",
taskDefinitionJson);
- putMsg(result, Status.DATA_IS_NOT_VALID, taskDefinitionJson);
- return result;
+ List<TaskDefinitionLog> taskDefinitionLogs =
JSONUtils.toList(taskDefinitionJson, TaskDefinitionLog.class);
+ if (CollectionUtils.isEmpty(taskDefinitionLogs)) {
+ logger.error("Generate task definition list failed, the given
taskDefinitionJson is invalided: {}",
+ taskDefinitionJson);
+ throw new ServiceException(Status.DATA_IS_NOT_VALID,
taskDefinitionJson);
}
for (TaskDefinitionLog taskDefinitionLog : taskDefinitionLogs) {
-
if
(!taskPluginManager.checkTaskParameters(ParametersNode.builder()
.taskType(taskDefinitionLog.getTaskType())
.taskParams(taskDefinitionLog.getTaskParams())
.dependence(taskDefinitionLog.getDependence())
.build())) {
- logger.error("task definition {} parameter invalid",
taskDefinitionLog.getName());
- putMsg(result, Status.PROCESS_NODE_S_PARAMETER_INVALID,
taskDefinitionLog.getName());
- return result;
+ logger.error(
+ "Generate task definition list failed, the given
task definition parameter is invalided, taskName: {}, taskDefinition: {}",
+ taskDefinitionLog.getName(), taskDefinitionLog);
+ throw new
ServiceException(Status.PROCESS_NODE_S_PARAMETER_INVALID,
taskDefinitionLog.getName());
}
}
- putMsg(result, Status.SUCCESS);
+ return taskDefinitionLogs;
+ } catch (ServiceException ex) {
+ throw ex;
} catch (Exception e) {
- result.put(Constants.STATUS,
Status.REQUEST_PARAMS_NOT_VALID_ERROR);
- result.put(Constants.MSG, e.getMessage());
+ logger.error("Generate task definition list failed, meet an
unknown exception", e);
+ throw new ServiceException(Status.REQUEST_PARAMS_NOT_VALID_ERROR);
}
- return result;
}
- private Map<String, Object>
checkTaskRelationList(List<ProcessTaskRelationLog> taskRelationList,
- String taskRelationJson,
- List<TaskDefinitionLog>
taskDefinitionLogs) {
- Map<String, Object> result = new HashMap<>();
+ private List<ProcessTaskRelationLog> generateTaskRelationList(String
taskRelationJson,
+
List<TaskDefinitionLog> taskDefinitionLogs) {
try {
- if (taskRelationList == null || taskRelationList.isEmpty()) {
- logger.error("task relation list is null");
- putMsg(result, Status.DATA_IS_NOT_VALID, taskRelationJson);
- return result;
+ List<ProcessTaskRelationLog> taskRelationList =
+ JSONUtils.toList(taskRelationJson,
ProcessTaskRelationLog.class);
+ if (CollectionUtils.isEmpty(taskRelationList)) {
+ logger.error("Generate task relation list failed the
taskRelation list is empty, taskRelationJson: {}",
+ taskRelationJson);
Review Comment:
## Logging should not be vulnerable to injection attacks
<!--SONAR_ISSUE_KEY:AYNENc3aS32ciPhy-rL6-->Change this code to not log
user-controlled data. <p>See more on <a
href="https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AYNENc3aS32ciPhy-rL6&open=AYNENc3aS32ciPhy-rL6&pullRequest=11931";>SonarCloud</a></p>
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/1291)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -317,66 +298,63 @@
logger.info("The task has not changed, so skip");
}
if (saveTaskResult == Constants.DEFINITION_FAILURE) {
- putMsg(result, Status.CREATE_TASK_DEFINITION_ERROR);
throw new ServiceException(Status.CREATE_TASK_DEFINITION_ERROR);
}
int insertVersion = processService.saveProcessDefine(loginUser,
processDefinition, Boolean.TRUE, Boolean.TRUE);
if (insertVersion == 0) {
- putMsg(result, Status.CREATE_PROCESS_DEFINITION_ERROR);
throw new ServiceException(Status.CREATE_PROCESS_DEFINITION_ERROR);
}
int insertResult = processService.saveTaskRelation(loginUser,
processDefinition.getProjectCode(),
processDefinition.getCode(),
insertVersion, taskRelationList, taskDefinitionLogs,
Boolean.TRUE);
- if (insertResult == Constants.EXIT_CODE_SUCCESS) {
- putMsg(result, Status.SUCCESS);
- result.put(Constants.DATA_LIST, processDefinition);
- } else {
- putMsg(result, Status.CREATE_PROCESS_TASK_RELATION_ERROR);
+ if (insertResult != Constants.EXIT_CODE_SUCCESS) {
throw new
ServiceException(Status.CREATE_PROCESS_TASK_RELATION_ERROR);
}
saveOtherRelation(loginUser, processDefinition, result,
otherParamsJson);
+
+ putMsg(result, Status.SUCCESS);
+ result.put(Constants.DATA_LIST, processDefinition);
return result;
}
- private Map<String, Object>
checkTaskDefinitionList(List<TaskDefinitionLog> taskDefinitionLogs,
- String
taskDefinitionJson) {
- Map<String, Object> result = new HashMap<>();
+ private List<TaskDefinitionLog> generateTaskDefinitionList(String
taskDefinitionJson) {
try {
- if (taskDefinitionLogs.isEmpty()) {
- logger.error("taskDefinitionJson invalid: {}",
taskDefinitionJson);
- putMsg(result, Status.DATA_IS_NOT_VALID, taskDefinitionJson);
- return result;
+ List<TaskDefinitionLog> taskDefinitionLogs =
JSONUtils.toList(taskDefinitionJson, TaskDefinitionLog.class);
+ if (CollectionUtils.isEmpty(taskDefinitionLogs)) {
+ logger.error("Generate task definition list failed, the given
taskDefinitionJson is invalided: {}",
+ taskDefinitionJson);
Review Comment:
## Logging should not be vulnerable to injection attacks
<!--SONAR_ISSUE_KEY:AYNENc3aS32ciPhy-rL7-->Change this code to not log
user-controlled data. <p>See more on <a
href="https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AYNENc3aS32ciPhy-rL7&open=AYNENc3aS32ciPhy-rL7&pullRequest=11931";>SonarCloud</a></p>
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/1290)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
