diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-4685.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-4685.txt index 774c23ff77..ada49372e0 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-4685.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-4685.txt @@ -34,3 +34,6 @@ HDFS-4685 (Unreleased) HDFS-5737. Replacing only the default ACL can fail to copy unspecified base entries from the access ACL. (cnauroth) + + HDFS-5739. ACL RPC must allow null name or unspecified permissions in ACL + entries. (cnauroth) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java index 8986e94abf..8604191a65 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelper.java @@ -1928,7 +1928,7 @@ private static AclEntryType convert(AclEntryTypeProto v) { } private static FsActionProto convert(FsAction v) { - return FsActionProto.valueOf(v.ordinal()); + return FsActionProto.valueOf(v != null ? v.ordinal() : 0); } private static FsAction convert(FsActionProto v) { @@ -1939,9 +1939,14 @@ public static List convertAclEntryProto( List aclSpec) { ArrayList r = Lists.newArrayListWithCapacity(aclSpec.size()); for (AclEntry e : aclSpec) { - r.add(AclEntryProto.newBuilder().setType(convert(e.getType())) - .setName(e.getName()).setPermissions(convert(e.getPermission())) - .setScope(convert(e.getScope())).build()); + AclEntryProto.Builder builder = AclEntryProto.newBuilder(); + builder.setType(convert(e.getType())); + builder.setScope(convert(e.getScope())); + builder.setPermissions(convert(e.getPermission())); + if (e.getName() != null) { + builder.setName(e.getName()); + } + r.add(builder.build()); } return r; } @@ -1949,9 +1954,14 @@ public static List convertAclEntryProto( public static List convertAclEntry(List aclSpec) { ArrayList r = Lists.newArrayListWithCapacity(aclSpec.size()); for (AclEntryProto e : aclSpec) { - r.add(new AclEntry.Builder().setType(convert(e.getType())) - .setName(e.getName()).setPermission(convert(e.getPermissions())) - .setScope(convert(e.getScope())).build()); + AclEntry.Builder builder = new AclEntry.Builder(); + builder.setType(convert(e.getType())); + builder.setScope(convert(e.getScope())); + builder.setPermission(convert(e.getPermissions())); + if (e.hasName()) { + builder.setName(e.getName()); + } + r.add(builder.build()); } return r; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/acl.proto b/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/acl.proto index 4511cc736a..4b68edf219 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/acl.proto +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/acl.proto @@ -50,7 +50,7 @@ message AclEntryProto { required AclEntryTypeProto type = 1; required AclEntryScopeProto scope = 2; required FsActionProto permissions = 3; - required string name = 4; + optional string name = 4; } message AclStatusProto { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java index c2f70faa58..8d12c185af 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java @@ -589,16 +589,27 @@ public void testChecksumTypeProto() { @Test public void testAclEntryProto() { - AclEntry e = new AclEntry.Builder().setName("test") + // All fields populated. + AclEntry e1 = new AclEntry.Builder().setName("test") .setPermission(FsAction.READ_EXECUTE).setScope(AclEntryScope.DEFAULT) .setType(AclEntryType.OTHER).build(); - AclEntry[] lists = new AclEntry[] { e }; - - Assert.assertArrayEquals( - lists, - Lists.newArrayList( - PBHelper.convertAclEntry(PBHelper.convertAclEntryProto(Lists - .newArrayList(e)))).toArray()); + // No name. + AclEntry e2 = new AclEntry.Builder().setScope(AclEntryScope.ACCESS) + .setType(AclEntryType.USER).setPermission(FsAction.ALL).build(); + // No permission, which will default to the 0'th enum element. + AclEntry e3 = new AclEntry.Builder().setScope(AclEntryScope.ACCESS) + .setType(AclEntryType.USER).setName("test").build(); + AclEntry[] expected = new AclEntry[] { e1, e2, + new AclEntry.Builder() + .setScope(e3.getScope()) + .setType(e3.getType()) + .setName(e3.getName()) + .setPermission(FsAction.NONE) + .build() }; + AclEntry[] actual = Lists.newArrayList( + PBHelper.convertAclEntry(PBHelper.convertAclEntryProto(Lists + .newArrayList(e1, e2, e3)))).toArray(new AclEntry[0]); + Assert.assertArrayEquals(expected, actual); } @Test