Record all bytes of the checksum in VerifyingIndexOutput

The fix in #13848 has an off by one issue where the first byte of the checksum was never written. Unfortunately most tests shadowed the problem and the first byte of the checksum seems to be very likely a 0 which causes only very rare failures. Relates to #13896 Relates to #13848
elastic · Oct 5, 2015 · 87d8b62 · 87d8b62
1 parent 16a8ce5
commit 87d8b62
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 20 deletions.
diff --git a/core/src/main/java/org/elasticsearch/index/store/Store.java b/core/src/main/java/org/elasticsearch/index/store/Store.java
@@ -1288,9 +1288,10 @@ public void verify() throws IOException {
         @Override
         public void writeByte(byte b) throws IOException {
             final long writtenBytes = this.writtenBytes++;
-            if (writtenBytes == checksumPosition) {
-                readAndCompareChecksum();
-            } else if (writtenBytes > checksumPosition) { // we are writing parts of the checksum....
+            if (writtenBytes >= checksumPosition) { // we are writing parts of the checksum....
+                if (writtenBytes == checksumPosition) {
+                    readAndCompareChecksum();
+                }
                 final long indexLong = writtenBytes - checksumPosition;
                 if ((int)indexLong != indexLong) {
                     throw new ArithmeticException("integer overflow");
@@ -1299,7 +1300,7 @@ public void writeByte(byte b) throws IOException {
                 if (index < footerChecksum.length) {
                     footerChecksum[index] = b;
                     if (index == footerChecksum.length-1) {
-                        verify();// we have recorded the entire checksum
+                        verify(); // we have recorded the entire checksum
                     }
                 } else {
                     verify(); // fail if we write more than expected
@@ -1321,24 +1322,14 @@ private void readAndCompareChecksum() throws IOException {
         @Override
         public void writeBytes(byte[] b, int offset, int length) throws IOException {
             if (writtenBytes + length > checksumPosition) {
-                if (actualChecksum == null) {
-                    assert writtenBytes <= checksumPosition;
-                    final int bytesToWrite = (int) (checksumPosition - writtenBytes);
-                    out.writeBytes(b, offset, bytesToWrite);
-                    readAndCompareChecksum();
-                    offset += bytesToWrite;
-                    length -= bytesToWrite;
-                    writtenBytes += bytesToWrite;
-                }
-                for (int i = 0; i < length; i++) {
+                for (int i = 0; i < length; i++) { // don't optimze writing the last block of bytes
                     writeByte(b[offset+i]);
                 }
             } else {
                 out.writeBytes(b, offset, length);
                 writtenBytes += length;
             }
         }
-
     }
 
     /**

diff --git a/core/src/test/java/org/elasticsearch/index/store/StoreTests.java b/core/src/test/java/org/elasticsearch/index/store/StoreTests.java
@@ -210,11 +210,6 @@ public void testChecksumCorrupted() throws IOException {
                     verifyingOutput.writeByte(checksumBytes.bytes[i]);
                 }
             }
-            if (randomBoolean()) {
-               appendRandomData(verifyingOutput);
-            } else {
-                Store.verify(verifyingOutput);
-            }
             fail("should be a corrupted index");
         } catch (CorruptIndexException | IndexFormatTooOldException | IndexFormatTooNewException ex) {
             // ok