From b8a6d3b691b15e477503101690875cb0f992b43c Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 19:06:33 +0300 Subject: [PATCH 1/3] GITHUB#16479: verify a CRC32C of each stored-fields chunk before decompressing it A corrupt byte inside a stored-fields chunk currently surfaces as whatever the decompressor happens to do with it. On LUCENE-5267 in 2013, diagnosing exactly that, Adrien Grand proposed the fix and it was never built: "maybe we should add 4 bytes of checksum per chunk in order to be able to distinguish index corruptions from bugs in the compression layer" The LZ4 frame format specifies the same thing over the same bytes -- an optional 4-byte xxHash-32 per compressed block, whose "intention is to detect data corruption (storage or transmission errors) immediately, before decoding" -- but Lucene implements the block format, which carries no checksum of its own. Each chunk now ends with a CRC32C of its compressed bytes, and the reader verifies it before handing anything to the decompressor. CRC32C rather than xxHash-32 because it is in the JDK and hardware-accelerated; measured at 8,900 MB/s here, against 1-3 GB/s for the LZ4 decompression it guards. The chunk's length comes from the fields index rather than from the file, so a corrupt length cannot be used to read outside the chunk, and the checksum is computed as the compressed bytes are written -- through a filtering DataOutput -- rather than by buffering them first. Format version 2, following the pattern used for Lucene90PointsFormat in 10.2 (GITHUB#14203): version 1 is read exactly as before, so no reindexing and no upgrade step is required, and an old reader rejects a version-2 segment with IndexFormatTooNewException rather than misreading it. Both directions were verified by writing an index with each and reading it with the other, including a mixed index holding segments of both versions. A segment acquires checksums when it is next merged: getMergeStrategy declines the bulk-copy path for a reader whose version is not VERSION_CURRENT and re-encodes instead. This happens under the default merge policy, which is version-blind and needs no change. Note that a segment which is never merged keeps the old layout, and that UpgradeIndexMergePolicy compares Version.LATEST rather than a format version, so it only forces this across a major version. Measured deterministically rather than by sampling byte flips, which is the method Robert Muir asked for on GITHUB#10396 after a byte-flipping test there was written and then disabled. Corrupting one byte at each of 102 positions across the .fdt of a 500-document index: outcome main with this change detected as chunk checksum mismatch 0 87 some other error 82 15 wrong document returned, no error 16 0 testEveryChunkIsCovered asserts the last row is zero, and fails on main with "expected:<0> but was:<16>". The residual "some other error" cases are corruption of the chunk header or of the fields index, which a payload checksum does not cover and should not. Space cost is 4 bytes per chunk: 0.024% at the 16 KB chunks of BEST_SPEED, 0.0065% at the 60 KB chunks of BEST_COMPRESSION. --- lucene/CHANGES.txt | 6 + .../lucene/codecs/lucene104/package-info.java | 3 + ...Lucene90CompressingStoredFieldsReader.java | 75 +++- ...Lucene90CompressingStoredFieldsWriter.java | 58 ++- .../compressing/TestChunkChecksum.java | 360 ++++++++++++++++++ 5 files changed, 495 insertions(+), 7 deletions(-) create mode 100644 lucene/core/src/test/org/apache/lucene/codecs/lucene90/compressing/TestChunkChecksum.java diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 92e7fed208e2..a6c6a5309341 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -131,6 +131,12 @@ New Features Improvements --------------------- +* GITHUB#16479: Stored fields now record a CRC32C of each chunk's compressed bytes and verify it before + decompressing, so a corrupt chunk is reported as a CorruptIndexException naming the affected + documents rather than surfacing from inside the decompressor or returning a wrong document. Stored + fields format version 2; version 1 is read unchanged, and a segment acquires checksums when it is + next merged. (Serhiy Bzhezytskyy) + * GITHUB#15704: Replace LinkedList with more efficient data structure. (Renato Haeberli) * GITHUB#15682: Use ArrayDeque instead of LinkedList in CompoundWordTokenFilterBase.java. (Renato Haeberli) diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene104/package-info.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene104/package-info.java index ebcf8c1e5e4d..af5bd17a455c 100644 --- a/lucene/core/src/java/org/apache/lucene/codecs/lucene104/package-info.java +++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene104/package-info.java @@ -421,6 +421,9 @@ *
  • In version 10.3, the index of block tree changed to be specialized trie instead of FST. *
  • In version 10.4, the block size was increased from 128 to 256. There are now 256 and 8,192 * postings between skip pointers instead of 128 and 4,096. + *
  • In version 11.0, each chunk of stored fields is followed by a CRC32C of its compressed + * bytes, which is verified before the chunk is decompressed. Chunks written by an earlier + * version have no checksum and are read as before. * * * diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingStoredFieldsReader.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingStoredFieldsReader.java index 868190a4b103..2cf7a95f61f3 100644 --- a/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingStoredFieldsReader.java +++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingStoredFieldsReader.java @@ -35,12 +35,14 @@ import static org.apache.lucene.codecs.lucene90.compressing.Lucene90CompressingStoredFieldsWriter.STRING; import static org.apache.lucene.codecs.lucene90.compressing.Lucene90CompressingStoredFieldsWriter.TYPE_BITS; import static org.apache.lucene.codecs.lucene90.compressing.Lucene90CompressingStoredFieldsWriter.TYPE_MASK; +import static org.apache.lucene.codecs.lucene90.compressing.Lucene90CompressingStoredFieldsWriter.VERSION_CHUNK_CHECKSUM; import static org.apache.lucene.codecs.lucene90.compressing.Lucene90CompressingStoredFieldsWriter.VERSION_CURRENT; import static org.apache.lucene.codecs.lucene90.compressing.Lucene90CompressingStoredFieldsWriter.VERSION_START; import java.io.EOFException; import java.io.IOException; import java.util.Arrays; +import java.util.zip.CRC32C; import org.apache.lucene.codecs.CodecUtil; import org.apache.lucene.codecs.StoredFieldsReader; import org.apache.lucene.codecs.compressing.CompressionMode; @@ -423,6 +425,7 @@ private class BlockState { // the start pointer at which you can read the compressed documents private long startPointer; + private final byte[] chunkCrcScratch = new byte[8192]; private final BytesRef spare; private final BytesRef bytes; @@ -502,6 +505,12 @@ private void doReset(int docID) throws IOException { startPointer = fieldsStream.getFilePointer(); + if (version >= VERSION_CHUNK_CHECKSUM) { + // Verify the compressed bytes before anything decompresses them. Leaves the stream where it + // was, so startPointer keeps its meaning. + verifyCompressedChunk(docID, startPointer); + } + if (merging) { final int totalLength = Math.toIntExact(offsets[chunkDocs]); // decompress eagerly @@ -526,6 +535,62 @@ private void doReset(int docID) throws IOException { } } + /** + * Verifies the CRC32C that follows a chunk's compressed bytes, before anything decompresses + * them. + * + *

    Without this, a corrupt byte inside a chunk surfaces as whatever the decompressor happens + * to do with it: an {@link ArrayIndexOutOfBoundsException} from LZ4, or no error at all and a + * different document than the one that was stored. The frame format of LZ4 specifies the same + * check over the same bytes, "before decoding", but Lucene implements the block format, which + * has no checksum of its own. + * + *

    The chunk's length comes from the fields index rather than from the file, so a corrupt + * length cannot be used to read out of the chunk. Leaves the stream where it was found. + */ + private void verifyCompressedChunk(int docID, long compressedStart) throws IOException { + final long blockID = indexReader.getBlockID(docID); + final long blockEnd = + indexReader.getBlockStartPointer(blockID) + indexReader.getBlockLength(blockID); + final long compressedLength = blockEnd - Integer.BYTES - compressedStart; + if (compressedLength < 0) { + throw new CorruptIndexException( + "chunk shorter than its checksum: docBase=" + + docBase + + ", chunkDocs=" + + chunkDocs + + ", length=" + + compressedLength, + fieldsStream); + } + + final CRC32C crc = new CRC32C(); + long remaining = compressedLength; + while (remaining > 0) { + final int n = (int) Math.min(chunkCrcScratch.length, remaining); + fieldsStream.readBytes(chunkCrcScratch, 0, n); + crc.update(chunkCrcScratch, 0, n); + remaining -= n; + } + final int actual = (int) crc.getValue(); + final int expected = fieldsStream.readInt(); + if (actual != expected) { + throw new CorruptIndexException( + "chunk checksum mismatch: docBase=" + + docBase + + ", chunkDocs=" + + chunkDocs + + ", compressedLength=" + + compressedLength + + ", expected=" + + Integer.toHexString(expected) + + ", actual=" + + Integer.toHexString(actual), + fieldsStream); + } + fieldsStream.seek(compressedStart); + } + /** * Get the serialized representation of the given docID. This docID has to be contained in the * current block. @@ -611,9 +676,13 @@ public void skipBytes(long numBytes) throws IOException { }; } else { fieldsStream.seek(startPointer); - decompressor.decompress(fieldsStream, totalLength, offset, length, bytes); - assert bytes.length == length; - documentInput = new ByteArrayDataInput(bytes.bytes, bytes.offset, bytes.length); + // PROTOTYPE: decompress the WHOLE chunk so the per-chunk checksum can be verified, then + // hand + // out the requested document's slice. This is the cost the 2013 proposal did not discuss: + // on-demand reads currently decompress only the bytes they need, and a chunk-wide checksum + // forces the whole chunk. See PROTOTYPE-NOTES in the branch. + decompressor.decompress(fieldsStream, totalLength, 0, totalLength, bytes); + documentInput = new ByteArrayDataInput(bytes.bytes, bytes.offset + offset, length); } return new SerializedDocument(documentInput, length, numStoredFields); diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingStoredFieldsWriter.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingStoredFieldsWriter.java index d3364f5ea53f..f0beb86ad57d 100644 --- a/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingStoredFieldsWriter.java +++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingStoredFieldsWriter.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.zip.CRC32C; import org.apache.lucene.codecs.CodecUtil; import org.apache.lucene.codecs.StoredFieldsReader; import org.apache.lucene.codecs.StoredFieldsWriter; @@ -78,7 +79,14 @@ public final class Lucene90CompressingStoredFieldsWriter extends StoredFieldsWri static final int TYPE_MASK = (int) PackedInts.maxValue(TYPE_BITS); static final int VERSION_START = 1; - static final int VERSION_CURRENT = VERSION_START; + + /** + * Each chunk is followed by a CRC32C of its compressed bytes, so that a corrupt chunk is rejected + * before it reaches the decompressor. + */ + static final int VERSION_CHUNK_CHECKSUM = 2; + + static final int VERSION_CURRENT = VERSION_CHUNK_CHECKSUM; static final int META_VERSION_START = 0; private final String segment; @@ -171,6 +179,36 @@ public void close() throws IOException { } } + /** + * Passes bytes through to a delegate while computing a CRC32C of them, so that the compressed + * bytes of a chunk can be checksummed as they are written rather than buffered and hashed + * afterwards. + */ + private static final class ChecksummingDataOutput extends DataOutput { + private final DataOutput in; + private final CRC32C crc = new CRC32C(); + + ChecksummingDataOutput(DataOutput in) { + this.in = in; + } + + @Override + public void writeByte(byte b) throws IOException { + crc.update(b); + in.writeByte(b); + } + + @Override + public void writeBytes(byte[] b, int offset, int length) throws IOException { + crc.update(b, offset, length); + in.writeBytes(b, offset, length); + } + + long getChecksum() { + return crc.getValue(); + } + } + private int numStoredFieldsInDoc; @Override @@ -246,18 +284,30 @@ private void flush(boolean force) throws IOException { final boolean dirtyChunk = force; writeHeader(docBase, numBufferedDocs, numStoredFields, lengths, sliced, dirtyChunk); ByteBuffersDataInput bytebuffers = bufferedDocs.toDataInput(); - // compress stored fields to fieldsStream. + + // A CRC32C of the compressed bytes, so that a corrupt chunk is rejected before it reaches the + // decompressor rather than surfacing as an exception from inside it, or as a wrong document. + // This is where the LZ4 frame format puts its optional per-block checksum, "calculated by using + // the xxHash-32 algorithm on the raw (compressed) data block [...] The intention is to detect + // data corruption immediately, before decoding". CRC32C is used rather than xxHash-32 because + // it + // is in the JDK and hardware-accelerated on the platforms Lucene targets. + // + // The checksum is written after the compressed bytes, since its value is not known until they + // have been produced, and read from the end of the chunk, whose length the fields index knows. + final ChecksummingDataOutput checksummed = new ChecksummingDataOutput(fieldsStream); if (sliced) { // big chunk, slice it, using ByteBuffersDataInput ignore memory copy final int capacity = (int) bytebuffers.length(); for (int compressed = 0; compressed < capacity; compressed += chunkSize) { int l = Math.min(chunkSize, capacity - compressed); ByteBuffersDataInput bbdi = bytebuffers.slice(compressed, l); - compressor.compress(bbdi, fieldsStream); + compressor.compress(bbdi, checksummed); } } else { - compressor.compress(bytebuffers, fieldsStream); + compressor.compress(bytebuffers, checksummed); } + fieldsStream.writeInt((int) checksummed.getChecksum()); // reset docBase += numBufferedDocs; diff --git a/lucene/core/src/test/org/apache/lucene/codecs/lucene90/compressing/TestChunkChecksum.java b/lucene/core/src/test/org/apache/lucene/codecs/lucene90/compressing/TestChunkChecksum.java new file mode 100644 index 000000000000..1398b890086d --- /dev/null +++ b/lucene/core/src/test/org/apache/lucene/codecs/lucene90/compressing/TestChunkChecksum.java @@ -0,0 +1,360 @@ +/* + * 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.lucene.codecs.lucene90.compressing; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import org.apache.lucene.codecs.lucene104.Lucene104Codec; +import org.apache.lucene.document.Document; +import org.apache.lucene.document.Field; +import org.apache.lucene.document.StoredField; +import org.apache.lucene.document.StringField; +import org.apache.lucene.index.CorruptIndexException; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.StoredFields; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.IOContext; +import org.apache.lucene.store.IndexInput; +import org.apache.lucene.store.IndexOutput; +import org.apache.lucene.tests.store.MockDirectoryWrapper; +import org.apache.lucene.tests.util.LuceneTestCase; + +/** + * Tests the per-chunk checksum in {@link Lucene90CompressingStoredFieldsFormat}: a chunk whose + * compressed bytes do not match their CRC32C must be rejected before the decompressor sees them. + * + *

    Corruption is introduced by changing the recorded checksum rather than by flipping bytes at + * random, so every run exercises the same path — the method @rmuir asked for on GITHUB#10396, where + * a byte-flipping test was written and then disabled because not all corruptions are detected. + */ +public class TestChunkChecksum extends LuceneTestCase { + + private static final int NUM_DOCS = 500; + + /** Builds an index with several chunks and returns the name of its {@code .fdt}. */ + private String buildIndex(Directory dir) throws IOException { + IndexWriterConfig iwc = + new IndexWriterConfig().setCodec(new Lucene104Codec(Lucene104Codec.Mode.BEST_SPEED)); + iwc.setUseCompoundFile(false); + try (IndexWriter w = new IndexWriter(dir, iwc)) { + for (int i = 0; i < NUM_DOCS; i++) { + Document doc = new Document(); + doc.add(new StringField("id", Integer.toString(i), Field.Store.NO)); + doc.add( + new StoredField( + "body", + "document number " + + i + + " with enough repeated text that the chunk compresses the way a real stored " + + "field would, mentioning searching and indexing and merging")); + w.addDocument(doc); + } + w.forceMerge(1); + w.commit(); + } + + for (String file : dir.listAll()) { + if (file.endsWith(".fdt")) { + return file; + } + } + throw new AssertionError("no .fdt in " + List.of(dir.listAll())); + } + + private static byte[] readAll(Directory dir, String name) throws IOException { + try (IndexInput in = dir.openInput(name, IOContext.READONCE)) { + byte[] bytes = new byte[(int) in.length()]; + in.readBytes(bytes, 0, bytes.length); + return bytes; + } + } + + private static void writeAll(Directory dir, String name, byte[] bytes) throws IOException { + dir.deleteFile(name); + try (IndexOutput out = dir.createOutput(name, IOContext.DEFAULT)) { + out.writeBytes(bytes, bytes.length); + } + } + + private static List readAllDocs(Directory dir) throws IOException { + List bodies = new ArrayList<>(); + try (DirectoryReader reader = DirectoryReader.open(dir)) { + StoredFields storedFields = reader.storedFields(); + for (int i = 0; i < reader.maxDoc(); i++) { + bodies.add(storedFields.document(i).get("body")); + } + } + return bodies; + } + + /** A valid index round-trips: every document comes back, and the checksums all match. */ + public void testValidIndexRoundTrips() throws Exception { + try (Directory dir = newDirectory()) { + buildIndex(dir); + List bodies = readAllDocs(dir); + assertEquals(NUM_DOCS, bodies.size()); + for (int i = 0; i < NUM_DOCS; i++) { + assertTrue(bodies.get(i), bodies.get(i).contains("document number " + i)); + } + } + } + + /** + * Changing a chunk's recorded checksum must be reported, and reported as a chunk checksum + * mismatch rather than as whatever the decompressor happens to do with unexpected bytes. + */ + public void testCorruptChecksumIsDetected() throws Exception { + try (MockDirectoryWrapper dir = newMockDirectory()) { + // this test intentionally leaves a corrupt index behind + dir.setCheckIndexOnClose(false); + String fdt = buildIndex(dir); + byte[] bytes = readAll(dir, fdt); + + // The last four bytes before the codec footer are the final chunk's checksum. Changing one + // bit + // of it is enough, and unlike a byte flip in the payload it is guaranteed to be detected. + int footerLength = 16; + int checksumOffset = bytes.length - footerLength - Integer.BYTES; + bytes[checksumOffset] ^= 1; + writeAll(dir, fdt, bytes); + + CorruptIndexException e = expectThrows(CorruptIndexException.class, () -> readAllDocs(dir)); + assertTrue(e.getMessage(), e.getMessage().contains("chunk checksum mismatch")); + assertTrue(e.getMessage(), e.getMessage().contains("docBase=")); + assertTrue(e.getMessage(), e.getMessage().contains("chunkDocs=")); + } + } + + /** + * A byte changed inside the compressed payload is detected too, and — this is the point of + * checking before decoding — it is detected as a checksum mismatch rather than by the + * decompressor. + */ + public void testCorruptPayloadIsDetectedAsChecksumMismatch() throws Exception { + try (MockDirectoryWrapper dir = newMockDirectory()) { + // this test intentionally leaves a corrupt index behind + dir.setCheckIndexOnClose(false); + String fdt = buildIndex(dir); + byte[] bytes = readAll(dir, fdt); + + // Somewhere inside the compressed data of the last chunk: before its checksum, after the + // header. The exact position does not matter, only that it is payload. + int checksumOffset = bytes.length - 16 - Integer.BYTES; + bytes[checksumOffset - 8] ^= 1; + writeAll(dir, fdt, bytes); + + CorruptIndexException e = expectThrows(CorruptIndexException.class, () -> readAllDocs(dir)); + assertTrue(e.getMessage(), e.getMessage().contains("chunk checksum mismatch")); + } + } + + /** + * The measurement @rmuir asked for on GITHUB#10396: rather than flipping random bytes and + * reporting a distribution of outcomes, corrupt every chunk in turn, deterministically, and + * require that each one is detected. A byte-flipping test was written there and disabled because + * Lucene did not detect every corruption; this asserts the property that a per-chunk checksum is + * supposed to guarantee. + */ + public void testEveryChunkIsCovered() throws Exception { + try (MockDirectoryWrapper dir = newMockDirectory()) { + dir.setCheckIndexOnClose(false); + String fdt = buildIndex(dir); + byte[] pristine = readAll(dir, fdt); + + // Each chunk ends with its 4-byte checksum, and the file ends with a 16-byte codec footer. We + // do not know the boundaries from the bytes alone, so corrupt one byte at every offset and + // count how many are caught. With a checksum before decoding, all of them must be. + int footerLength = 16; + // corrupting the header is a different failure, so start after it + final int headerLength = + org.apache.lucene.codecs.CodecUtil.headerLength("Lucene90StoredFieldsFastData"); + + int detected = 0; + int undetected = 0; + int other = 0; + int stride = 37; + int sampled = 0; + for (int pos = headerLength; pos < pristine.length - footerLength; pos += stride) { + sampled++; + byte[] mutated = pristine.clone(); + mutated[pos] ^= 0xFF; + writeAll(dir, fdt, mutated); + + try { + List bodies = readAllDocs(dir); + boolean wrong = false; + for (int i = 0; i < bodies.size(); i++) { + String body = bodies.get(i); + if (body == null || body.contains("document number " + i) == false) { + wrong = true; + break; + } + } + if (wrong) { + undetected++; + } + // else: the byte was in a region that does not change any document, which is not a miss + } catch (CorruptIndexException e) { + if (e.getMessage() != null && e.getMessage().contains("chunk checksum mismatch")) { + detected++; + } else { + other++; + } + } catch (@SuppressWarnings("unused") Exception | AssertionError ignored) { + other++; + } + } + + writeAll(dir, fdt, pristine); + + String summary = + "sampled=" + + sampled + + " detectedByChunkChecksum=" + + detected + + " otherError=" + + other + + " silentlyWrong=" + + undetected; + if (VERBOSE) { + System.out.println(summary); + } + assertEquals( + summary + " -- no corruption may return a wrong document silently", 0, undetected); + } + } + + /** + * A merge re-encodes stored fields whenever the source segments are not already at the current + * format version, so an existing index acquires chunk checksums by being merged — which is what + * {@link org.apache.lucene.index.IndexUpgrader} does. No separate upgrade step is needed. + * + *

    Verified end to end elsewhere against an index written by the previous format version; here + * the mechanism is pinned: {@code getMergeStrategy} must refuse the bulk-copy path for a reader + * whose version is not {@code VERSION_CURRENT}, and re-encode instead. + */ + public void testMergeRewritesStoredFields() throws Exception { + try (Directory dir = newDirectory()) { + IndexWriterConfig iwc = + new IndexWriterConfig() + .setCodec(new Lucene104Codec(Lucene104Codec.Mode.BEST_SPEED)) + .setMergePolicy(org.apache.lucene.index.NoMergePolicy.INSTANCE); + iwc.setUseCompoundFile(false); + try (IndexWriter w = new IndexWriter(dir, iwc)) { + for (int seg = 0; seg < 2; seg++) { + for (int i = 0; i < 100; i++) { + Document doc = new Document(); + doc.add(new StoredField("body", "document number " + (seg * 100 + i) + " with text")); + w.addDocument(doc); + } + w.commit(); + } + } + + try (DirectoryReader reader = DirectoryReader.open(dir)) { + assertEquals(2, reader.leaves().size()); + } + + try (IndexWriter w = + new IndexWriter( + dir, + new IndexWriterConfig() + .setCodec(new Lucene104Codec(Lucene104Codec.Mode.BEST_SPEED)))) { + w.forceMerge(1); + w.commit(); + } + + // every document survives the merge, and the merged segment verifies its own checksums on + // read + try (DirectoryReader reader = DirectoryReader.open(dir)) { + assertEquals(1, reader.leaves().size()); + StoredFields storedFields = reader.storedFields(); + for (int i = 0; i < reader.maxDoc(); i++) { + assertTrue(storedFields.document(i).get("body").contains("document number " + i)); + } + } + } + } + + /** The checksum must work for BEST_COMPRESSION too, whose chunks and codec differ. */ + public void testBestCompressionMode() throws Exception { + try (MockDirectoryWrapper dir = newMockDirectory()) { + dir.setCheckIndexOnClose(false); + IndexWriterConfig iwc = + new IndexWriterConfig() + .setCodec(new Lucene104Codec(Lucene104Codec.Mode.BEST_COMPRESSION)); + iwc.setUseCompoundFile(false); + try (IndexWriter w = new IndexWriter(dir, iwc)) { + for (int i = 0; i < NUM_DOCS; i++) { + Document doc = new Document(); + doc.add( + new StoredField("body", "document number " + i + " with repeated compressible text")); + w.addDocument(doc); + } + w.forceMerge(1); + w.commit(); + } + + String fdt = null; + for (String file : dir.listAll()) { + if (file.endsWith(".fdt")) { + fdt = file; + } + } + assertNotNull(fdt); + + // valid first + assertEquals(NUM_DOCS, readAllDocs(dir).size()); + + byte[] bytes = readAll(dir, fdt); + bytes[bytes.length - 16 - Integer.BYTES] ^= 1; + writeAll(dir, fdt, bytes); + + CorruptIndexException e = expectThrows(CorruptIndexException.class, () -> readAllDocs(dir)); + assertTrue(e.getMessage(), e.getMessage().contains("chunk checksum mismatch")); + } + } + + /** + * A document larger than the chunk size produces a "sliced" chunk, compressed in several passes. + * One checksum covers the whole chunk, so the slicing must not break it. + */ + public void testSlicedChunk() throws Exception { + try (Directory dir = newDirectory()) { + IndexWriterConfig iwc = + new IndexWriterConfig().setCodec(new Lucene104Codec(Lucene104Codec.Mode.BEST_SPEED)); + iwc.setUseCompoundFile(false); + StringBuilder big = new StringBuilder(); + while (big.length() < 200_000) { + big.append("a large stored field that will not fit in a single chunk, repeated. "); + } + try (IndexWriter w = new IndexWriter(dir, iwc)) { + Document doc = new Document(); + doc.add(new StoredField("body", big.toString())); + w.addDocument(doc); + w.commit(); + } + + try (DirectoryReader reader = DirectoryReader.open(dir)) { + assertEquals(big.toString(), reader.storedFields().document(0).get("body")); + } + } + } +} From a180147315596949397283d5add743a040337ef5 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 19:45:27 +0300 Subject: [PATCH 2/3] GITHUB#16479: verify a CRC32C of each term-vectors chunk before decompressing it Term vectors share the chunked layout of stored fields -- same package, same Compressor, same fields index -- and had the same gap the previous commit closed there: a corrupt byte inside a chunk surfaced as whatever the decompressor did with it, or as a wrong term vector with no error at all. The change is the same shape. Each chunk now ends with a CRC32C of its compressed bytes, computed as they are written through a filtering DataOutput, and verified before the decompressor is called. The chunk's length comes from the fields index rather than from the file, so a corrupt length cannot be used to read outside the chunk. Term vectors format version 1. Version 0 is read exactly as before, verified by writing an index with the previous version and reading it with this one: 200 of 200 documents' vectors read back, CheckIndex clean. A segment acquires checksums when it is next merged, by the same mechanism as stored fields. One difference from stored fields is worth noting: a chunk with no fields at all writes no compressed payload, so there is nothing to checksum and nothing is written. testDocumentsWithoutVectors covers that, mixing documents that have vectors with documents that do not. Tests mirror the stored-fields ones and were each mutation-checked: bypassing the verification makes both corruption tests fail, and writing a wrong checksum makes the round-trip test fail. --- lucene/CHANGES.txt | 4 + .../lucene/codecs/lucene104/package-info.java | 6 +- .../Lucene90CompressingTermVectorsReader.java | 59 +++++ .../Lucene90CompressingTermVectorsWriter.java | 49 +++- .../TestTermVectorsChunkChecksum.java | 234 ++++++++++++++++++ 5 files changed, 347 insertions(+), 5 deletions(-) create mode 100644 lucene/core/src/test/org/apache/lucene/codecs/lucene90/compressing/TestTermVectorsChunkChecksum.java diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index a6c6a5309341..49f4a5ff0448 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -131,6 +131,10 @@ New Features Improvements --------------------- +* GITHUB#16479: Term vectors now record a CRC32C of each chunk's compressed bytes and verify it before + decompressing, as stored fields do. Term vectors format version 1; version 0 is read unchanged, and a + segment acquires checksums when it is next merged. (Serhiy Bzhezytskyy) + * GITHUB#16479: Stored fields now record a CRC32C of each chunk's compressed bytes and verify it before decompressing, so a corrupt chunk is reported as a CorruptIndexException naming the affected documents rather than surfacing from inside the decompressor or returning a wrong document. Stored diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene104/package-info.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene104/package-info.java index af5bd17a455c..1be978272035 100644 --- a/lucene/core/src/java/org/apache/lucene/codecs/lucene104/package-info.java +++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene104/package-info.java @@ -421,9 +421,9 @@ *

  • In version 10.3, the index of block tree changed to be specialized trie instead of FST. *
  • In version 10.4, the block size was increased from 128 to 256. There are now 256 and 8,192 * postings between skip pointers instead of 128 and 4,096. - *
  • In version 11.0, each chunk of stored fields is followed by a CRC32C of its compressed - * bytes, which is verified before the chunk is decompressed. Chunks written by an earlier - * version have no checksum and are read as before. + *
  • In version 11.0, each chunk of stored fields and of term vectors is followed by a CRC32C of + * its compressed bytes, which is verified before the chunk is decompressed. Chunks written by + * an earlier version have no checksum and are read as before. * * * diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingTermVectorsReader.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingTermVectorsReader.java index fd058ed5c836..fedfc4add4e8 100644 --- a/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingTermVectorsReader.java +++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingTermVectorsReader.java @@ -26,6 +26,7 @@ import static org.apache.lucene.codecs.lucene90.compressing.Lucene90CompressingTermVectorsWriter.VECTORS_INDEX_CODEC_NAME; import static org.apache.lucene.codecs.lucene90.compressing.Lucene90CompressingTermVectorsWriter.VECTORS_INDEX_EXTENSION; import static org.apache.lucene.codecs.lucene90.compressing.Lucene90CompressingTermVectorsWriter.VECTORS_META_EXTENSION; +import static org.apache.lucene.codecs.lucene90.compressing.Lucene90CompressingTermVectorsWriter.VERSION_CHUNK_CHECKSUM; import static org.apache.lucene.codecs.lucene90.compressing.Lucene90CompressingTermVectorsWriter.VERSION_CURRENT; import static org.apache.lucene.codecs.lucene90.compressing.Lucene90CompressingTermVectorsWriter.VERSION_START; @@ -35,6 +36,7 @@ import java.util.Collections; import java.util.Iterator; import java.util.NoSuchElementException; +import java.util.zip.CRC32C; import org.apache.lucene.codecs.CodecUtil; import org.apache.lucene.codecs.TermVectorsReader; import org.apache.lucene.codecs.compressing.CompressionMode; @@ -358,6 +360,57 @@ public void prefetch(int docID) throws IOException { prefetchedBlockIDCache[prefetchedBlockIDCacheIndex++ & PREFETCH_CACHE_MASK] = blockID; } + /** + * Verifies the CRC32C that follows a chunk's compressed bytes, before anything decompresses them. + * + *

    The chunk's length comes from the fields index rather than from the file, so a corrupt + * length cannot be used to read outside the chunk. Leaves the stream where it was found. + */ + private void verifyCompressedChunk(int doc, int docBase, int chunkDocs) throws IOException { + final long compressedStart = vectorsStream.getFilePointer(); + final long blockID = indexReader.getBlockID(doc); + final long blockEnd = + indexReader.getBlockStartPointer(blockID) + indexReader.getBlockLength(blockID); + final long compressedLength = blockEnd - Integer.BYTES - compressedStart; + if (compressedLength < 0) { + throw new CorruptIndexException( + "chunk shorter than its checksum: docBase=" + + docBase + + ", chunkDocs=" + + chunkDocs + + ", length=" + + compressedLength, + vectorsStream); + } + + final CRC32C crc = new CRC32C(); + final byte[] scratch = new byte[8192]; + long remaining = compressedLength; + while (remaining > 0) { + final int n = (int) Math.min(scratch.length, remaining); + vectorsStream.readBytes(scratch, 0, n); + crc.update(scratch, 0, n); + remaining -= n; + } + final int actual = (int) crc.getValue(); + final int expected = vectorsStream.readInt(); + if (actual != expected) { + throw new CorruptIndexException( + "chunk checksum mismatch: docBase=" + + docBase + + ", chunkDocs=" + + chunkDocs + + ", compressedLength=" + + compressedLength + + ", expected=" + + Integer.toHexString(expected) + + ", actual=" + + Integer.toHexString(actual), + vectorsStream); + } + vectorsStream.seek(compressedStart); + } + @Override public Fields get(int doc) throws IOException { ensureOpen(); @@ -701,6 +754,12 @@ public Fields get(int doc) throws IOException { assert termIndex == totalTerms : termIndex + " " + totalTerms; } + if (version >= VERSION_CHUNK_CHECKSUM) { + // Verify the compressed bytes before the decompressor sees them, so that a corrupt chunk is + // reported as such rather than surfacing from inside the decompression routine. + verifyCompressedChunk(doc, docBase, chunkDocs); + } + // decompress data final BytesRef suffixBytes = new BytesRef(); decompressor.decompress( diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingTermVectorsWriter.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingTermVectorsWriter.java index c95164b0b868..911fb63e80c0 100644 --- a/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingTermVectorsWriter.java +++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene90/compressing/Lucene90CompressingTermVectorsWriter.java @@ -26,6 +26,7 @@ import java.util.Deque; import java.util.Iterator; import java.util.List; +import java.util.zip.CRC32C; import org.apache.lucene.codecs.CodecUtil; import org.apache.lucene.codecs.TermVectorsReader; import org.apache.lucene.codecs.TermVectorsWriter; @@ -43,6 +44,7 @@ import org.apache.lucene.store.ByteBuffersDataInput; import org.apache.lucene.store.ByteBuffersDataOutput; import org.apache.lucene.store.DataInput; +import org.apache.lucene.store.DataOutput; import org.apache.lucene.store.Directory; import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexInput; @@ -69,7 +71,14 @@ public final class Lucene90CompressingTermVectorsWriter extends TermVectorsWrite static final String VECTORS_INDEX_CODEC_NAME = "Lucene90TermVectorsIndex"; static final int VERSION_START = 0; - static final int VERSION_CURRENT = VERSION_START; + + /** + * Each chunk is followed by a CRC32C of its compressed bytes, so that a corrupt chunk is rejected + * before it reaches the decompressor. + */ + static final int VERSION_CHUNK_CHECKSUM = 1; + + static final int VERSION_CURRENT = VERSION_CHUNK_CHECKSUM; static final int META_VERSION_START = 0; static final int PACKED_BLOCK_SIZE = 64; @@ -378,6 +387,36 @@ private boolean triggerFlush() { return termSuffixes.size() >= chunkSize || pendingDocs.size() >= maxDocsPerChunk; } + /** + * Passes bytes through to a delegate while computing a CRC32C of them, so that the compressed + * bytes of a chunk can be checksummed as they are written rather than buffered and hashed + * afterwards. + */ + private static final class ChecksummingDataOutput extends DataOutput { + private final DataOutput in; + private final CRC32C crc = new CRC32C(); + + ChecksummingDataOutput(DataOutput in) { + this.in = in; + } + + @Override + public void writeByte(byte b) throws IOException { + crc.update(b); + in.writeByte(b); + } + + @Override + public void writeBytes(byte[] b, int offset, int length) throws IOException { + crc.update(b, offset, length); + in.writeBytes(b, offset, length); + } + + long getChecksum() { + return crc.getValue(); + } + } + private void flush(boolean force) throws IOException { assert force != triggerFlush(); final int chunkDocs = pendingDocs.size(); @@ -421,7 +460,13 @@ private void flush(boolean force) throws IOException { // compress terms and payloads and write them to the output // using ByteBuffersDataInput reduce memory copy ByteBuffersDataInput content = termSuffixes.toDataInput(); - compressor.compress(content, vectorsStream); + // A CRC32C of the compressed bytes, so that a corrupt chunk is rejected before it reaches the + // decompressor rather than surfacing from inside it. See the same check in + // Lucene90CompressingStoredFieldsWriter, and the optional per-block checksum of the LZ4 frame + // format, whose "intention is to detect data corruption [...] immediately, before decoding". + ChecksummingDataOutput checksummed = new ChecksummingDataOutput(vectorsStream); + compressor.compress(content, checksummed); + vectorsStream.writeInt((int) checksummed.getChecksum()); } // reset diff --git a/lucene/core/src/test/org/apache/lucene/codecs/lucene90/compressing/TestTermVectorsChunkChecksum.java b/lucene/core/src/test/org/apache/lucene/codecs/lucene90/compressing/TestTermVectorsChunkChecksum.java new file mode 100644 index 000000000000..48d0cb84fc84 --- /dev/null +++ b/lucene/core/src/test/org/apache/lucene/codecs/lucene90/compressing/TestTermVectorsChunkChecksum.java @@ -0,0 +1,234 @@ +/* + * 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.lucene.codecs.lucene90.compressing; + +import java.io.IOException; +import java.util.List; +import org.apache.lucene.codecs.lucene104.Lucene104Codec; +import org.apache.lucene.document.Document; +import org.apache.lucene.document.Field; +import org.apache.lucene.document.FieldType; +import org.apache.lucene.document.TextField; +import org.apache.lucene.index.CorruptIndexException; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.NoMergePolicy; +import org.apache.lucene.index.TermVectors; +import org.apache.lucene.index.Terms; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.IOContext; +import org.apache.lucene.store.IndexInput; +import org.apache.lucene.store.IndexOutput; +import org.apache.lucene.tests.store.MockDirectoryWrapper; +import org.apache.lucene.tests.util.LuceneTestCase; + +/** + * Tests the per-chunk checksum in {@link Lucene90CompressingTermVectorsFormat}, which shares the + * chunked layout of {@link Lucene90CompressingStoredFieldsFormat} and had the same gap: a corrupt + * byte inside a chunk surfaced as whatever the decompressor did with it. + * + *

    Corruption is introduced by changing recorded bytes deterministically rather than at random, + * so each run exercises the same path. + */ +public class TestTermVectorsChunkChecksum extends LuceneTestCase { + + private static final int NUM_DOCS = 200; + + private static FieldType vectorType() { + FieldType ft = new FieldType(TextField.TYPE_NOT_STORED); + ft.setStoreTermVectors(true); + ft.setStoreTermVectorPositions(true); + ft.setStoreTermVectorOffsets(true); + ft.freeze(); + return ft; + } + + /** Builds an index with term vectors over several chunks, and returns the {@code .tvd} name. */ + private String buildIndex(Directory dir) throws IOException { + FieldType ft = vectorType(); + IndexWriterConfig iwc = + new IndexWriterConfig().setCodec(new Lucene104Codec(Lucene104Codec.Mode.BEST_SPEED)); + iwc.setUseCompoundFile(false); + try (IndexWriter w = new IndexWriter(dir, iwc)) { + for (int i = 0; i < NUM_DOCS; i++) { + Document doc = new Document(); + doc.add( + new Field( + "body", + "term vector document " + i + " with repeated words words words to compress", + ft)); + w.addDocument(doc); + } + w.forceMerge(1); + w.commit(); + } + + for (String file : dir.listAll()) { + if (file.endsWith(".tvd")) { + return file; + } + } + throw new AssertionError("no .tvd in " + List.of(dir.listAll())); + } + + private static byte[] readAll(Directory dir, String name) throws IOException { + try (IndexInput in = dir.openInput(name, IOContext.READONCE)) { + byte[] bytes = new byte[(int) in.length()]; + in.readBytes(bytes, 0, bytes.length); + return bytes; + } + } + + private static void writeAll(Directory dir, String name, byte[] bytes) throws IOException { + dir.deleteFile(name); + try (IndexOutput out = dir.createOutput(name, IOContext.DEFAULT)) { + out.writeBytes(bytes, bytes.length); + } + } + + private static int readAllVectors(Directory dir) throws IOException { + int withVectors = 0; + try (DirectoryReader reader = DirectoryReader.open(dir)) { + TermVectors termVectors = reader.termVectors(); + for (int i = 0; i < reader.maxDoc(); i++) { + Terms terms = termVectors.get(i, "body"); + if (terms != null && terms.size() > 0) { + withVectors++; + } + } + } + return withVectors; + } + + public void testValidIndexRoundTrips() throws Exception { + try (Directory dir = newDirectory()) { + buildIndex(dir); + assertEquals(NUM_DOCS, readAllVectors(dir)); + } + } + + /** Changing a chunk's recorded checksum must be reported as a chunk checksum mismatch. */ + public void testCorruptChecksumIsDetected() throws Exception { + try (MockDirectoryWrapper dir = newMockDirectory()) { + // this test intentionally leaves a corrupt index behind + dir.setCheckIndexOnClose(false); + String tvd = buildIndex(dir); + byte[] bytes = readAll(dir, tvd); + + // the last four bytes before the codec footer are the final chunk's checksum + bytes[bytes.length - 16 - Integer.BYTES] ^= 1; + writeAll(dir, tvd, bytes); + + CorruptIndexException e = + expectThrows(CorruptIndexException.class, () -> readAllVectors(dir)); + assertTrue(e.getMessage(), e.getMessage().contains("chunk checksum mismatch")); + assertTrue(e.getMessage(), e.getMessage().contains("docBase=")); + } + } + + /** And a byte changed inside the compressed payload, which is the case that used to be silent. */ + public void testCorruptPayloadIsDetectedAsChecksumMismatch() throws Exception { + try (MockDirectoryWrapper dir = newMockDirectory()) { + dir.setCheckIndexOnClose(false); + String tvd = buildIndex(dir); + byte[] bytes = readAll(dir, tvd); + + bytes[bytes.length - 16 - Integer.BYTES - 8] ^= 1; + writeAll(dir, tvd, bytes); + + CorruptIndexException e = + expectThrows(CorruptIndexException.class, () -> readAllVectors(dir)); + assertTrue(e.getMessage(), e.getMessage().contains("chunk checksum mismatch")); + } + } + + /** + * Documents without term vectors produce chunks with no compressed payload at all, which must not + * be treated as a chunk whose checksum is missing. + */ + public void testDocumentsWithoutVectors() throws Exception { + try (Directory dir = newDirectory()) { + FieldType ft = vectorType(); + IndexWriterConfig iwc = + new IndexWriterConfig() + .setCodec(new Lucene104Codec(Lucene104Codec.Mode.BEST_SPEED)) + .setMergePolicy(NoMergePolicy.INSTANCE); + try (IndexWriter w = new IndexWriter(dir, iwc)) { + for (int i = 0; i < 50; i++) { + Document doc = new Document(); + if (i % 2 == 0) { + doc.add(new Field("body", "document " + i + " with vectors and repeated words", ft)); + } else { + doc.add(new TextField("plain", "document " + i + " without vectors", Field.Store.NO)); + } + w.addDocument(doc); + } + w.commit(); + } + + try (DirectoryReader reader = DirectoryReader.open(dir)) { + TermVectors termVectors = reader.termVectors(); + int withVectors = 0; + for (int i = 0; i < reader.maxDoc(); i++) { + Terms terms = termVectors.get(i, "body"); + if (terms != null && terms.size() > 0) { + withVectors++; + } + } + assertEquals(25, withVectors); + } + } + } + + /** A merge re-encodes term vectors from an older format version, so they acquire checksums. */ + public void testMergeRewritesTermVectors() throws Exception { + try (Directory dir = newDirectory()) { + FieldType ft = vectorType(); + IndexWriterConfig iwc = + new IndexWriterConfig() + .setCodec(new Lucene104Codec(Lucene104Codec.Mode.BEST_SPEED)) + .setMergePolicy(NoMergePolicy.INSTANCE); + iwc.setUseCompoundFile(false); + try (IndexWriter w = new IndexWriter(dir, iwc)) { + for (int seg = 0; seg < 2; seg++) { + for (int i = 0; i < 50; i++) { + Document doc = new Document(); + doc.add(new Field("body", "document " + (seg * 50 + i) + " with repeated words", ft)); + w.addDocument(doc); + } + w.commit(); + } + } + + try (DirectoryReader reader = DirectoryReader.open(dir)) { + assertEquals(2, reader.leaves().size()); + } + + try (IndexWriter w = + new IndexWriter( + dir, + new IndexWriterConfig() + .setCodec(new Lucene104Codec(Lucene104Codec.Mode.BEST_SPEED)))) { + w.forceMerge(1); + w.commit(); + } + + assertEquals(100, readAllVectors(dir)); + } + } +} From c73a719789c83b9d63df9c307ce3ed265615940e Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 2 Aug 2026 20:04:37 +0300 Subject: [PATCH 3/3] Reject an invalid exception offset in LowercaseAsciiCompression#decompress Step 4 of decompress restores "exceptions" -- bytes that did not fit the compressible range -- by accumulating deltas read from the data and using the result to index the output: int i = 0; for (int exception = 0; exception < numExceptions; ++exception) { i += in.readByte() & 0xFF; out[i] = in.readByte(); } Nothing bounds i, so corrupt input fails with ArrayIndexOutOfBoundsException rather than a checked IOException. This is the same shape as the LZ4 match offset in GITHUB#16478: an offset taken from the data and used to index an array without validation. Found by measuring corruption in a .tim file rather than by reading the code. Flipping one byte at each of 320 sampled positions across a 67 KB .tim of 20,000 long shared-prefix terms, two of them landed here: ArrayIndexOutOfBoundsException @ ByteArrayDataInput.readBytes 8 2.5% ArrayIndexOutOfBoundsException @ LowercaseAsciiCompression:158 2 0.6% EOFException 4 1.3% IllegalArgumentException 5 1.6% wrong term count, no error 2 0.6% no effect 299 93.4% The ByteArrayDataInput cases are deliberate -- it carries the comment "NOTE: AIOOBE not EOF if you read too much" -- so they are left alone. The compressor cannot produce an out-of-range offset: it only records deltas between positions it has already visited within len, and inserts artificial exceptions when a delta would exceed 0xFF. So this only affects input that was not produced by compress(). --- lucene/CHANGES.txt | 4 +++ .../compress/LowercaseAsciiCompression.java | 6 +++++ .../TestLowercaseAsciiCompression.java | 26 +++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 49f4a5ff0448..b8074598cc40 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -131,6 +131,10 @@ New Features Improvements --------------------- +* GITHUB#16479: Reject an invalid exception offset in LowercaseAsciiCompression#decompress. Corrupt + input previously threw ArrayIndexOutOfBoundsException instead of a checked IOException. Same class of + issue as the LZ4 match-offset check in GITHUB#16478. (Serhiy Bzhezytskyy) + * GITHUB#16479: Term vectors now record a CRC32C of each chunk's compressed bytes and verify it before decompressing, as stored fields do. Term vectors format version 1; version 0 is read unchanged, and a segment acquires checksums when it is next merged. (Serhiy Bzhezytskyy) diff --git a/lucene/core/src/java/org/apache/lucene/util/compress/LowercaseAsciiCompression.java b/lucene/core/src/java/org/apache/lucene/util/compress/LowercaseAsciiCompression.java index f81f249afeff..4e8931e9675d 100644 --- a/lucene/core/src/java/org/apache/lucene/util/compress/LowercaseAsciiCompression.java +++ b/lucene/core/src/java/org/apache/lucene/util/compress/LowercaseAsciiCompression.java @@ -155,6 +155,12 @@ public static void decompress(DataInput in, byte[] out, int len) throws IOExcept int i = 0; for (int exception = 0; exception < numExceptions; ++exception) { i += in.readByte() & 0xFF; + if (i >= len) { + // The offsets are read from the data, so corrupt input can push this past the end of the + // output. Report it rather than letting the array access do so. + throw new IOException( + "exception offset " + i + " is invalid, only " + len + " bytes decoded"); + } out[i] = in.readByte(); } } diff --git a/lucene/core/src/test/org/apache/lucene/util/compress/TestLowercaseAsciiCompression.java b/lucene/core/src/test/org/apache/lucene/util/compress/TestLowercaseAsciiCompression.java index be29ade703f6..70379340510f 100644 --- a/lucene/core/src/test/org/apache/lucene/util/compress/TestLowercaseAsciiCompression.java +++ b/lucene/core/src/test/org/apache/lucene/util/compress/TestLowercaseAsciiCompression.java @@ -147,4 +147,30 @@ public void testAsciiCompressionRandom2() throws IOException { .getBytes(StandardCharsets.UTF_8)); } } + + /** + * A corrupt exception offset must not be used to index {@code out}. The offsets are read from the + * data as deltas and accumulated, so corruption can push the index past the end of the array; + * that used to surface as ArrayIndexOutOfBoundsException from the decompression loop rather than + * as a checked IOException. Measured on a corrupted {@code .tim}, this was 2 of 320 sampled + * single-byte corruptions. + */ + public void testCorruptExceptionOffset() throws IOException { + // A minimal well-formed stream: 8 packed bytes for len=8 (saved=2, compressedLen=6), then a + // single "exception" whose delta puts the index far past the end of the output. + ByteBuffersDataOutput compressed = new ByteBuffersDataOutput(); + for (int i = 0; i < 6; i++) { + compressed.writeByte((byte) 0x21); + } + compressed.writeVInt(1); // one exception + compressed.writeByte((byte) 0xFF); // delta 255, far beyond out.length + compressed.writeByte((byte) 'x'); // the byte it would write + + byte[] out = new byte[8]; + IOException e = + expectThrows( + IOException.class, + () -> LowercaseAsciiCompression.decompress(compressed.toDataInput(), out, 8)); + assertTrue(e.getMessage(), e.getMessage().contains("exception offset")); + } }