LUCENE-7822: CodecUtil#checkFooter should throw a CorruptIndexException as the main exception. (#1482)

lucene/CHANGES.txt

@@ -150,6 +150,10 @@ API Changes
   provide them lazily.  TermsInSetQuery switches to using this method
   to report matching terms. (Alan Woodward)
 
+* LUCENE-7822: CodecUtil#checkFooter(IndexInput, Throwable) now throws a
+  CorruptIndexException if checksums mismatch or if checksums can't be verified.
+  (Martin Amirault, Adrien Grand)
+
 New Features
 ---------------------
 (No changes)

lucene/core/src/java/org/apache/lucene/codecs/CodecUtil.java

@@ -448,24 +448,27 @@ public final class CodecUtil {
       checkFooter(in);
     } else {
       try {
+        // If we have evidence of corruption then we return the corruption as the
+        // main exception and the prior exception gets suppressed. Otherwise we
+        // return the prior exception with a suppressed exception that notifies
+        // the user that checksums matched.
         long remaining = in.length() - in.getFilePointer();
         if (remaining < footerLength()) {
           // corruption caused us to read into the checksum footer already: we can't proceed
-          priorException.addSuppressed(new CorruptIndexException("checksum status indeterminate: remaining=" + remaining +
+          throw new CorruptIndexException("checksum status indeterminate: remaining=" + remaining +
-                                                                 ", please run checkindex for more details", in));
+                                          "; please run checkindex for more details", in);
         } else {
           // otherwise, skip any unread bytes.
           in.skipBytes(remaining - footerLength());
           
           // now check the footer
-          try {
+          long checksum = checkFooter(in);
-            long checksum = checkFooter(in);
+          priorException.addSuppressed(new CorruptIndexException("checksum passed (" + Long.toHexString(checksum) +
-            priorException.addSuppressed(new CorruptIndexException("checksum passed (" + Long.toHexString(checksum) + 
+                                                                 "). possibly transient resource issue, or a Lucene or JVM bug", in));
-                                                                   "). possibly transient resource issue, or a Lucene or JVM bug", in));
-          } catch (CorruptIndexException t) {
-            priorException.addSuppressed(t);
-          }
         }
+      } catch (CorruptIndexException corruptException) {
+        corruptException.addSuppressed(priorException);
+        throw corruptException;
       } catch (Throwable t) {
         // catch-all for things that shouldn't go wrong (e.g. OOM during readInt) but could...
         priorException.addSuppressed(new CorruptIndexException("checksum status indeterminate: unexpected exception", in, t));

lucene/core/src/test/org/apache/lucene/codecs/TestCodecUtil.java

@@ -148,13 +148,13 @@ public class TestCodecUtil extends LuceneTestCase {
     // bogusly read a byte too far (can happen)
     input.readByte();
     Exception mine = new RuntimeException("fake exception");
-    RuntimeException expected = expectThrows(RuntimeException.class, () -> {
+    CorruptIndexException expected = expectThrows(CorruptIndexException.class, () -> {
       CodecUtil.checkFooter(input, mine);
     });
-    assertEquals("fake exception", expected.getMessage());
+    assertTrue(expected.getMessage().contains("checksum status indeterminate"));
     Throwable suppressed[] = expected.getSuppressed();
     assertEquals(1, suppressed.length);
-    assertTrue(suppressed[0].getMessage().contains("checksum status indeterminate"));
+    assertEquals("fake exception", suppressed[0].getMessage());
     input.close();
   }
   
@@ -172,13 +172,13 @@ public class TestCodecUtil extends LuceneTestCase {
     CodecUtil.checkHeader(input, "FooBar", 5, 5);
     assertEquals("this is the data", input.readString());
     Exception mine = new RuntimeException("fake exception");
-    RuntimeException expected = expectThrows(RuntimeException.class, () -> {
+    CorruptIndexException expected = expectThrows(CorruptIndexException.class, () -> {
       CodecUtil.checkFooter(input, mine);
     });
-    assertEquals("fake exception", expected.getMessage());
+    assertTrue(expected.getMessage().contains("checksum failed"));
     Throwable suppressed[] = expected.getSuppressed();
     assertEquals(1, suppressed.length);
-    assertTrue(suppressed[0].getMessage().contains("checksum failed"));
+    assertEquals("fake exception", suppressed[0].getMessage());
     input.close();
   }
   

lucene/core/src/test/org/apache/lucene/util/TestOfflineSorter.java

@@ -17,7 +17,6 @@
 package org.apache.lucene.util;
 
 
-import java.io.EOFException;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
@@ -351,14 +350,12 @@ public class TestOfflineSorter extends LuceneTestCase {
       IndexOutput unsorted = dir.createTempOutput("unsorted", "tmp", IOContext.DEFAULT);
       writeAll(unsorted, generateFixed(5*1024));
 
-      // This corruption made OfflineSorter fail with its own exception, but we verify it also went and added (as suppressed) that the
+      // This corruption made OfflineSorter fail with its own exception, but we verify and throw a CorruptIndexException
-      // checksum was wrong:
+      // instead when checksums don't match.
-      EOFException e = expectThrows(EOFException.class, () -> {
+      CorruptIndexException e = expectThrows(CorruptIndexException.class, () -> {
           new OfflineSorter(dir, "foo").sort(unsorted.getName());
         });
-      assertEquals(1, e.getSuppressed().length);
+      assertTrue(e.getMessage().contains("checksum failed (hardware problem?)"));
-      assertTrue(e.getSuppressed()[0] instanceof CorruptIndexException);
-      assertTrue(e.getSuppressed()[0].getMessage().contains("checksum failed (hardware problem?)"));
     }
   }
 
@@ -436,12 +433,10 @@ public class TestOfflineSorter extends LuceneTestCase {
       IndexOutput unsorted = dir.createTempOutput("unsorted", "tmp", IOContext.DEFAULT);
       writeAll(unsorted, generateFixed((int) (OfflineSorter.MB * 3)));
 
-      EOFException e = expectThrows(EOFException.class, () -> {
+      CorruptIndexException e = expectThrows(CorruptIndexException.class, () -> {
           new OfflineSorter(dir, "foo", OfflineSorter.DEFAULT_COMPARATOR, BufferSize.megabytes(1), 10, -1, null, 0).sort(unsorted.getName());
         });
-      assertEquals(1, e.getSuppressed().length);
+      assertTrue(e.getMessage().contains("checksum failed (hardware problem?)"));
-      assertTrue(e.getSuppressed()[0] instanceof CorruptIndexException);
-      assertTrue(e.getSuppressed()[0].getMessage().contains("checksum failed (hardware problem?)"));
     }
   }