an5er commented on code in PR #15758:
URL: 
https://github.com/apache/dolphinscheduler/pull/15758#discussion_r1536634212


##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/loop/template/http/parser/HttpTaskDefinitionParser.java:
##########
@@ -60,7 +60,7 @@
     }
 
     protected @NonNull LoopTaskYamlDefinition parseYamlConfigFile(@NonNull 
String yamlConfigFile) throws IOException {
-        Yaml yaml = new Yaml(new Constructor(LoopTaskYamlDefinition.class));
+        Yaml yaml = new Yaml(new SafeConstructor());

Review Comment:
   First of all I think the code `Yaml yaml = new Yaml(new 
Constructor(LoopTaskYamlDefinition.class));` can't be changed to `Yaml yaml = 
new Yaml(new SafeConstructor());` . 
   I would suggest to introduce the class 
[ClassFilterConstructor.java](https://github.com/apache/skywalking/blob/master/oap-server/server-library/library-util/src/main/java/org/apache/skywalking/oap/server/library/util/yaml/ClassFilterConstructor.java#L28),
 and then to replace the code in `AbstractK8sTaskExecutor` to be changed to
   ```
   import java.util.List;
   
   Yaml yaml = new Yaml(new ClassFilterConstructor(new Class[] {
           List.class
   })).
   ```
   This is for visibility, because here we expect the user's input to be of 
type list, you can also remove this map as it is already defined in the 
SafeConstructor
   ```
   Yaml yaml = new Yaml(new ClassFilterConstructor(new Class[] {
   }));
   ```
   I didn't test the security of the parseYamlConfigFile function, so if you 
want to change it, you can also change to
   ```
   Yaml yaml = new Yaml(new ClassFilterConstructor(new Class[] {
                   LoopTaskYamlDefinition.class
           }));
   ```
   
   
   



-- 
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]

Reply via email to