From a28ffd0fdeff64a12612e60f4ebc5e6311b4b112 Mon Sep 17 00:00:00 2001 From: Chris Nauroth Date: Thu, 6 Oct 2016 12:45:11 -0700 Subject: [PATCH] HADOOP-13150. Avoid use of toString() in output of HDFS ACL shell commands. Contributed by Chris Nauroth. (cherry picked from commit 1d330fbaf6b50802750aa461640773fb788ef884) --- .../apache/hadoop/fs/permission/AclEntry.java | 24 +++++++++++++++++-- .../hadoop/fs/permission/AclEntryScope.java | 2 +- .../hadoop/fs/permission/AclEntryType.java | 23 +++++++++++++++++- .../hadoop/fs/permission/AclStatus.java | 2 +- .../apache/hadoop/fs/shell/AclCommands.java | 6 ++--- .../web/resources/AclPermissionParam.java | 23 +++++++++++++++--- .../org/apache/hadoop/hdfs/web/JsonUtil.java | 2 +- 7 files changed, 70 insertions(+), 12 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java index 45402f8a2f7..b42c36525a8 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntry.java @@ -36,7 +36,7 @@ * to create a new instance. */ @InterfaceAudience.Public -@InterfaceStability.Evolving +@InterfaceStability.Stable public class AclEntry { private final AclEntryType type; private final String name; @@ -100,13 +100,29 @@ public int hashCode() { } @Override + @InterfaceStability.Unstable public String toString() { + // This currently just delegates to the stable string representation, but it + // is permissible for the output of this method to change across versions. + return toStringStable(); + } + + /** + * Returns a string representation guaranteed to be stable across versions to + * satisfy backward compatibility requirements, such as for shell command + * output or serialization. The format of this string representation matches + * what is expected by the {@link #parseAclSpec(String, boolean)} and + * {@link #parseAclEntry(String, boolean)} methods. + * + * @return stable, backward compatible string representation + */ + public String toStringStable() { StringBuilder sb = new StringBuilder(); if (scope == AclEntryScope.DEFAULT) { sb.append("default:"); } if (type != null) { - sb.append(StringUtils.toLowerCase(type.toString())); + sb.append(StringUtils.toLowerCase(type.toStringStable())); } sb.append(':'); if (name != null) { @@ -203,6 +219,8 @@ private AclEntry(AclEntryType type, String name, FsAction permission, AclEntrySc /** * Parses a string representation of an ACL spec into a list of AclEntry * objects. Example: "user::rwx,user:foo:rw-,group::r--,other::---" + * The expected format of ACL entries in the string parameter is the same + * format produced by the {@link #toStringStable()} method. * * @param aclSpec * String representation of an ACL spec. @@ -228,6 +246,8 @@ public static List parseAclSpec(String aclSpec, /** * Parses a string representation of an ACL into a AclEntry object.
+ * The expected format of ACL entries in the string parameter is the same + * format produced by the {@link #toStringStable()} method. * * @param aclStr * String representation of an ACL.
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntryScope.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntryScope.java index 6d941e7117d..64c70aa8ca8 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntryScope.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntryScope.java @@ -24,7 +24,7 @@ * Specifies the scope or intended usage of an ACL entry. */ @InterfaceAudience.Public -@InterfaceStability.Evolving +@InterfaceStability.Stable public enum AclEntryScope { /** * An ACL entry that is inspected during permission checks to enforce diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntryType.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntryType.java index ffd62d7080b..002ead2b6a9 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntryType.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclEntryType.java @@ -24,7 +24,7 @@ * Specifies the type of an ACL entry. */ @InterfaceAudience.Public -@InterfaceStability.Evolving +@InterfaceStability.Stable public enum AclEntryType { /** * An ACL entry applied to a specific user. These ACL entries can be unnamed, @@ -55,4 +55,25 @@ public enum AclEntryType { * of the more specific ACL entry types. */ OTHER; + + @Override + @InterfaceStability.Unstable + public String toString() { + // This currently just delegates to the stable string representation, but it + // is permissible for the output of this method to change across versions. + return toStringStable(); + } + + /** + * Returns a string representation guaranteed to be stable across versions to + * satisfy backward compatibility requirements, such as for shell command + * output or serialization. + * + * @return stable, backward compatible string representation + */ + public String toStringStable() { + // The base implementation uses the enum value names, which are public API + // and therefore stable. + return super.toString(); + } } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclStatus.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclStatus.java index 9d7500a697b..131aa199435 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclStatus.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/permission/AclStatus.java @@ -31,7 +31,7 @@ * instances are immutable. Use a {@link Builder} to create a new instance. */ @InterfaceAudience.Public -@InterfaceStability.Evolving +@InterfaceStability.Stable public class AclStatus { private final String owner; private final String group; diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/AclCommands.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/AclCommands.java index 9a5404032b0..a5e386c785e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/AclCommands.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/AclCommands.java @@ -117,7 +117,7 @@ private void printAclEntriesForSingleScope(AclStatus aclStatus, } if (AclUtil.isMinimalAcl(entries)) { for (AclEntry entry: entries) { - out.println(entry); + out.println(entry.toStringStable()); } } else { for (AclEntry entry: entries) { @@ -145,10 +145,10 @@ private void printExtendedAclEntry(AclStatus aclStatus, out.println(String.format("%s\t#effective:%s", entry, effectivePerm.SYMBOL)); } else { - out.println(entry); + out.println(entry.toStringStable()); } } else { - out.println(entry); + out.println(entry.toStringStable()); } } } diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/resources/AclPermissionParam.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/resources/AclPermissionParam.java index 48f202c4f57..130c8fd6cf6 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/resources/AclPermissionParam.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/resources/AclPermissionParam.java @@ -20,11 +20,11 @@ import static org.apache.hadoop.hdfs.client.HdfsClientConfigKeys .DFS_WEBHDFS_ACL_PERMISSION_PATTERN_DEFAULT; +import java.util.Iterator; import java.util.List; import java.util.regex.Pattern; import org.apache.hadoop.fs.permission.AclEntry; -import org.apache.commons.lang.StringUtils; /** AclPermission parameter. */ public class AclPermissionParam extends StringParam { @@ -63,7 +63,24 @@ public List getAclPermission(boolean includePermission) { /** * @return parse {@code aclEntry} and return aclspec */ - private static String parseAclSpec(List aclEntry) { - return StringUtils.join(aclEntry, ","); + private static String parseAclSpec(List aclEntries) { + if (aclEntries == null) { + return null; + } + if (aclEntries.isEmpty()) { + return ""; + } + if (aclEntries.size() == 1) { + AclEntry entry = aclEntries.get(0); + return entry == null ? "" : entry.toStringStable(); + } + StringBuilder sb = new StringBuilder(); + Iterator iter = aclEntries.iterator(); + sb.append(iter.next().toStringStable()); + while (iter.hasNext()) { + AclEntry entry = iter.next(); + sb.append(',').append(entry == null ? "" : entry.toStringStable()); + } + return sb.toString(); } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/JsonUtil.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/JsonUtil.java index 33477eecdb9..ac9ab77dd6c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/JsonUtil.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/JsonUtil.java @@ -360,7 +360,7 @@ public static String toJsonString(final AclStatus status) { final List stringEntries = new ArrayList<>(); for (AclEntry entry : status.getEntries()) { - stringEntries.add(entry.toString()); + stringEntries.add(entry.toStringStable()); } m.put("entries", stringEntries);