From b944084b32268d4c259bde894a80207010b5c103 Mon Sep 17 00:00:00 2001 From: Viraj Jasani 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 --- .../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 254e9790da0..58e9d5fdd95 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; @@ -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 */ 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 1aff7669e98..e5746a09095 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"); }