HDFS-9343. Empty caller context considered invalid. (Contributed by Mingliang Liu)

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;

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) {

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

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));

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