This is an automated email from the ASF dual-hosted git repository.
steveloughran pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/trunk by this push:
new 5f5229f0d5b HADOOP-19936. TFile parsing is too brittle (#8594)
5f5229f0d5b is described below
commit 5f5229f0d5be4639172b9b12b18629078b3e54f0
Author: Steve Loughran <[email protected]>
AuthorDate: Thu Jul 23 12:46:55 2026 +0100
HADOOP-19936. TFile parsing is too brittle (#8594)
Contains content generated by Claude.ai
Contributed by Steve Loughran
---
.../org/apache/hadoop/io/file/tfile/BCFile.java | 30 +++-
.../org/apache/hadoop/io/file/tfile/TFile.java | 88 +++++++---
.../org/apache/hadoop/io/file/tfile/Utils.java | 31 +++-
.../hadoop/io/file/tfile/TestTFileByteArrays.java | 3 +
.../hadoop/io/file/tfile/TestTFileComparator2.java | 81 ++++-----
.../hadoop/io/file/tfile/TestTFileComparators.java | 73 +++-----
.../hadoop/io/file/tfile/TestTFileLoading.java | 189 +++++++++++++++++++++
7 files changed, 370 insertions(+), 125 deletions(-)
diff --git
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/BCFile.java
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/BCFile.java
index 43d829937e7..f43226b2c55 100644
---
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/BCFile.java
+++
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/BCFile.java
@@ -56,6 +56,9 @@ final class BCFile {
static final Version API_VERSION = new Version((short) 1, (short) 0);
static final Logger LOG = LoggerFactory.getLogger(BCFile.class);
+ // Largest encoded length accepted for the short metadata strings: {@value}.
+ private static final int MAX_META_STRING_LENGTH = 64 * 1024;
+
/**
* Prevent the instantiation of BCFile objects.
*/
@@ -636,11 +639,8 @@ public Reader(FSDataInputStream fin, long fileLength,
Configuration conf)
metaIndex = new MetaIndex(fin);
// read data:BCFile.index, the data block index
- BlockReader blockR = getMetaBlock(DataIndex.BLOCK_NAME);
- try {
+ try (BlockReader blockR = getMetaBlock(DataIndex.BLOCK_NAME)) {
dataIndex = new DataIndex(blockR);
- } finally {
- blockR.close();
}
}
@@ -771,6 +771,10 @@ public MetaIndex() {
// for read, construct the map from the file
public MetaIndex(DataInput in) throws IOException {
int count = Utils.readVInt(in);
+ if (count < 0) {
+ throw new IOException("Corrupted meta index: negative entry count "
+ + count);
+ }
index = new TreeMap<String, MetaIndexEntry>();
for (int nx = 0; nx < count; nx++) {
@@ -807,7 +811,7 @@ static final class MetaIndexEntry {
private final BlockRegion region;
public MetaIndexEntry(DataInput in) throws IOException {
- String fullMetaName = Utils.readString(in);
+ String fullMetaName = Utils.readString(in, MAX_META_STRING_LENGTH);
if (fullMetaName.startsWith(defaultPrefix)) {
metaName =
fullMetaName.substring(defaultPrefix.length(), fullMetaName
@@ -817,7 +821,8 @@ public MetaIndexEntry(DataInput in) throws IOException {
}
compressionAlgorithm =
- Compression.getCompressionAlgorithmByName(Utils.readString(in));
+ Compression.getCompressionAlgorithmByName(
+ Utils.readString(in, MAX_META_STRING_LENGTH));
region = new BlockRegion(in);
}
@@ -863,10 +868,17 @@ static class DataIndex {
// for read, deserialized from a file
public DataIndex(DataInput in) throws IOException {
defaultCompressionAlgorithm =
- Compression.getCompressionAlgorithmByName(Utils.readString(in));
+ Compression.getCompressionAlgorithmByName(
+ Utils.readString(in, MAX_META_STRING_LENGTH));
int n = Utils.readVInt(in);
- listRegions = new ArrayList<BlockRegion>(n);
+ if (n < 0) {
+ throw new IOException("Corrupted data index: negative block count "
+ + n);
+ }
+ // Only a capacity hint: bound it so a corrupt count cannot force a huge
+ // pre-allocation. The loop is limited by the bytes actually available.
+ listRegions = new ArrayList<>(Math.min(n, 1024));
for (int i = 0; i < n; i++) {
BlockRegion region = new BlockRegion(in);
@@ -879,7 +891,7 @@ public DataIndex(String defaultCompressionAlgorithmName) {
this.defaultCompressionAlgorithm =
Compression
.getCompressionAlgorithmByName(defaultCompressionAlgorithmName);
- listRegions = new ArrayList<BlockRegion>();
+ listRegions = new ArrayList<>();
}
public Algorithm getDefaultCompressionAlgorithm() {
diff --git
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/TFile.java
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/TFile.java
index aeacc16a78f..a686b5c0532 100644
---
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/TFile.java
+++
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/TFile.java
@@ -166,6 +166,21 @@ static int getFSOutputBufferSize(Configuration conf) {
/** comparator prefix: java class */
public static final String COMPARATOR_JCLASS = "jclass:";
+ /**
+ * {@value}.
+ */
+ public static final String TFILE_COMPARATOR_JCLASS_ENABLED =
+ "tfile.comparator.jclass.enabled";
+ public static final boolean TFILE_COMPARATOR_JCLASS_ENABLED_DEFAULT = false;
+
+ // Largest encoded length accepted for the comparator string on read.
+ private static final int MAX_COMPARATOR_LENGTH = 64 * 1024;
+ // Upper bound on the serialized size of one index entry: a key no larger
+ // than MAX_KEY_SIZE plus its vint/vlong length prefixes.
+ private static final int MAX_INDEX_ENTRY_SIZE = MAX_KEY_SIZE + 16;
+ // Cap on the pre-allocated capacity of index lists sized from file data.
+ private static final int INDEX_CAPACITY_HINT_CAP = 1024;
+
/**
* Make a raw comparator from a string name.
*
@@ -282,7 +297,7 @@ public Writer(FSDataOutputStream fsdos, int minBlockSize,
String compressName, String comparator, Configuration conf)
throws IOException {
sizeMinBlock = minBlockSize;
- tfileMeta = new TFileMeta(comparator);
+ tfileMeta = new TFileMeta(comparator, conf);
tfileIndex = new TFileIndex(tfileMeta.getComparator());
writerBCF = new BCFile.Writer(fsdos, compressName, conf);
@@ -804,11 +819,8 @@ public Reader(FSDataInputStream fsdis, long fileLength,
Configuration conf)
readerBCF = new BCFile.Reader(fsdis, fileLength, conf);
// first, read TFile meta
- BlockReader brMeta = readerBCF.getMetaBlock(TFileMeta.BLOCK_NAME);
- try {
- tfileMeta = new TFileMeta(brMeta);
- } finally {
- brMeta.close();
+ try (BlockReader brMeta = readerBCF.getMetaBlock(TFileMeta.BLOCK_NAME)) {
+ tfileMeta = new TFileMeta(brMeta, conf);
}
comparator = tfileMeta.getComparator();
@@ -1603,6 +1615,9 @@ void checkKey() throws IOException {
valueChecked = false;
klen = Utils.readVInt(blkReader);
+ if (klen < 0 || klen > MAX_KEY_SIZE) {
+ throw new IOException("Key length out of range: " + klen);
+ }
blkReader.readFully(keyBuffer, 0, klen);
valueBufferInputStream.reset(blkReader);
if (valueBufferInputStream.isLastChunk()) {
@@ -2048,27 +2063,32 @@ static final class TFileMeta {
private final BytesComparator comparator;
// ctor for writes
- public TFileMeta(String comparator) {
+ TFileMeta(String comparator, Configuration conf) {
// set fileVersion to API version when we create it.
version = TFile.API_VERSION;
recordCount = 0;
strComparator = (comparator == null) ? "" : comparator;
- this.comparator = makeComparator(strComparator);
+ this.comparator = makeComparator(strComparator, conf);
}
// ctor for reads
- public TFileMeta(DataInput in) throws IOException {
+ TFileMeta(DataInput in, Configuration conf) throws IOException {
version = new Version(in);
if (!version.compatibleWith(TFile.API_VERSION)) {
throw new RuntimeException("Incompatible TFile fileVersion.");
}
recordCount = Utils.readVLong(in);
- strComparator = Utils.readString(in);
- comparator = makeComparator(strComparator);
+ strComparator = Utils.readString(in, MAX_COMPARATOR_LENGTH);
+ comparator = makeComparator(strComparator, conf);
}
- @SuppressWarnings("unchecked")
static BytesComparator makeComparator(String comparator) {
+ return makeComparator(comparator, new Configuration());
+ }
+
+ @SuppressWarnings("unchecked")
+ static BytesComparator makeComparator(String comparator,
+ Configuration conf) {
if (comparator.length() == 0) {
// unsorted keys
return null;
@@ -2077,13 +2097,24 @@ static BytesComparator makeComparator(String
comparator) {
// default comparator
return new BytesComparator(new MemcmpRawComparator());
} else if (comparator.startsWith(COMPARATOR_JCLASS)) {
+ if (!conf.getBoolean(TFILE_COMPARATOR_JCLASS_ENABLED,
+ TFILE_COMPARATOR_JCLASS_ENABLED_DEFAULT)) {
+ throw new IllegalArgumentException(
+ "Class-name comparators are not enabled (set "
+ + TFILE_COMPARATOR_JCLASS_ENABLED + "=true to allow): "
+ + comparator);
+ }
String compClassName =
comparator.substring(COMPARATOR_JCLASS.length()).trim();
try {
- Class compClass = Class.forName(compClassName);
- // use its default ctor to create an instance
- return new BytesComparator((RawComparator<Object>) compClass
- .newInstance());
+ // Resolve without running the class initializer, confirm it really
+ // is a RawComparator, and only then load and construct it.
+ Class<?> compClass =
+ Class.forName(compClassName, false, conf.getClassLoader());
+ RawComparator<Object> rawComparator =
+ (RawComparator<Object>) compClass.asSubclass(RawComparator.class)
+ .getDeclaredConstructor().newInstance();
+ return new BytesComparator(rawComparator);
} catch (Exception e) {
throw new IllegalArgumentException(
"Failed to instantiate comparator: " + comparator + "("
@@ -2144,21 +2175,35 @@ static class TFileIndex {
*/
public TFileIndex(int entryCount, DataInput in, BytesComparator comparator)
throws IOException {
- index = new ArrayList<TFileIndexEntry>(entryCount);
- recordNumIndex = new ArrayList<Long>(entryCount);
+ // entryCount is derived from the file; only use it as a capacity hint,
+ // bounded, so a corrupt value cannot force a huge pre-allocation. The
+ // loops below are limited by the actual bytes available in the stream.
+ int capacityHint = Math.max(0, Math.min(entryCount,
INDEX_CAPACITY_HINT_CAP));
+ index = new ArrayList<>(capacityHint);
+ recordNumIndex = new ArrayList<>(capacityHint);
int size = Utils.readVInt(in); // size for the first key entry.
if (size > 0) {
+ if (size > MAX_INDEX_ENTRY_SIZE) {
+ throw new IOException("First key entry size out of range: " + size);
+ }
byte[] buffer = new byte[size];
in.readFully(buffer);
DataInputStream firstKeyInputStream =
new DataInputStream(new ByteArrayInputStream(buffer, 0, size));
int firstKeyLength = Utils.readVInt(firstKeyInputStream);
+ if (firstKeyLength < 0 || firstKeyLength > MAX_KEY_SIZE) {
+ throw new IOException("First key length out of range: "
+ + firstKeyLength);
+ }
firstKey = new ByteArray(new byte[firstKeyLength]);
firstKeyInputStream.readFully(firstKey.buffer());
for (int i = 0; i < entryCount; i++) {
size = Utils.readVInt(in);
+ if (size < 0 || size > MAX_INDEX_ENTRY_SIZE) {
+ throw new IOException("Index entry size out of range: " + size);
+ }
if (buffer.length < size) {
buffer = new byte[size];
}
@@ -2226,8 +2271,8 @@ public int upperBound(RawComparable key) {
* For writing to file.
*/
public TFileIndex(BytesComparator comparator) {
- index = new ArrayList<TFileIndexEntry>();
- recordNumIndex = new ArrayList<Long>();
+ index = new ArrayList<>();
+ recordNumIndex = new ArrayList<>();
this.comparator = comparator;
}
@@ -2301,6 +2346,9 @@ static final class TFileIndexEntry implements
RawComparable {
public TFileIndexEntry(DataInput in) throws IOException {
int len = Utils.readVInt(in);
+ if (len < 0 || len > MAX_KEY_SIZE) {
+ throw new IOException("Index entry key length out of range: " + len);
+ }
key = new byte[len];
in.readFully(key, 0, len);
kvEntries = Utils.readVLong(in);
diff --git
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/Utils.java
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/Utils.java
index 714dc5a12ac..10d0a548a53 100644
---
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/Utils.java
+++
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/file/tfile/Utils.java
@@ -19,6 +19,7 @@
import java.io.DataInput;
import java.io.DataOutput;
+import java.io.EOFException;
import java.io.IOException;
import java.util.Comparator;
import java.util.List;
@@ -274,8 +275,36 @@ public static void writeString(DataOutput out, String s)
throws IOException {
* @throws IOException raised on errors performing I/O.
*/
public static String readString(DataInput in) throws IOException {
+ return readString(in, -1);
+ }
+
+ /**
+ * Read a String as a VInt n, followed by n Bytes in Text format, rejecting
+ * lengths that are -2 or less. or larger than the supplied bound before any
+ * buffer is allocated.
+ * A length of -1 means "no data" and mapped to a null string.
+ *
+ * @param in The input stream.
+ * @param maxLength The largest permitted encoded length in bytes, negative
for no limit.
+ * @return The string or null.
+ * @throws EOFException input data length exceeds {@code maxLength}.
+ * @throws IOException IO failure.
+ * @throws NegativeArraySizeException string length was minus two or less.
+ */
+ public static String readString(DataInput in, int maxLength)
+ throws IOException {
int length = readVInt(in);
- if (length == -1) return null;
+ if (length == -1) {
+ return null;
+ }
+ if (length < 0) {
+ throw new NegativeArraySizeException("Corrupted data: negative string
length "
+ + length);
+ }
+ if (maxLength >= 0 && length > maxLength) {
+ throw new EOFException("String length " + length
+ + " exceeds the limit of " + maxLength);
+ }
byte[] buffer = new byte[length];
in.readFully(buffer);
return Text.decode(buffer);
diff --git
a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileByteArrays.java
b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileByteArrays.java
index 2661ce10186..3134e3a5e74 100644
---
a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileByteArrays.java
+++
b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileByteArrays.java
@@ -89,6 +89,9 @@ public void init(String compression, String comparator,
public void init(String compression, String comparator) {
this.compression = compression;
this.comparator = comparator;
+ if (comparator.startsWith(TFile.COMPARATOR_JCLASS)) {
+ conf.setBoolean(TFile.TFILE_COMPARATOR_JCLASS_ENABLED, true);
+ }
}
@BeforeEach
diff --git
a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileComparator2.java
b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileComparator2.java
index 4177d7362a1..212d240cd12 100644
---
a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileComparator2.java
+++
b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileComparator2.java
@@ -33,8 +33,8 @@
import static org.junit.jupiter.api.Assertions.*;
public class TestTFileComparator2 {
- private static String ROOT = GenericTestUtils.getTestDir().getAbsolutePath();
- private static final String name = "test-tfile-comparator2";
+ private static final String ROOT =
GenericTestUtils.getTestDir().getAbsolutePath();
+ private static final String NAME = "test-tfile-comparator2";
private final static int BLOCK_SIZE = 512;
private static final String VALUE = "value";
private static final String jClassLongWritableComparator = "jclass:"
@@ -52,55 +52,42 @@ private static String buildValue(long i) {
@Test
public void testSortedLongWritable() throws IOException {
Configuration conf = new Configuration();
- Path path = new Path(ROOT, name);
+ conf.setBoolean(TFile.TFILE_COMPARATOR_JCLASS_ENABLED, true);
+ Path path = new Path(ROOT, NAME);
FileSystem fs = path.getFileSystem(conf);
- FSDataOutputStream out = fs.create(path);
- try {
- TFile.Writer writer = new Writer(out, BLOCK_SIZE, "gz",
- jClassLongWritableComparator, conf);
- try {
- LongWritable key = new LongWritable(0);
- for (long i=0; i<NENTRY; ++i) {
- key.set(cube(i-NENTRY/2));
- DataOutputStream dos = writer.prepareAppendKey(-1);
- try {
- key.write(dos);
- } finally {
- dos.close();
- }
- dos = writer.prepareAppendValue(-1);
- try {
- dos.write(buildValue(i).getBytes());
- } finally {
- dos.close();
- }
+ try (FSDataOutputStream out = fs.create(path);
+ Writer writer = new Writer(out, BLOCK_SIZE, "gz",
+ jClassLongWritableComparator, conf)) {
+ LongWritable key = new LongWritable(0);
+ for (long i = 0; i < NENTRY; ++i) {
+ key.set(cube(i - NENTRY / 2));
+ DataOutputStream dos = writer.prepareAppendKey(-1);
+ try {
+ key.write(dos);
+ } finally {
+ dos.close();
}
- } finally {
- writer.close();
- }
- } finally {
- out.close();
- }
-
- FSDataInputStream in = fs.open(path);
- try {
- TFile.Reader reader = new TFile.Reader(in, fs.getFileStatus(path)
- .getLen(), conf);
- try {
- TFile.Reader.Scanner scanner = reader.createScanner();
- long i=0;
- BytesWritable value = new BytesWritable();
- for (; !scanner.atEnd(); scanner.advance()) {
- scanner.entry().getValue(value);
- assertEquals(buildValue(i), new String(value.getBytes(), 0, value
- .getLength()));
- ++i;
+ dos = writer.prepareAppendValue(-1);
+ try {
+ dos.write(buildValue(i).getBytes());
+ } finally {
+ dos.close();
}
- } finally {
- reader.close();
}
- } finally {
- in.close();
+ }
+ final long len = fs.getFileStatus(path).getLen();
+
+ try (FSDataInputStream in = fs.open(path);
+ TFile.Reader reader = new TFile.Reader(in, len, conf)) {
+ TFile.Reader.Scanner scanner = reader.createScanner();
+ long i = 0;
+ BytesWritable value = new BytesWritable();
+ for (; !scanner.atEnd(); scanner.advance()) {
+ scanner.entry().getValue(value);
+ assertEquals(buildValue(i), new String(value.getBytes(), 0, value
+ .getLength()));
+ ++i;
+ }
}
}
}
diff --git
a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileComparators.java
b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileComparators.java
index f349b581461..54f36866e66 100644
---
a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileComparators.java
+++
b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileComparators.java
@@ -17,8 +17,6 @@
package org.apache.hadoop.io.file.tfile;
-import static org.junit.jupiter.api.Assertions.fail;
-
import java.io.IOException;
import org.junit.jupiter.api.AfterEach;
@@ -32,6 +30,8 @@
import org.apache.hadoop.io.file.tfile.TFile.Writer;
import org.apache.hadoop.test.GenericTestUtils;
+import static org.apache.hadoop.test.LambdaTestUtils.intercept;
+
/**
*
* Byte arrays test case class using GZ compression codec, base class of none
@@ -39,79 +39,56 @@
*
*/
public class TestTFileComparators {
- private static String ROOT = GenericTestUtils.getTestDir().getAbsolutePath();
- private final static int BLOCK_SIZE = 512;
+ private static final String ROOT =
GenericTestUtils.getTestDir().getAbsolutePath();
+ private static final int BLOCK_SIZE = 512;
private FileSystem fs;
private Configuration conf;
private Path path;
private FSDataOutputStream out;
private Writer writer;
- private String compression = Compression.Algorithm.GZ.getName();
- private String outputFile = "TFileTestComparators";
- /*
- * pre-sampled numbers of records in one block, based on the given the
- * generated key and value strings
- */
- // private int records1stBlock = 4314;
- // private int records2ndBlock = 4108;
- private int records1stBlock = 4480;
- private int records2ndBlock = 4263;
+ private static final String COMPRESSION = Compression.Algorithm.GZ.getName();
+ private static final String OUTPUT_FILE = "TFileTestComparators";
+
@BeforeEach
public void setUp() throws IOException {
conf = new Configuration();
- path = new Path(ROOT, outputFile);
+ path = new Path(ROOT, OUTPUT_FILE);
fs = path.getFileSystem(conf);
out = fs.create(path);
}
@AfterEach
public void tearDown() throws IOException {
+ closeOutput();
fs.delete(path, true);
}
// bad comparator format
@Test
- public void testFailureBadComparatorNames() throws IOException {
- try {
- writer = new Writer(out, BLOCK_SIZE, compression, "badcmp", conf);
- fail("Failed to catch unsupported comparator names");
- }
- catch (Exception e) {
- // noop, expecting exceptions
- e.printStackTrace();
- }
+ public void testFailureBadComparatorNames() throws Exception {
+ intercept(IllegalArgumentException.class, "Unsupported comparator", () ->
+ new Writer(out, BLOCK_SIZE, COMPRESSION, "badcmp", conf));
}
- // jclass that doesn't exist
+ // jclass that doesn't exist: fails to instantiate, not because the feature
+ // is disabled.
@Test
- public void testFailureBadJClassNames() throws IOException {
- try {
- writer =
- new Writer(out, BLOCK_SIZE, compression,
- "jclass: some.non.existence.clazz", conf);
- fail("Failed to catch unsupported comparator names");
- }
- catch (Exception e) {
- // noop, expecting exceptions
- e.printStackTrace();
- }
+ public void testFailureBadJClassNames() throws Exception {
+ conf.setBoolean(TFile.TFILE_COMPARATOR_JCLASS_ENABLED, true);
+ intercept(IllegalArgumentException.class, "Failed to instantiate
comparator",
+ () -> new Writer(out, BLOCK_SIZE, COMPRESSION,
+ "jclass: some.non.existence.clazz", conf));
}
- // class exists but not a RawComparator
+ // class exists but is not a RawComparator
@Test
- public void testFailureBadJClasses() throws IOException {
- try {
- writer =
- new Writer(out, BLOCK_SIZE, compression,
- "jclass:org.apache.hadoop.io.file.tfile.Chunk", conf);
- fail("Failed to catch unsupported comparator names");
- }
- catch (Exception e) {
- // noop, expecting exceptions
- e.printStackTrace();
- }
+ public void testFailureBadJClasses() throws Exception {
+ conf.setBoolean(TFile.TFILE_COMPARATOR_JCLASS_ENABLED, true);
+ intercept(IllegalArgumentException.class, "Failed to instantiate
comparator",
+ () -> new Writer(out, BLOCK_SIZE, COMPRESSION,
+ "jclass:org.apache.hadoop.io.file.tfile.Chunk", conf));
}
private void closeOutput() throws IOException {
diff --git
a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileLoading.java
b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileLoading.java
new file mode 100644
index 00000000000..a3e03e971d7
--- /dev/null
+++
b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/file/tfile/TestTFileLoading.java
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+
+package org.apache.hadoop.io.file.tfile;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.EOFException;
+import java.io.IOException;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.io.LongWritable;
+import org.apache.hadoop.io.file.tfile.TFile.Reader;
+import org.apache.hadoop.io.file.tfile.TFile.Writer;
+import org.apache.hadoop.test.GenericTestUtils;
+
+import static org.apache.hadoop.test.LambdaTestUtils.intercept;
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Test TFile loading resilience.
+ */
+public class TestTFileLoading {
+
+ private static final String ROOT =
+ GenericTestUtils.getTestDir().getAbsolutePath();
+
+ private static final int BLOCK_SIZE = 512;
+
+ private static final String COMPRESSION =
Compression.Algorithm.NONE.getName();
+
+ private static final String JCLASS_COMPARATOR =
+ TFile.COMPARATOR_JCLASS + LongWritable.Comparator.class.getName();
+
+ private FileSystem fs;
+
+ private Path path;
+
+ @BeforeEach
+ public void setUp() throws IOException {
+ Configuration conf = new Configuration();
+ path = new Path(ROOT, "TestTFileLoading");
+ fs = FileSystem.getLocal(conf);
+ }
+
+ @AfterEach
+ public void tearDown() throws IOException {
+ fs.delete(path, true);
+ }
+
+ private Configuration enableJclass() {
+ Configuration conf = new Configuration();
+ conf.setBoolean(TFile.TFILE_COMPARATOR_JCLASS_ENABLED, true);
+ return conf;
+ }
+
+ @Test
+ public void testJClassComparatorRejectedByDefault() throws Exception {
+ try (FSDataOutputStream out = fs.create(path)) {
+ intercept(IllegalArgumentException.class, () ->
+ new Writer(out, BLOCK_SIZE, COMPRESSION, JCLASS_COMPARATOR, new
Configuration()));
+ }
+ }
+
+ @Test
+ public void testJClassComparatorMustBeRawComparator() throws Exception {
+ Configuration conf = enableJclass();
+ String notAComparator =
+ TFile.COMPARATOR_JCLASS + Chunk.class.getName();
+ try (FSDataOutputStream out = fs.create(path)) {
+ intercept(IllegalArgumentException.class, () ->
+ new Writer(out, BLOCK_SIZE, COMPRESSION, notAComparator, conf));
+ }
+ }
+
+ /**
+ * When jclass support is disabled, tfile readers fail to load files
+ * containing them.
+ */
+ @Test
+ public void testReaderRejectsJClassWhenDisabled() throws Exception {
+ // write a valid, sorted file naming a jclass comparator.
+ try (FSDataOutputStream out = fs.create(path);
+ Writer writer = new Writer(out, BLOCK_SIZE, COMPRESSION,
JCLASS_COMPARATOR,
+ enableJclass())) {
+ LongWritable key = new LongWritable(0);
+ for (long i = 0; i < 4; ++i) {
+ key.set(i);
+ try (DataOutputStream dos = writer.prepareAppendKey(-1)) {
+ key.write(dos);
+ }
+ try (DataOutputStream dos = writer.prepareAppendValue(-1)) {
+ dos.write(("value-" + i).getBytes());
+ }
+ }
+ }
+
+ long len = fs.getFileStatus(path).getLen();
+ // a reader that has not enabled the feature refuses the file.
+ Configuration disabled = new Configuration();
+ try (FSDataInputStream in = fs.open(path)) {
+ intercept(IllegalArgumentException.class, () ->
+ new Reader(in, len, disabled));
+ }
+ // with the feature enabled the same file opens.
+ try (FSDataInputStream in = fs.open(path)) {
+ Reader reader = new Reader(in, len, enableJclass());
+ assertThat(reader.getComparator())
+ .describedAs("comparator")
+ .isNotNull();
+ reader.close();
+ }
+ }
+
+ /**
+ * A bounded string read rejects an over-long length before allocating.
+ */
+ @Test
+ public void testReadStringRejectsOversizedLength() throws Exception {
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ // claim a huge length but supply no payload.
+ Utils.writeVInt(new DataOutputStream(bos), Integer.MAX_VALUE);
+ DataInputStream in =
+ new DataInputStream(new ByteArrayInputStream(bos.toByteArray()));
+ intercept(EOFException.class, () -> Utils.readString(in, 1024));
+ }
+
+ /**
+ * A string length less than minus 1 is rejected.
+ */
+ @Test
+ public void testReadStringRejectsNegativeLength() throws Exception {
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ Utils.writeVInt(new DataOutputStream(bos), -2);
+ DataInputStream in =
+ new DataInputStream(new ByteArrayInputStream(bos.toByteArray()));
+ intercept(NegativeArraySizeException.class, () -> Utils.readString(in));
+ }
+
+ /**
+ * A length of -1 is null; this is the classic behavior.
+ */
+ @Test
+ public void testReadStringMinus1MapsToNull() throws Exception {
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ Utils.writeVInt(new DataOutputStream(bos), -1);
+ DataInputStream in =
+ new DataInputStream(new ByteArrayInputStream(bos.toByteArray()));
+ assertThat(Utils.readString(in)).isNull();
+ }
+
+ /**
+ * Reject index entry records out of range.
+ */
+ @Test
+ public void testIndexEntryKeyLengthBounded() throws Exception {
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ // a key length well beyond the 64KB maximum.
+ Utils.writeVInt(new DataOutputStream(bos), 1 << 20);
+ DataInputStream in =
+ new DataInputStream(new ByteArrayInputStream(bos.toByteArray()));
+ intercept(IOException.class, () ->
+ new TFile.TFileIndexEntry(in));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
