github-advanced-security[bot] commented on code in PR #15997:
URL:
https://github.com/apache/dolphinscheduler/pull/15997#discussion_r1602536169
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java:
##########
@@ -43,13 +44,12 @@
* @param type type
* @param pid parent id
* @param currentDir current directory
- * @return create directory result
*/
- Result<Object> createDirectory(User loginUser,
- String name,
- ResourceType type,
- int pid,
- String currentDir);
+ void createDirectory(User loginUser,
+ String name,
+ ResourceType type,
+ int pid,
Review Comment:
## Useless parameter
The parameter 'pid' is never used.
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/4141)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java:
##########
@@ -349,79 +321,66 @@
}
ApiServerMetrics.recordApiResourceUploadSize(file.getSize());
- return result;
+ return;
}
// get the path of dest file in hdfs
String destHdfsFileName = fullName;
try {
log.info("start copy {} -> {}", originFullName, destHdfsFileName);
storageOperate.copy(originFullName, destHdfsFileName, true, true);
- putMsg(result, Status.SUCCESS);
} catch (Exception e) {
log.error(MessageFormat.format(" copy {0} -> {1} fail",
originFullName, destHdfsFileName), e);
- putMsg(result, Status.HDFS_COPY_FAIL);
throw new ServiceException(
MessageFormat.format(Status.HDFS_COPY_FAIL.getMsg(),
originFullName, destHdfsFileName));
}
-
- return result;
}
- private Result<Object> verifyFile(String name, ResourceType type,
MultipartFile file) {
- Result<Object> result = new Result<>();
- putMsg(result, Status.SUCCESS);
-
+ private void verifyFile(String name, ResourceType type, MultipartFile
file) {
if (FileUtils.directoryTraversal(name)) {
log.warn("Parameter file alias name verify failed,
fileAliasName:{}.", RegexUtils.escapeNRT(name));
- putMsg(result, Status.VERIFY_PARAMETER_NAME_FAILED);
- return result;
+ throw new ServiceException(Status.VERIFY_PARAMETER_NAME_FAILED);
}
- if (file != null &&
FileUtils.directoryTraversal(Objects.requireNonNull(file.getOriginalFilename())))
{
- log.warn("File original name verify failed, fileOriginalName:{}.",
+ if (file == null) {
+ return;
+ }
+
+ // file is empty
+ if (file.isEmpty()) {
+ log.warn("Parameter file is empty, fileOriginalName:{}.",
RegexUtils.escapeNRT(file.getOriginalFilename()));
- putMsg(result, Status.VERIFY_PARAMETER_NAME_FAILED);
- return result;
+ throw new ServiceException(Status.RESOURCE_FILE_IS_EMPTY);
}
- if (file != null) {
- // file is empty
- if (file.isEmpty()) {
- log.warn("Parameter file is empty, fileOriginalName:{}.",
- RegexUtils.escapeNRT(file.getOriginalFilename()));
- putMsg(result, Status.RESOURCE_FILE_IS_EMPTY);
- return result;
- }
+ if
(FileUtils.directoryTraversal(Objects.requireNonNull(file.getOriginalFilename())))
{
+ log.warn("File original name verify failed, fileOriginalName:{}.",
+ RegexUtils.escapeNRT(file.getOriginalFilename()));
+ throw new ServiceException(Status.VERIFY_PARAMETER_NAME_FAILED);
+ }
- // file suffix
- String fileSuffix =
Files.getFileExtension(file.getOriginalFilename());
- String nameSuffix = Files.getFileExtension(name);
+ // file suffix
+ String fileSuffix = Files.getFileExtension(file.getOriginalFilename());
+ String nameSuffix = Files.getFileExtension(name);
- // determine file suffix
- if (!fileSuffix.equalsIgnoreCase(nameSuffix)) {
- // rename file suffix and original suffix must be consistent
- log.warn("Rename file suffix and original suffix must be
consistent, fileOriginalName:{}.",
- RegexUtils.escapeNRT(file.getOriginalFilename()));
- putMsg(result, Status.RESOURCE_SUFFIX_FORBID_CHANGE);
- return result;
- }
+ // determine file suffix
+ if (!fileSuffix.equalsIgnoreCase(nameSuffix)) {
+ // rename file suffix and original suffix must be consistent
+ log.warn("Rename file suffix and original suffix must be
consistent, fileOriginalName:{}.",
+ RegexUtils.escapeNRT(file.getOriginalFilename()));
+ throw new ServiceException(Status.RESOURCE_SUFFIX_FORBID_CHANGE);
+ }
- // If resource type is UDF, only jar packages are allowed to be
uploaded, and the suffix must be .jar
- if (Constants.UDF.equals(type.name()) &&
!JAR.equalsIgnoreCase(fileSuffix)) {
- log.warn(Status.UDF_RESOURCE_SUFFIX_NOT_JAR.getMsg());
- putMsg(result, Status.UDF_RESOURCE_SUFFIX_NOT_JAR);
- return result;
- }
- if (file.getSize() > Constants.MAX_FILE_SIZE) {
- log.warn(
- "Resource file size is larger than max file size,
fileOriginalName:{}, fileSize:{}, maxFileSize:{}.",
- RegexUtils.escapeNRT(file.getOriginalFilename()),
file.getSize(), Constants.MAX_FILE_SIZE);
- putMsg(result, Status.RESOURCE_SIZE_EXCEED_LIMIT);
- return result;
- }
+ // If resource type is UDF, only jar packages are allowed to be
uploaded, and the suffix must be .jar
+ if (Constants.UDF.equals(type.name()) &&
!JAR.equalsIgnoreCase(fileSuffix)) {
+ throw new ServiceException(Status.UDF_RESOURCE_SUFFIX_NOT_JAR);
+ }
+ if (file.getSize() > Constants.MAX_FILE_SIZE) {
+ log.warn(
+ "Resource file size is larger than max file size,
fileOriginalName:{}, fileSize:{}, maxFileSize:{}.",
+ RegexUtils.escapeNRT(file.getOriginalFilename()),
file.getSize(), Constants.MAX_FILE_SIZE);
Review Comment:
## Log Injection
This log entry depends on a [user-provided value](1).
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/4154)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java:
##########
@@ -349,79 +321,66 @@
}
ApiServerMetrics.recordApiResourceUploadSize(file.getSize());
- return result;
+ return;
}
// get the path of dest file in hdfs
String destHdfsFileName = fullName;
try {
log.info("start copy {} -> {}", originFullName, destHdfsFileName);
storageOperate.copy(originFullName, destHdfsFileName, true, true);
- putMsg(result, Status.SUCCESS);
} catch (Exception e) {
log.error(MessageFormat.format(" copy {0} -> {1} fail",
originFullName, destHdfsFileName), e);
- putMsg(result, Status.HDFS_COPY_FAIL);
throw new ServiceException(
MessageFormat.format(Status.HDFS_COPY_FAIL.getMsg(),
originFullName, destHdfsFileName));
}
-
- return result;
}
- private Result<Object> verifyFile(String name, ResourceType type,
MultipartFile file) {
- Result<Object> result = new Result<>();
- putMsg(result, Status.SUCCESS);
-
+ private void verifyFile(String name, ResourceType type, MultipartFile
file) {
if (FileUtils.directoryTraversal(name)) {
log.warn("Parameter file alias name verify failed,
fileAliasName:{}.", RegexUtils.escapeNRT(name));
- putMsg(result, Status.VERIFY_PARAMETER_NAME_FAILED);
- return result;
+ throw new ServiceException(Status.VERIFY_PARAMETER_NAME_FAILED);
}
- if (file != null &&
FileUtils.directoryTraversal(Objects.requireNonNull(file.getOriginalFilename())))
{
- log.warn("File original name verify failed, fileOriginalName:{}.",
+ if (file == null) {
+ return;
+ }
+
+ // file is empty
+ if (file.isEmpty()) {
+ log.warn("Parameter file is empty, fileOriginalName:{}.",
RegexUtils.escapeNRT(file.getOriginalFilename()));
- putMsg(result, Status.VERIFY_PARAMETER_NAME_FAILED);
- return result;
+ throw new ServiceException(Status.RESOURCE_FILE_IS_EMPTY);
}
- if (file != null) {
- // file is empty
- if (file.isEmpty()) {
- log.warn("Parameter file is empty, fileOriginalName:{}.",
- RegexUtils.escapeNRT(file.getOriginalFilename()));
- putMsg(result, Status.RESOURCE_FILE_IS_EMPTY);
- return result;
- }
+ if
(FileUtils.directoryTraversal(Objects.requireNonNull(file.getOriginalFilename())))
{
+ log.warn("File original name verify failed, fileOriginalName:{}.",
+ RegexUtils.escapeNRT(file.getOriginalFilename()));
Review Comment:
## Log Injection
This log entry depends on a [user-provided value](1).
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/4147)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/ResourcesService.java:
##########
@@ -112,33 +111,30 @@
* @param type resource type
* @return resource list
*/
- Result<Object> queryResourceByProgramType(User loginUser, ResourceType
type, ProgramType programType);
+ List<ResourceComponent> queryResourceByProgramType(User loginUser,
ResourceType type, ProgramType programType);
/**
* delete resource
*
* @param loginUser login user
- * @return delete result code
* @throws IOException exception
*/
- Result<Object> delete(User loginUser, String fullName, String tenantCode)
throws IOException;
+ void delete(User loginUser, String fullName, String tenantCode) throws
IOException;
/**
* verify resource by name and type
* @param loginUser login user
* @param fullName resource full name
* @param type resource type
- * @return true if the resource name not exists, otherwise return false
*/
- Result<Object> verifyResourceName(String fullName, ResourceType type, User
loginUser);
+ void verifyResourceName(String fullName, ResourceType type, User
loginUser);
Review Comment:
## Useless parameter
The parameter 'loginUser' is never used.
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/4140)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java:
##########
@@ -349,79 +321,66 @@
}
ApiServerMetrics.recordApiResourceUploadSize(file.getSize());
- return result;
+ return;
}
// get the path of dest file in hdfs
String destHdfsFileName = fullName;
try {
log.info("start copy {} -> {}", originFullName, destHdfsFileName);
storageOperate.copy(originFullName, destHdfsFileName, true, true);
- putMsg(result, Status.SUCCESS);
} catch (Exception e) {
log.error(MessageFormat.format(" copy {0} -> {1} fail",
originFullName, destHdfsFileName), e);
- putMsg(result, Status.HDFS_COPY_FAIL);
throw new ServiceException(
MessageFormat.format(Status.HDFS_COPY_FAIL.getMsg(),
originFullName, destHdfsFileName));
}
-
- return result;
}
- private Result<Object> verifyFile(String name, ResourceType type,
MultipartFile file) {
- Result<Object> result = new Result<>();
- putMsg(result, Status.SUCCESS);
-
+ private void verifyFile(String name, ResourceType type, MultipartFile
file) {
if (FileUtils.directoryTraversal(name)) {
log.warn("Parameter file alias name verify failed,
fileAliasName:{}.", RegexUtils.escapeNRT(name));
- putMsg(result, Status.VERIFY_PARAMETER_NAME_FAILED);
- return result;
+ throw new ServiceException(Status.VERIFY_PARAMETER_NAME_FAILED);
}
- if (file != null &&
FileUtils.directoryTraversal(Objects.requireNonNull(file.getOriginalFilename())))
{
- log.warn("File original name verify failed, fileOriginalName:{}.",
+ if (file == null) {
+ return;
+ }
+
+ // file is empty
+ if (file.isEmpty()) {
+ log.warn("Parameter file is empty, fileOriginalName:{}.",
RegexUtils.escapeNRT(file.getOriginalFilename()));
- putMsg(result, Status.VERIFY_PARAMETER_NAME_FAILED);
- return result;
+ throw new ServiceException(Status.RESOURCE_FILE_IS_EMPTY);
}
- if (file != null) {
- // file is empty
- if (file.isEmpty()) {
- log.warn("Parameter file is empty, fileOriginalName:{}.",
- RegexUtils.escapeNRT(file.getOriginalFilename()));
- putMsg(result, Status.RESOURCE_FILE_IS_EMPTY);
- return result;
- }
+ if
(FileUtils.directoryTraversal(Objects.requireNonNull(file.getOriginalFilename())))
{
+ log.warn("File original name verify failed, fileOriginalName:{}.",
+ RegexUtils.escapeNRT(file.getOriginalFilename()));
+ throw new ServiceException(Status.VERIFY_PARAMETER_NAME_FAILED);
+ }
- // file suffix
- String fileSuffix =
Files.getFileExtension(file.getOriginalFilename());
- String nameSuffix = Files.getFileExtension(name);
+ // file suffix
+ String fileSuffix = Files.getFileExtension(file.getOriginalFilename());
+ String nameSuffix = Files.getFileExtension(name);
- // determine file suffix
- if (!fileSuffix.equalsIgnoreCase(nameSuffix)) {
- // rename file suffix and original suffix must be consistent
- log.warn("Rename file suffix and original suffix must be
consistent, fileOriginalName:{}.",
- RegexUtils.escapeNRT(file.getOriginalFilename()));
- putMsg(result, Status.RESOURCE_SUFFIX_FORBID_CHANGE);
- return result;
- }
+ // determine file suffix
+ if (!fileSuffix.equalsIgnoreCase(nameSuffix)) {
+ // rename file suffix and original suffix must be consistent
+ log.warn("Rename file suffix and original suffix must be
consistent, fileOriginalName:{}.",
+ RegexUtils.escapeNRT(file.getOriginalFilename()));
Review Comment:
## Log Injection
This log entry depends on a [user-provided value](1).
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/4148)
--
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]
