diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/CRC32CDigestManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/CRC32CDigestManager.java index 0b9c469beb1..9d07f0feec0 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/CRC32CDigestManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/CRC32CDigestManager.java @@ -54,4 +54,14 @@ int internalUpdate(int digest, ByteBuf data, int offset, int len) { int internalUpdate(int digest, byte[] buffer, int offset, int len) { return Crc32cIntChecksum.resumeChecksum(digest, buffer, offset, len); } -} + + @Override + protected int initialDigest() { + return Crc32cIntChecksum.initialValue(); + } + + @Override + protected int finalizeDigest(int digest) { + return Crc32cIntChecksum.finalizeValue(digest); + } +} \ No newline at end of file diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/DigestManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/DigestManager.java index 1b69b997db1..0d6bb0f664d 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/DigestManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/DigestManager.java @@ -61,12 +61,31 @@ public abstract class DigestManager { abstract int internalUpdate(int digest, byte[] buffer, int offset, int len); + protected int initialDigest() { + return 0; + }; + + protected int finalizeDigest(int digest) { + return digest; + } + final int update(int digest, ByteBuf buffer, int offset, int len) { + if (len == 0) { + return digest; + } MutableInt digestRef = new MutableInt(digest); ByteBufVisitor.visitBuffers(buffer, offset, len, byteBufVisitorCallback, digestRef); return digestRef.intValue(); } + final protected int initializeAndUpdate(ByteBuf buf, int offset, int len) { + return update(initialDigest(), buf, offset, len); + } + + private int updateAndFinalize(ByteBuf data, int digest, int offset, int len) { + return finalizeDigest(update(digest, data, offset, len)); + } + abstract void populateValueAndReset(int digest, ByteBuf buffer); abstract boolean isInt32Digest(); @@ -145,8 +164,8 @@ private ReferenceCounted computeDigestAndPackageForSendingV2(long entryId, long buf.writeLong(length); // Compute checksum over the headers - int digest = update(0, buf, buf.readerIndex(), buf.readableBytes()); - digest = update(digest, data, data.readerIndex(), data.readableBytes()); + int digest = initializeAndUpdate(buf, buf.readerIndex(), buf.readableBytes()); + digest = updateAndFinalize(data, digest, data.readerIndex(), data.readableBytes()); populateValueAndReset(digest, buf); @@ -170,8 +189,8 @@ private ByteBufList computeDigestAndPackageForSendingV3(long entryId, long lastA headersBuffer.writeLong(lastAddConfirmed); headersBuffer.writeLong(length); - int digest = update(0, headersBuffer, 0, METADATA_LENGTH); - digest = update(digest, data, data.readerIndex(), data.readableBytes()); + int digest = initializeAndUpdate(headersBuffer, 0, METADATA_LENGTH); + digest = updateAndFinalize(data, digest, data.readerIndex(), data.readableBytes()); populateValueAndReset(digest, headersBuffer); return ByteBufList.get(headersBuffer, data); } @@ -193,7 +212,7 @@ public ByteBufList computeDigestAndPackageForSendingLac(long lac) { headersBuffer.writeLong(ledgerId); headersBuffer.writeLong(lac); - int digest = update(0, headersBuffer, 0, LAC_METADATA_LENGTH); + int digest = finalizeDigest(initializeAndUpdate(headersBuffer, 0, LAC_METADATA_LENGTH)); populateValueAndReset(digest, headersBuffer); return ByteBufList.get(headersBuffer); @@ -229,10 +248,10 @@ private void verifyDigest(long entryId, ByteBuf dataReceived, boolean skipEntryI this.getClass().getName(), dataReceived.readableBytes()); throw new BKDigestMatchException(); } - int digest = update(0, dataReceived, 0, METADATA_LENGTH); + int digest = initializeAndUpdate(dataReceived, 0, METADATA_LENGTH); int offset = METADATA_LENGTH + macCodeLength; - digest = update(digest, dataReceived, offset, dataReceived.readableBytes() - offset); + digest = updateAndFinalize(dataReceived, digest, offset, dataReceived.readableBytes() - offset); if (isInt32Digest()) { int receivedDigest = dataReceived.getInt(METADATA_LENGTH); @@ -277,7 +296,7 @@ public long verifyDigestAndReturnLac(ByteBuf dataReceived) throws BKDigestMatchE throw new BKDigestMatchException(); } - int digest = update(0, dataReceived, 0, LAC_METADATA_LENGTH); + int digest = finalizeDigest(initializeAndUpdate(dataReceived, 0, LAC_METADATA_LENGTH)); if (isInt32Digest()) { int receivedDigest = dataReceived.getInt(LAC_METADATA_LENGTH); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/checksum/CompositeByteBufUnwrapBugReproduceTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/checksum/CompositeByteBufUnwrapBugReproduceTest.java index f4ef4f18a8c..4ce2910bf2c 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/checksum/CompositeByteBufUnwrapBugReproduceTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/proto/checksum/CompositeByteBufUnwrapBugReproduceTest.java @@ -129,6 +129,16 @@ int internalUpdate(int digest, ByteBuf data, int offset, int len) { int internalUpdate(int digest, byte[] buffer, int offset, int len) { return intHash.resume(digest, buffer, offset, len); } + + @Override + protected int initialDigest() { + return intHash.initialValue(); + } + + @Override + protected int finalizeDigest(int digest) { + return intHash.finalizeValue(digest); + } } @Test diff --git a/circe-checksum/src/main/java/com/scurrilous/circe/checksum/Crc32cIntChecksum.java b/circe-checksum/src/main/java/com/scurrilous/circe/checksum/Crc32cIntChecksum.java index 0f0bcbe4528..27dab5a9afb 100644 --- a/circe-checksum/src/main/java/com/scurrilous/circe/checksum/Crc32cIntChecksum.java +++ b/circe-checksum/src/main/java/com/scurrilous/circe/checksum/Crc32cIntChecksum.java @@ -94,4 +94,11 @@ public static int resumeChecksum(int previousChecksum, byte[] payload, int offse return CRC32C_HASH.resume(previousChecksum, payload, offset, len); } + public static int initialValue() { + return CRC32C_HASH.initialValue(); + } + + public static int finalizeValue(int current) { + return CRC32C_HASH.finalizeValue(current); + } } diff --git a/circe-checksum/src/main/java/com/scurrilous/circe/checksum/IntHash.java b/circe-checksum/src/main/java/com/scurrilous/circe/checksum/IntHash.java index 84fb1c73364..92ab93f7862 100644 --- a/circe-checksum/src/main/java/com/scurrilous/circe/checksum/IntHash.java +++ b/circe-checksum/src/main/java/com/scurrilous/circe/checksum/IntHash.java @@ -30,4 +30,12 @@ public interface IntHash { int resume(int current, ByteBuf buffer, int offset, int len); int resume(int current, byte[] buffer, int offset, int len); + + default int initialValue() { + return 0; + } + + default int finalizeValue(int current) { + return current; + } } diff --git a/circe-checksum/src/main/java/com/scurrilous/circe/checksum/Java9IntHash.java b/circe-checksum/src/main/java/com/scurrilous/circe/checksum/Java9IntHash.java index dc2ec58ef79..28dbfaedc21 100644 --- a/circe-checksum/src/main/java/com/scurrilous/circe/checksum/Java9IntHash.java +++ b/circe-checksum/src/main/java/com/scurrilous/circe/checksum/Java9IntHash.java @@ -68,12 +68,12 @@ protected byte[] initialValue() { @Override public int calculate(ByteBuf buffer) { - return resume(0, buffer); + return finalizeValue(resume(initialValue(), buffer)); } @Override public int calculate(ByteBuf buffer, int offset, int len) { - return resume(0, buffer, offset, len); + return finalizeValue(resume(initialValue(), buffer, offset, len)); } private int resume(int current, long address, int offset, int length) { @@ -100,13 +100,11 @@ public int resume(int current, ByteBuf buffer) { @Override public int resume(int current, ByteBuf buffer, int offset, int len) { - int negCrc = ~current; - if (buffer.hasMemoryAddress()) { - negCrc = resume(negCrc, buffer.memoryAddress(), offset, len); + current = resume(current, buffer.memoryAddress(), offset, len); } else if (buffer.hasArray()) { int arrayOffset = buffer.arrayOffset() + offset; - negCrc = resume(negCrc, buffer.array(), arrayOffset, len); + current = resume(current, buffer.array(), arrayOffset, len); } else { byte[] b = TL_BUFFER.get(); int toRead = len; @@ -114,12 +112,23 @@ public int resume(int current, ByteBuf buffer, int offset, int len) { while (toRead > 0) { int length = Math.min(toRead, b.length); buffer.slice(loopOffset, length).readBytes(b, 0, length); - negCrc = resume(negCrc, b, 0, length); + current = resume(current, b, 0, length); toRead -= length; loopOffset += length; } } + return current; + } + + @Override + public int initialValue() { + // match java.util.zip.CRC32C.reset logic + return 0xFFFFFFFF; + } - return ~negCrc; + @Override + public int finalizeValue(int current) { + // match java.util.zip.CRC32C.getValue logic + return ~current; } } diff --git a/circe-checksum/src/test/java/com/scurrilous/circe/checksum/ChecksumTest.java b/circe-checksum/src/test/java/com/scurrilous/circe/checksum/ChecksumTest.java index f68b362a1af..b8283fd633b 100644 --- a/circe-checksum/src/test/java/com/scurrilous/circe/checksum/ChecksumTest.java +++ b/circe-checksum/src/test/java/com/scurrilous/circe/checksum/ChecksumTest.java @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ + package com.scurrilous.circe.checksum; import static com.scurrilous.circe.params.CrcParameters.CRC32C; @@ -44,7 +45,9 @@ public void testCrc32cValue() { @Test public void testCrc32cValueResume() { final byte[] bytes = "Some String".getBytes(); - int checksum = Crc32cIntChecksum.resumeChecksum(0, Unpooled.wrappedBuffer(bytes), 0, bytes.length); + int checksum = Crc32cIntChecksum.finalizeValue( + Crc32cIntChecksum.resumeChecksum(Crc32cIntChecksum.initialValue(), Unpooled.wrappedBuffer(bytes), 0, + bytes.length)); assertEquals(608512271, checksum); } @@ -56,22 +59,26 @@ public void testCrc32cValueIncremental() { int checksum = Crc32cIntChecksum.computeChecksum(Unpooled.wrappedBuffer(bytes)); assertEquals(608512271, checksum); - checksum = Crc32cIntChecksum.computeChecksum(Unpooled.wrappedBuffer(bytes, 0, 1)); + checksum = + Crc32cIntChecksum.resumeChecksum(Crc32cIntChecksum.initialValue(), Unpooled.wrappedBuffer(bytes, 0, 1)); for (int i = 1; i < bytes.length; i++) { checksum = Crc32cIntChecksum.resumeChecksum(checksum, Unpooled.wrappedBuffer(bytes), i, 1); } + checksum = Crc32cIntChecksum.finalizeValue(checksum); assertEquals(608512271, checksum); - checksum = Crc32cIntChecksum.computeChecksum(Unpooled.wrappedBuffer(bytes, 0, 4)); + checksum = + Crc32cIntChecksum.resumeChecksum(Crc32cIntChecksum.initialValue(), Unpooled.wrappedBuffer(bytes, 0, 4)); checksum = Crc32cIntChecksum.resumeChecksum(checksum, Unpooled.wrappedBuffer(bytes), 4, 7); + checksum = Crc32cIntChecksum.finalizeValue(checksum); assertEquals(608512271, checksum); ByteBuf buffer = Unpooled.wrappedBuffer(bytes, 0, 4); - checksum = Crc32cIntChecksum.computeChecksum(buffer); + checksum = Crc32cIntChecksum.resumeChecksum(Crc32cIntChecksum.initialValue(), buffer); checksum = Crc32cIntChecksum.resumeChecksum( - checksum, Unpooled.wrappedBuffer(bytes), 4, bytes.length - 4); - + checksum, Unpooled.wrappedBuffer(bytes), 4, bytes.length - 4); + checksum = Crc32cIntChecksum.finalizeValue(checksum); assertEquals(608512271, checksum); } @@ -86,7 +93,10 @@ public void testCrc32cLongValue() { @Test public void testCrc32cLongValueResume() { final byte[] bytes = "Some String".getBytes(); - long checksum = Crc32cIntChecksum.resumeChecksum(0, Unpooled.wrappedBuffer(bytes), 0, bytes.length); + long checksum = + Crc32cIntChecksum.resumeChecksum(Crc32cIntChecksum.initialValue(), Unpooled.wrappedBuffer(bytes), 0, + bytes.length); + checksum = Crc32cIntChecksum.finalizeValue((int) checksum); assertEquals(608512271L, checksum); } diff --git a/circe-checksum/src/test/java/com/scurrilous/circe/checksum/Java9IntHashTest.java b/circe-checksum/src/test/java/com/scurrilous/circe/checksum/Java9IntHashTest.java index 3fb57b4a1aa..69c54af8e60 100644 --- a/circe-checksum/src/test/java/com/scurrilous/circe/checksum/Java9IntHashTest.java +++ b/circe-checksum/src/test/java/com/scurrilous/circe/checksum/Java9IntHashTest.java @@ -65,8 +65,8 @@ public void calculateCheckSumUsingCompositeByteBuf() { int checksumRes1 = Crc32cIntChecksum.computeChecksum(bTotal); // Calculate: case-2. - int b1CheckSum = Crc32cIntChecksum.computeChecksum(b1); - int checksumRes2 = Crc32cIntChecksum.resumeChecksum(b1CheckSum, b2); + int b1CheckSum = Crc32cIntChecksum.resumeChecksum(Crc32cIntChecksum.initialValue(), b1); + int checksumRes2 = Crc32cIntChecksum.finalizeValue(Crc32cIntChecksum.resumeChecksum(b1CheckSum, b2)); // Verify: the results of both ways to calculate the checksum are same. Assert.assertEquals(checksumRes1, checksumRes2); @@ -90,8 +90,8 @@ public void calculateCheckSumUsingNoArrayNoMemoryAddrByteBuf() { int checksumRes1 = Crc32cIntChecksum.computeChecksum(bTotal); // Calculate: case-2. - int b1CheckSum = Crc32cIntChecksum.computeChecksum(b1); - int checksumRes2 = Crc32cIntChecksum.resumeChecksum(b1CheckSum, b2); + int b1CheckSum = Crc32cIntChecksum.resumeChecksum(Crc32cIntChecksum.initialValue(), b1); + int checksumRes2 = Crc32cIntChecksum.finalizeValue(Crc32cIntChecksum.resumeChecksum(b1CheckSum, b2)); // Verify: the results of both ways to calculate the checksum are same. Assert.assertEquals(checksumRes1, checksumRes2);