HDFS-16007. Deserialization of ReplicaState should avoid throwing ArrayIndexOutOfBoundsException (#2982)

Signed-off-by: Akira Ajisaka <aajisaka@apache.org>

Author: virajjasani

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;
@@ -287,6 +288,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;
@@ -299,13 +304,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 */

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