Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-3491

HttpFs does not set permissions correctly

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-alpha
    • Fix Version/s: 2.0.2-alpha
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      HttpFs seems to have these problems:

      1. can't set permissions to 777 at file creation or 1777 with setpermission
      2. does not accept 01777 permissions (which is valid in WebHdfs)

      WebHdfs
      curl -X PUT "http://localhost:50070/webhdfs/v1/tmp/test-perm-webhdfs?permission=1777&op=MKDIRS&user.name=hue&doas=hue"

      {"boolean":true}

      curl "http://localhost:50070/webhdfs/v1/tmp/test-perm-webhdfs?op=GETFILESTATUS&user.name=hue&doas=hue"
      {"FileStatus":{"accessTime":0,"blockSize":0,"group":"supergroup","length":0,"modificationTime":1338581075040,"owner":"hue","pathSuffix":"","permission":"1777","replication":0,"type":"DIRECTORY"}}

      curl -X PUT "http://localhost:50070/webhdfs/v1/tmp/test-perm-webhdfs?permission=01777&op=MKDIRS&user.name=hue&doas=hue"

      {"boolean":true}

      HttpFs
      curl -X PUT "http://localhost:14000/webhdfs/v1/tmp/test-perm-httpfs?permission=1777&op=MKDIRS&user.name=hue&doas=hue"

      {"boolean":true}

      curl "http://localhost:14000/webhdfs/v1/tmp/test-perm-httpfs?op=GETFILESTATUS&user.name=hue&doas=hue"
      {"FileStatus":{"pathSuffix":"","type":"DIRECTORY","length":0,"owner":"hue","group":"supergroup","permission":"755","accessTime":0,"modificationTime":1338580912205,"blockSize":0,"replication":0}}

      curl -X PUT "http://localhost:14000/webhdfs/v1/tmp/test-perm-httpfs?op=SETPERMISSION&PERMISSION=1777&user.name=hue&doas=hue"
      curl "http://localhost:14000/webhdfs/v1/tmp/test-perm-httpfs?op=GETFILESTATUS&user.name=hue&doas=hue"
      {"FileStatus":{"pathSuffix":"","type":"DIRECTORY","length":0,"owner":"hue","group":"supergroup","permission":"777","accessTime":0,"modificationTime":1338581075040,"blockSize":0,"replication":0}}

      curl -X PUT "http://localhost:14000/webhdfs/v1/tmp/test-perm-httpfs?permission=01777&op=MKDIRS&user.name=hue&doas=hue"
      {"RemoteException":{"message":"java.lang.IllegalArgumentException: Parameter [permission], invalid value [01777], value must be [default|[0-1]?[0-7][0-7][0-7]]","exception":"QueryParamException","javaClassName":"com.sun.jersey.api.ParamException$QueryParamException"}}

      1. HDFS-3491.patch
        10 kB
        Alejandro Abdelnur
      2. HDFS-3491.patch
        11 kB
        Alejandro Abdelnur
      3. HDFS-3491.patch
        11 kB
        Alejandro Abdelnur

        Issue Links

          Activity

          Hide
          Alejandro Abdelnur added a comment -

          The permission param type in httpfs should be changed from a string with a regex to short.

          Show
          Alejandro Abdelnur added a comment - The permission param type in httpfs should be changed from a string with a regex to short.
          Hide
          Alejandro Abdelnur added a comment -

          changed PermissionParam to be a subclass of ShortParam. ShortParam now supports radix (which PermissionParam sets to 8).

          Show
          Alejandro Abdelnur added a comment - changed PermissionParam to be a subclass of ShortParam. ShortParam now supports radix (which PermissionParam sets to 8).
          Hide
          Eli Collins added a comment -

          Change looks good, needs a test.

          Show
          Eli Collins added a comment - Change looks good, needs a test.
          Hide
          Alejandro Abdelnur added a comment -

          updated patch adds testcase for octal shorts.

          Show
          Alejandro Abdelnur added a comment - updated patch adds testcase for octal shorts.
          Hide
          Eli Collins added a comment -

          The TestParam addition is good but what test covers that the values passed to the "permission" parameter get reflected when accessing the same file via FileSystem?

          Show
          Eli Collins added a comment - The TestParam addition is good but what test covers that the values passed to the "permission" parameter get reflected when accessing the same file via FileSystem?
          Hide
          Alejandro Abdelnur added a comment -

          The bug was not exposed when using a client FileSystem implementation but when using the REST API directly. The client FileSystem did not see the issue as the string conversion of 1777 is not adding the '0' in front of it.

          Show
          Alejandro Abdelnur added a comment - The bug was not exposed when using a client FileSystem implementation but when using the REST API directly. The client FileSystem did not see the issue as the string conversion of 1777 is not adding the '0' in front of it.
          Hide
          Eli Collins added a comment -

          So there's no test that covers that the REST API actually does the intended operation? Thought there was a test for that.

          Show
          Eli Collins added a comment - So there's no test that covers that the REST API actually does the intended operation? Thought there was a test for that.
          Hide
          Alejandro Abdelnur added a comment -

          The HTTP REST API is fully tested using webhdfs filesystem client implementations. You don't see this issue there. This particular bug is exposed by using directly the HTTP REST API with curl. The type of the permission parameter was string with a regex mask that was not accounting for a leading zero for the '01777' mask. This patch changes the permission parameter to be a short radix 8 and the testcase verifies the parameter parsing handles the mask of the bug.

          Show
          Alejandro Abdelnur added a comment - The HTTP REST API is fully tested using webhdfs filesystem client implementations. You don't see this issue there. This particular bug is exposed by using directly the HTTP REST API with curl. The type of the permission parameter was string with a regex mask that was not accounting for a leading zero for the '01777' mask. This patch changes the permission parameter to be a short radix 8 and the testcase verifies the parameter parsing handles the mask of the bug.
          Hide
          Eli Collins added a comment -

          Ah, that makes sense. +1 pending jenkins

          Show
          Eli Collins added a comment - Ah, that makes sense. +1 pending jenkins
          Hide
          Alejandro Abdelnur added a comment -

          reuploading same patch for our friend jenkins to take notice.

          Show
          Alejandro Abdelnur added a comment - reuploading same patch for our friend jenkins to take notice.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12533916/HDFS-3491.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 1 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-httpfs.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2721//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2721//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12533916/HDFS-3491.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-httpfs. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2721//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2721//console This message is automatically generated.
          Hide
          Alejandro Abdelnur added a comment -

          committed to trunk and branch-2

          Show
          Alejandro Abdelnur added a comment - committed to trunk and branch-2
          Hide
          Romain Rigaux added a comment -

          This is still broken. Octal is correctly accepted but not applied when during a MKDIR:

          romain@runreal:~/projects/hue$ curl -X PUT "http://localhost:14000/webhdfs/v1/tmp/test-perm-httpfs?permission=01777&op=MKDIRS&user.name=hue&doas=hue"

          {"boolean":true}

          romain@runreal:~/projects/hue$ curl "http://localhost:14000/webhdfs/v1/tmp/test-perm-httpfs?op=GETFILESTATUS&user.name=hue&doas=hue"
          {"FileStatus":{"pathSuffix":"","type":"DIRECTORY","length":0,"owner":"hue","group":"supergroup","permission":"755","accessTime":0,"modificationTime":1345658456869,"blockSize":0,"replication":0}}

          Show
          Romain Rigaux added a comment - This is still broken. Octal is correctly accepted but not applied when during a MKDIR: romain@runreal:~/projects/hue$ curl -X PUT "http://localhost:14000/webhdfs/v1/tmp/test-perm-httpfs?permission=01777&op=MKDIRS&user.name=hue&doas=hue" {"boolean":true} romain@runreal:~/projects/hue$ curl "http://localhost:14000/webhdfs/v1/tmp/test-perm-httpfs?op=GETFILESTATUS&user.name=hue&doas=hue" {"FileStatus":{"pathSuffix":"","type":"DIRECTORY","length":0,"owner":"hue","group":"supergroup","permission":"755","accessTime":0,"modificationTime":1345658456869,"blockSize":0,"replication":0}}

            People

            • Assignee:
              Alejandro Abdelnur
              Reporter:
              Romain Rigaux
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development