Github user srdo commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2430#discussion_r152371328
  
    --- Diff: 
storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/handler/LogviewerLogSearchHandler.java
 ---
    @@ -97,8 +98,14 @@ public LogviewerLogSearchHandler(Map<String, Object> 
stormConf, String logRoot,
             this.logRoot = logRoot;
             this.daemonLogRoot = daemonLogRoot;
             this.resourceAuthorizer = resourceAuthorizer;
    -
    -        this.logviewerPort = 
ObjectReader.getInt(stormConf.get(DaemonConfig.LOGVIEWER_PORT));
    +        Integer httpsPort = 
ObjectReader.getInt(stormConf.get(DaemonConfig.LOGVIEWER_HTTPS_PORT), 0);
    --- End diff --
    
    @revans2 Your code snippet is more or less what I had in mind. I think 
@Ethanlm has a point too though. I don't think the validation annotations are 
checked anywhere in the Logviewer process.
    
    The stormConf seems to be read here 
https://github.com/apache/storm/blob/master/storm-webapp/src/main/java/org/apache/storm/daemon/logviewer/LogviewerServer.java#L158
 but I don't see any references to ConfigValidation in storm-webapp. 
    
    I think we should consider running ConfigValidation.validateFields in the 
LogviewerServer main, so we can trust that the annotation constraints are 
enforced.


---

Reply via email to