From 3cde6931cb5055a9d92503f4ecefa35571e7b07f Mon Sep 17 00:00:00 2001 From: Arpit Agarwal Date: Sun, 1 Nov 2015 15:35:02 -0800 Subject: [PATCH] HDFS-9343. Empty caller context considered invalid. (Contributed by Mingliang Liu) --- .../java/org/apache/hadoop/ipc/CallerContext.java | 13 ++++++++----- .../java/org/apache/hadoop/util/ProtoUtil.java | 2 +- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 +++ .../hadoop/hdfs/server/namenode/FSNamesystem.java | 8 ++++---- .../hdfs/server/namenode/TestAuditLogger.java | 15 ++++++++++++--- 5 files changed, 28 insertions(+), 13 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java index 8be7e355123..b197575e052 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java @@ -44,6 +44,7 @@ public class CallerContext { * {@link org.apache.hadoop.fs.CommonConfigurationKeysPublic#HADOOP_CALLER_CONTEXT_MAX_SIZE_DEFAULT} */ private final String context; + /** The caller's signature for validation. * * The signature is optional. The null or empty signature will be abandoned. @@ -58,10 +59,6 @@ public class CallerContext { this.signature = builder.signature; } - public boolean isValid() { - return context != null; - } - public String getContext() { return context; } @@ -71,6 +68,11 @@ public class CallerContext { null : Arrays.copyOf(signature, signature.length); } + @InterfaceAudience.Private + public boolean isContextValid() { + return context != null && !context.isEmpty(); + } + @Override public int hashCode() { return new HashCodeBuilder().append(context).toHashCode(); @@ -92,9 +94,10 @@ public class CallerContext { .isEquals(); } } + @Override public String toString() { - if (!isValid()) { + if (!isContextValid()) { return ""; } String str = context; diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ProtoUtil.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ProtoUtil.java index 4bfcd66bf5c..1a5acbab6ec 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ProtoUtil.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/ProtoUtil.java @@ -180,7 +180,7 @@ public abstract class ProtoUtil { // Add caller context if it is not null CallerContext callerContext = CallerContext.getCurrent(); - if (callerContext != null && callerContext.isValid()) { + if (callerContext != null && callerContext.isContextValid()) { RPCCallerContextProto.Builder contextBuilder = RPCCallerContextProto .newBuilder().setContext(callerContext.getContext()); if (callerContext.getSignature() != null) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 30cdfeeecce..8e6634a2eae 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -2201,6 +2201,9 @@ Release 2.8.0 - UNRELEASED HDFS-9332. Fix Precondition failures from NameNodeEditLogRoller while saving namespace. (wang) + HDFS-9343. Empty caller context considered invalid. (Mingliang Liu via + Arpit Agarwal) + Release 2.7.2 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 8d70fbc6110..65b40c87ff9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -7498,9 +7498,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, sb.append(NamenodeWebHdfsMethods.isWebHdfsInvocation() ? "webhdfs" : "rpc"); if (isCallerContextEnabled && callerContext != null && - callerContext.isValid() && - (callerContext.getSignature() == null || - callerContext.getSignature().length <= callerSignatureMaxLen)) { + callerContext.isContextValid()) { sb.append("\t").append("callerContext="); if (callerContext.getContext().length() > callerContextMaxLen) { sb.append(callerContext.getContext().substring(0, @@ -7508,7 +7506,9 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, } else { sb.append(callerContext.getContext()); } - if (callerContext.getSignature() != null) { + if (callerContext.getSignature() != null && + callerContext.getSignature().length > 0 && + callerContext.getSignature().length <= callerSignatureMaxLen) { sb.append(":"); sb.append(new String(callerContext.getSignature(), CallerContext.SIGNATURE_ENCODING)); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java index 168b9d62d05..252f7afe0ed 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java @@ -243,7 +243,6 @@ public class TestAuditLogger { CallerContext.setCurrent(context); LOG.info("Set current caller context as {}", CallerContext.getCurrent()); fs.setTimes(p, time, time); - System.out.println("LLLLLL" + auditlog.getOutput()); assertTrue(auditlog.getOutput().endsWith("callerContext=setTimes\n")); auditlog.clearOutput(); @@ -270,6 +269,16 @@ public class TestAuditLogger { "callerContext=" + longContext.substring(0, 128) + ":L\n")); auditlog.clearOutput(); + // empty context is ignored + context = new CallerContext.Builder("") + .setSignature("L".getBytes(CallerContext.SIGNATURE_ENCODING)) + .build(); + CallerContext.setCurrent(context); + LOG.info("Set empty caller context"); + fs.setTimes(p, time, time); + assertFalse(auditlog.getOutput().contains("callerContext=")); + auditlog.clearOutput(); + // caller context is inherited in child thread context = new CallerContext.Builder("setTimes") .setSignature("L".getBytes(CallerContext.SIGNATURE_ENCODING)) @@ -333,14 +342,14 @@ public class TestAuditLogger { assertTrue(auditlog.getOutput().endsWith("callerContext=mkdirs:L\n")); auditlog.clearOutput(); - // caller context with too long signature is abandoned + // too long signature is ignored context = new CallerContext.Builder("setTimes") .setSignature(new byte[41]) .build(); CallerContext.setCurrent(context); LOG.info("Set current caller context as {}", CallerContext.getCurrent()); fs.setTimes(p, time, time); - assertFalse(auditlog.getOutput().contains("callerContext=")); + assertTrue(auditlog.getOutput().endsWith("callerContext=setTimes\n")); auditlog.clearOutput(); // null signature is ignored