Browse Source

HADOOP-18014. CallerContext should not include some characters. (#3698)

Reviewed-by: Viraj Jasani <vjasani@apache.org>
Reviewed-by: Mingliang Liu <liuml07@apache.org>
Reviewed-by: Hui Fei <ferhui@apache.org>
Takanobu Asanuma 3 years ago
parent
commit
9c887e5b82

+ 0 - 2
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java

@@ -164,8 +164,6 @@ public final class CallerContext {
 
     /**
      * Whether the field is valid.
-     * The field should not contain '\t', '\n', '='.
-     * Because the context could be written to audit log.
      * @param field one of the fields in context.
      * @return true if the field is not null or empty.
      */

+ 6 - 6
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java

@@ -8802,18 +8802,18 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
             callerContext != null &&
             callerContext.isContextValid()) {
           sb.append("\t").append("callerContext=");
-          if (callerContext.getContext().length() > callerContextMaxLen) {
-            sb.append(callerContext.getContext().substring(0,
-                callerContextMaxLen));
+          String context = escapeJava(callerContext.getContext());
+          if (context.length() > callerContextMaxLen) {
+            sb.append(context, 0, callerContextMaxLen);
           } else {
-            sb.append(callerContext.getContext());
+            sb.append(context);
           }
           if (callerContext.getSignature() != null &&
               callerContext.getSignature().length > 0 &&
               callerContext.getSignature().length <= callerSignatureMaxLen) {
             sb.append(":")
-                .append(new String(callerContext.getSignature(),
-                CallerContext.SIGNATURE_ENCODING));
+                .append(escapeJava(new String(callerContext.getSignature(),
+                CallerContext.SIGNATURE_ENCODING)));
           }
         }
         logAuditMessage(sb.toString());

+ 34 - 5
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAuditLogger.java

@@ -256,10 +256,9 @@ public class TestAuditLogger {
     conf.setBoolean(HADOOP_CALLER_CONTEXT_ENABLED_KEY, true);
     conf.setInt(HADOOP_CALLER_CONTEXT_MAX_SIZE_KEY, 128);
     conf.setInt(HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_KEY, 40);
-    MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build();
-    LogCapturer auditlog = LogCapturer.captureLogs(FSNamesystem.auditLog);
 
-    try {
+    try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build()) {
+      LogCapturer auditlog = LogCapturer.captureLogs(FSNamesystem.auditLog);
       cluster.waitClusterUp();
       final FileSystem fs = cluster.getFileSystem();
       final long time = System.currentTimeMillis();
@@ -414,8 +413,8 @@ public class TestAuditLogger {
       assertFalse(auditlog.getOutput().contains("callerContext="));
       auditlog.clearOutput();
 
-    } finally {
-      cluster.shutdown();
+      // clear client context
+      CallerContext.setCurrent(null);
     }
   }
 
@@ -599,6 +598,36 @@ public class TestAuditLogger {
     }
   }
 
+  @Test
+  public void testCallerContextCharacterEscape() throws IOException {
+    Configuration conf = new HdfsConfiguration();
+    conf.setBoolean(HADOOP_CALLER_CONTEXT_ENABLED_KEY, true);
+    conf.setInt(HADOOP_CALLER_CONTEXT_MAX_SIZE_KEY, 128);
+    conf.setInt(HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_KEY, 40);
+
+    try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build()) {
+      LogCapturer auditlog = LogCapturer.captureLogs(FSNamesystem.auditLog);
+      cluster.waitClusterUp();
+      final FileSystem fs = cluster.getFileSystem();
+      final long time = System.currentTimeMillis();
+      final Path p = new Path("/");
+
+      assertNull(CallerContext.getCurrent());
+
+      CallerContext context = new CallerContext.Builder("c1\nc2").append("c3\tc4")
+          .setSignature("s1\ns2".getBytes(CallerContext.SIGNATURE_ENCODING)).build();
+      CallerContext.setCurrent(context);
+      LOG.info("Set current caller context as {}", CallerContext.getCurrent());
+      fs.setTimes(p, time, time);
+      assertTrue(auditlog.getOutput().endsWith(
+          String.format("callerContext=c1\\nc2,c3\\tc4:s1\\ns2%n")));
+      auditlog.clearOutput();
+
+      // clear client context
+      CallerContext.setCurrent(null);
+    }
+  }
+
   public static class DummyAuditLogger implements AuditLogger {
 
     static boolean initialized;