From 8debc9d0c2ee7c507d5ed7a4b738ec083151fb6d Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Thu, 10 Sep 2020 11:12:59 +0200 Subject: [PATCH] LUCENE-9517: Don't subclass Deflater and instead create a patch for setDictionary() using a functional interface (#1850) --- .../forbidden-apis/defaults.all.txt | 4 ++ .../lucene87/BugfixDeflater_JDK8252739.java | 46 +++++++++---------- .../DeflateWithPresetDictCompressionMode.java | 6 ++- 3 files changed, 30 insertions(+), 26 deletions(-) diff --git a/gradle/validation/forbidden-apis/defaults.all.txt b/gradle/validation/forbidden-apis/defaults.all.txt index 6ff080bc7df..6c6e0296cc5 100644 --- a/gradle/validation/forbidden-apis/defaults.all.txt +++ b/gradle/validation/forbidden-apis/defaults.all.txt @@ -62,3 +62,7 @@ java.lang.Double#(java.lang.String) @defaultMessage Java deserialization is unsafe when the data is untrusted. The java developer is powerless: no checks or casts help, exploitation can happen in places such as clinit or finalize! java.io.ObjectInputStream java.io.ObjectOutputStream + +@defaultMessage Don't set a dictionary on a Deflater using a method that takes an offset or ByteBuffer (JDK-8252739) +java.util.zip.Deflater#setDictionary(byte[],int,int) +java.util.zip.Deflater#setDictionary(java.nio.ByteBuffer) diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene87/BugfixDeflater_JDK8252739.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene87/BugfixDeflater_JDK8252739.java index ae5b9019f8e..582715efd09 100644 --- a/lucene/core/src/java/org/apache/lucene/codecs/lucene87/BugfixDeflater_JDK8252739.java +++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene87/BugfixDeflater_JDK8252739.java @@ -21,49 +21,47 @@ import java.util.zip.DataFormatException; import java.util.zip.Deflater; import java.util.zip.Inflater; +import org.apache.lucene.util.SuppressForbidden; + /** * This class is a workaround for JDK bug * JDK-8252739. */ -final class BugfixDeflater_JDK8252739 extends Deflater { +@FunctionalInterface +interface BugfixDeflater_JDK8252739 { public static final boolean IS_BUGGY_JDK = detectBuggyJDK(); /** - * Creates a {@link Deflater} instance, which works around JDK-8252739. + * Creates a bugfix for {@link Deflater} instances, which works around JDK-8252739. *

- * Use this whenever you intend to call {@link #setDictionary(byte[], int, int)} or - * {@link #setDictionary(java.nio.ByteBuffer)} on a {@code Deflater}. + * Use this whenever you intend to call {@link Deflater#setDictionary(byte[], int, int)} + * on a {@code Deflater}. * */ - public static Deflater createDeflaterInstance(int level, boolean nowrap, int dictLength) { + @SuppressForbidden(reason = "Works around bug, so it must call forbidden method") + public static BugfixDeflater_JDK8252739 createBugfix(Deflater deflater, int dictLength) { if (dictLength < 0) { throw new IllegalArgumentException("dictLength must be >= 0"); } if (IS_BUGGY_JDK) { - return new BugfixDeflater_JDK8252739(level, nowrap, dictLength); + final byte[] dictBytesScratch = new byte[dictLength]; + return (dictBytes, off, len) -> { + if (off > 0) { + System.arraycopy(dictBytes, off, dictBytesScratch, 0, len); + deflater.setDictionary(dictBytesScratch, 0, len); + } else { + deflater.setDictionary(dictBytes, off, len); + } + }; } else { - return new Deflater(level, nowrap); + return deflater::setDictionary; } } + /** Call this method as a workaround */ + void setDictionary(byte[] dictBytes, int off, int len); - private final byte[] dictBytesScratch; - - private BugfixDeflater_JDK8252739(int level, boolean nowrap, int dictLength) { - super(level, nowrap); - this.dictBytesScratch = new byte[dictLength]; - } - - @Override - public void setDictionary(byte[] dictBytes, int off, int len) { - if (off > 0) { - System.arraycopy(dictBytes, off, dictBytesScratch, 0, len); - super.setDictionary(dictBytesScratch, 0, len); - } else { - super.setDictionary(dictBytes, off, len); - } - } - + @SuppressForbidden(reason = "Detector for the bug, so it must call buggy method") private static boolean detectBuggyJDK() { final byte[] testData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 }; final byte[] compressed = new byte[32]; // way enough space diff --git a/lucene/core/src/java/org/apache/lucene/codecs/lucene87/DeflateWithPresetDictCompressionMode.java b/lucene/core/src/java/org/apache/lucene/codecs/lucene87/DeflateWithPresetDictCompressionMode.java index 91c1480c632..42697409887 100644 --- a/lucene/core/src/java/org/apache/lucene/codecs/lucene87/DeflateWithPresetDictCompressionMode.java +++ b/lucene/core/src/java/org/apache/lucene/codecs/lucene87/DeflateWithPresetDictCompressionMode.java @@ -157,11 +157,13 @@ public final class DeflateWithPresetDictCompressionMode extends CompressionMode private final int dictLength, blockLength; final Deflater compressor; + final BugfixDeflater_JDK8252739 deflaterBugfix; byte[] compressed; boolean closed; DeflateWithPresetDictCompressor(int level, int dictLength, int blockLength) { - compressor = BugfixDeflater_JDK8252739.createDeflaterInstance(level, true, dictLength); + compressor = new Deflater(level, true); + deflaterBugfix = BugfixDeflater_JDK8252739.createBugfix(compressor, dictLength); compressed = new byte[64]; this.dictLength = dictLength; this.blockLength = blockLength; @@ -208,7 +210,7 @@ public final class DeflateWithPresetDictCompressionMode extends CompressionMode // And then sub blocks for (int start = off + dictLength; start < end; start += blockLength) { compressor.reset(); - compressor.setDictionary(bytes, off, dictLength); + deflaterBugfix.setDictionary(bytes, off, dictLength); doCompress(bytes, start, Math.min(blockLength, off + len - start), out); } }