[ 
https://issues.apache.org/jira/browse/DRILL-5663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16087847#comment-16087847
 ] 

ASF GitHub Bot commented on DRILL-5663:
---------------------------------------

Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/874#discussion_r127529165
  
    --- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/WebServer.java 
---
    @@ -264,19 +265,26 @@ private ServerConnector createHttpsConnector() throws 
Exception {
     
         final SslContextFactory sslContextFactory = new SslContextFactory();
     
    -    if (config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH) &&
    -        
!Strings.isNullOrEmpty(config.getString(ExecConstants.HTTP_KEYSTORE_PATH))) {
    -      logger.info("Using configured SSL settings for web server");
    -      
sslContextFactory.setKeyStorePath(config.getString(ExecConstants.HTTP_KEYSTORE_PATH));
    -      
sslContextFactory.setKeyStorePassword(config.getString(ExecConstants.HTTP_KEYSTORE_PASSWORD));
    -
    -      // TrustStore and TrustStore password are optional
    -      if (config.hasPath(ExecConstants.HTTP_TRUSTSTORE_PATH)) {
    -        
sslContextFactory.setTrustStorePath(config.getString(ExecConstants.HTTP_TRUSTSTORE_PATH));
    -        if (config.hasPath(ExecConstants.HTTP_TRUSTSTORE_PASSWORD)) {
    -          
sslContextFactory.setTrustStorePassword(config.getString(ExecConstants.HTTP_TRUSTSTORE_PASSWORD));
    -        }
    +    final boolean hasPath = 
config.hasPath(ExecConstants.HTTP_KEYSTORE_PATH);
    --- End diff --
    
    To avoid this double-checking, standard practice is:
    
    * Use a blank value to indicate the value is unset.
    * Provide a default (blank) value in `drill-module.conf`
    
    Then, the code can just be:
    
    ```
    String path = config.getString(ExecConstants.HTTP_KEYSTORE_PATH).trim();
    if (! path.isEmpty()) {
      // do stuff
    ```
    
    Also, if the user is expected to set this key, please provide an example in 
`drill-override-example.conf`.


> Drillbit fails to start when only keystore path is provided without keystore 
> password.
> --------------------------------------------------------------------------------------
>
>                 Key: DRILL-5663
>                 URL: https://issues.apache.org/jira/browse/DRILL-5663
>             Project: Apache Drill
>          Issue Type: Bug
>            Reporter: Sorabh Hamirwasia
>            Assignee: Sindhuri Ramanarayan Rayavaram
>             Fix For: 1.11.0
>
>
> When we configure keystore path without keystore password inside 
> drill-override.conf for WebServer, then Drillbit fails to start. We should 
> explicitly check for either both being present or both being absent. If any 
> one of them is only present then throw startup exception for Drill.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to