kezhenxu94 commented on a change in pull request #5983:
URL: https://github.com/apache/dolphinscheduler/pull/5983#discussion_r693479795
##########
File path:
dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/log/LoggerRequestProcessor.java
##########
@@ -96,26 +99,26 @@ public void process(Channel channel, Command command) {
for (String line : lines) {
builder.append(line + "\r\n");
}
- RollViewLogResponseCommand rollViewLogRequestResponse = new
RollViewLogResponseCommand(builder.toString());
-
channel.writeAndFlush(rollViewLogRequestResponse.convert2Command(command.getOpaque()));
+ RollViewLogResponseCommand rollViewLogRequestResponse = new
RollViewLogResponseCommand(
+ builder.toString());
+ channel
+
.writeAndFlush(rollViewLogRequestResponse.convert2Command(command.getOpaque()));
break;
case REMOVE_TAK_LOG_REQUEST:
RemoveTaskLogRequestCommand removeTaskLogRequest =
JSONUtils.parseObject(
command.getBody(), RemoveTaskLogRequestCommand.class);
-
- String taskLogPath = removeTaskLogRequest.getPath();
-
- File taskLogFile = new File(taskLogPath);
+ List<String> taskLogPaths = removeTaskLogRequest.getPath();
+ OsSystemNativeCommand os = new LinuxSystem();
+ String cmd = "";
+ for (String path : taskLogPaths) {
+ cmd += os.deleteCmd() + path + ";";
+ }
Review comment:
> 第一，ds 的使用场景并不是对外的，而是对内的。
Company employees are totally possible to destroy their servers / databases.
>
第二，如果存在安全漏洞，其实基本可以认为是不太可能的。因为删除的文件名是来自数据库内自动生成的文件，如果会有可注入的风险，就是因为数据的库的信息被串改了。完全属于没病呻吟的情况。
Do you mean if the database is hacked, it's reasonable that your servers
should be also hacked? You should limit the exploding radius instead of
expanding it.
> 第三，作为DS 4个版本的使用者，我要说一句，现在批量删除的逻辑是有可能导致 这个系统异常的。日志本身在运维阶段属于可以被定期清理的部分。如果UI
操作删除批量日志能导致使用者直接面对系统宕机，系统本身的稳定性是需要紧急优化的。
You said the original logic is possible to cause exceptions, but didn't
explain what exceptions might be caused and why your changes will resolve it.
> 第四，非得强调jdk 提供的API 才是最安全的这个情况。个人不是没有考虑过服用jdk 的API 。
但是在牺牲一次性删除的方案下，找个看着可能对的方法去做，并不是解决问题的方式。当然可以说开启异步线程去清理。。。但是有必要吗？
> 请不要按照开发人员的方式去思考用户该怎么用，因为官方文档里也没有限制用户可嵌套的层级。
> 后期我个人带着团队用的时候都是直接删除数据库的。
Please don't take it personally and don't be so aggressive, reviewers just
give their opinions and raise their concerns, giving your reasons is just
enough, that's how open source community works, again, don't take it
personally, don't be aggressive.
--
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]
