skysiders opened a new pull request, #3894:
URL: https://github.com/apache/hive/pull/3894

   In the QueryResultsCache function of class QueryResultsCache, there is the 
following code segment
   
   ```java
     private QueryResultsCache(HiveConf configuration) throws IOException {
       ......
       FileSystem fs = cacheDirPath.getFileSystem(conf);
       FsPermission fsPermission = new FsPermission("700");
       fs.mkdirs(cacheDirPath, fsPermission);
       ......
   }
   ```
   It can be seen that the function will use the mkdirs to create cacheDirPath, 
and the parameters passed in include the path variable cacheDirPath and a 
permission 700. But we haven't confirmed whether the permission is correctly 
assigned to the file.
   
   The above question is raised because there are two mkdir functions of 
hadoop,`mkdirs(Path f, FsPermission permission)` with `mkdirs(FileSystem fs, 
Path dir, FsPermission permission)`and the first one is used here. The 
permissions of this function will be affected by the underlying umask. Although 
700 here will hardly be affected by umask, but I think from a rigorous point of 
view, we should have one more permission check and permission grant here. So I 
judge whether the file permission is correctly granted by detecting the 
influence of umask on the permission, and if not, give it the correct 
permission through setPermission.
   
   ```java
       if 
(!permission.equals(permission.applyUMask(FsPermission.getUMask(conf)))) {
           fs.setPermission(dir, permission);
       }
   ```
   
   And I find same issue in other three methods here.
   In class Context
   ```java
   private Path getScratchDir(String scheme, String authority,
         boolean mkdir, String scratchDir) {
             ......
             FileSystem fs = dirPath.getFileSystem(conf);
             dirPath = new Path(fs.makeQualified(dirPath).toString());
             FsPermission fsPermission = new FsPermission(scratchDirPermission);
   
             if (!fs.mkdirs(dirPath, fsPermission)) {
               throw new RuntimeException("Cannot make directory: "
                   + dirPath.toString());
             ......
     }
   ```
   In class SessionState
   ```java
     static void createPath(HiveConf conf, Path path, String permission, 
boolean isLocal,
         boolean isCleanUp) throws IOException {
       FsPermission fsPermission = new FsPermission(permission);
       FileSystem fs;
       ......
       if (!fs.mkdirs(path, fsPermission)) {
         throw new IOException("Failed to create directory " + path + " on fs " 
+ fs.getUri());
       }
       ......
     }
   ```
   
   and in class TezSessionState
   ```java
   private Path createTezDir(String sessionId, String suffix) throws 
IOException {
       ......
       Path tezDir = new Path(hdfsScratchDir, TEZ_DIR);
       FileSystem fs = tezDir.getFileSystem(conf);
       FsPermission fsPermission = new FsPermission(HiveConf.getVar(conf, 
HiveConf.ConfVars.SCRATCHDIRPERMISSION));
       fs.mkdirs(tezDir, fsPermission);
       ......
     }
   ```


-- 
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: gitbox-unsubscr...@hive.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to