From 3176ab028b2f8f22d968bb4c225d2fd9e3ef4936 Mon Sep 17 00:00:00 2001
From: Viraj Jasani <vjasani@apache.org>
Date: Tue, 11 May 2021 09:08:15 +0530
Subject: [PATCH] HDFS-16007. Deserialization of ReplicaState should avoid
 throwing ArrayIndexOutOfBoundsException (#2982)

Signed-off-by: Akira Ajisaka <aajisaka@apache.org>
(cherry picked from commit b944084b32268d4c259bde894a80207010b5c103)
---
 .../server/common/HdfsServerConstants.java    | 28 +++++++++++++++++--
 .../hdfs/server/common/TestJspHelper.java     | 19 +++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java
index b6b4463df1..03b180121f 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/HdfsServerConstants.java
@@ -23,6 +23,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.apache.commons.lang3.Validate;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hdfs.DFSUtil;
@@ -294,6 +295,10 @@ enum ReplicaState {
     /** Temporary replica: created for replication and relocation only. */
     TEMPORARY(4);
 
+    // Since ReplicaState (de)serialization depends on ordinal, either adding
+    // new value should be avoided to this enum or newly appended value should
+    // be handled by NameNodeLayoutVersion#Feature.
+
     private static final ReplicaState[] cachedValues = ReplicaState.values();
 
     private final int value;
@@ -306,13 +311,32 @@ public int getValue() {
       return value;
     }
 
+    /**
+     * Retrieve ReplicaState corresponding to given index.
+     *
+     * @param v Index to retrieve {@link ReplicaState}.
+     * @return {@link ReplicaState} object.
+     * @throws IndexOutOfBoundsException if the index is invalid.
+     */
     public static ReplicaState getState(int v) {
+      Validate.validIndex(cachedValues, v, "Index Expected range: [0, "
+          + (cachedValues.length - 1) + "]. Actual value: " + v);
       return cachedValues[v];
     }
 
-    /** Read from in */
+    /**
+     * Retrieve ReplicaState corresponding to index provided in binary stream.
+     *
+     * @param in Index value provided as bytes in given binary stream.
+     * @return {@link ReplicaState} object.
+     * @throws IOException if an I/O error occurs while reading bytes.
+     * @throws IndexOutOfBoundsException if the index is invalid.
+     */
     public static ReplicaState read(DataInput in) throws IOException {
-      return cachedValues[in.readByte()];
+      byte idx = in.readByte();
+      Validate.validIndex(cachedValues, idx, "Index Expected range: [0, "
+          + (cachedValues.length - 1) + "]. Actual value: " + idx);
+      return cachedValues[idx];
     }
 
     /** Write to out */
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/common/TestJspHelper.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/common/TestJspHelper.java
index 1aff7669e9..e5746a0909 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/common/TestJspHelper.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/common/TestJspHelper.java
@@ -457,6 +457,25 @@ public void testReadWriteReplicaState() {
         out.reset();
         in.reset();
       }
+      out = new DataOutputBuffer();
+      out.writeByte(100);
+      in.reset(out.getData(), out.getLength());
+      try {
+        HdfsServerConstants.ReplicaState.read(in);
+        fail("Should not have reached here");
+      } catch (IndexOutOfBoundsException e) {
+        assertEquals(e.getMessage(),
+            "Index Expected range: [0, 4]. Actual value: 100");
+      }
+      out.reset();
+      in.reset();
+      try {
+        HdfsServerConstants.ReplicaState.getState(200);
+        fail("Should not have reached here");
+      } catch (IndexOutOfBoundsException e) {
+        assertEquals(e.getMessage(),
+            "Index Expected range: [0, 4]. Actual value: 200");
+      }
     } catch (Exception ex) {
       fail("testReadWrite ex error ReplicaState");
     }