From f1a9c2ebe02870622b8215cb6bc5f28542228b49 Mon Sep 17 00:00:00 2001 From: Bert Massop Date: Sun, 17 Nov 2024 22:42:42 +0100 Subject: [PATCH 1/2] Don't attempt to "intern" byte[] and other mutable data While beneficial for memory usage, reusing mutable objects may lead to all kinds of data consistency issues. It's just not worth saving the few bytes. --- src/freenet/keys/ClientCHK.java | 29 +++++++------------------- src/freenet/keys/ClientSSK.java | 9 ++++---- src/freenet/keys/FreenetURI.java | 35 +++++++------------------------- 3 files changed, 19 insertions(+), 54 deletions(-) diff --git a/src/freenet/keys/ClientCHK.java b/src/freenet/keys/ClientCHK.java index a70a34f8f5b..ceefc79f03d 100644 --- a/src/freenet/keys/ClientCHK.java +++ b/src/freenet/keys/ClientCHK.java @@ -163,9 +163,7 @@ public void writeRawBinaryKey(DataOutputStream dos) throws IOException { dos.write(routingKey); dos.write(cryptoKey); } - - static byte[] lastExtra; - + public byte[] getExtra() { return getExtra(cryptoAlgorithm, compressionAlgorithm, controlDocument); } @@ -177,32 +175,19 @@ public static byte[] getExtra(byte cryptoAlgorithm, short compressionAlgorithm, extra[2] = (byte) (controlDocument ? 2 : 0); extra[3] = (byte) (compressionAlgorithm >> 8); extra[4] = (byte) compressionAlgorithm; - byte[] last = lastExtra; - // No synchronization required IMHO - if(Arrays.equals(last, extra)) return last; - assert(extra.length == EXTRA_LENGTH); - lastExtra = extra; return extra; } public static byte getCryptoAlgorithmFromExtra(byte[] extra) { return extra[1]; } - - static HashSet standardExtras = new HashSet(); - static { - for(byte cryptoAlgorithm = Key.ALGO_AES_PCFB_256_SHA256; cryptoAlgorithm <= Key.ALGO_AES_CTR_256_SHA256; cryptoAlgorithm++) { - for(short compressionAlgorithm = -1; compressionAlgorithm <= (short)(COMPRESSOR_TYPE.countCompressors()); compressionAlgorithm++) { - standardExtras.add(new ByteArrayWrapper(getExtra(cryptoAlgorithm, compressionAlgorithm, true))); - standardExtras.add(new ByteArrayWrapper(getExtra(cryptoAlgorithm, compressionAlgorithm, false))); - } - } - } - + + /** + * @return the provided argument + * @deprecated mutable data cannot safely be interned + */ + @Deprecated public static byte[] internExtra(byte[] extra) { - for(ByteArrayWrapper baw : standardExtras) { - if(Arrays.equals(baw.get(), extra)) return baw.get(); - } return extra; } diff --git a/src/freenet/keys/ClientSSK.java b/src/freenet/keys/ClientSSK.java index 9387b2a53d5..811c4e0a775 100644 --- a/src/freenet/keys/ClientSSK.java +++ b/src/freenet/keys/ClientSSK.java @@ -143,11 +143,12 @@ protected static byte[] getExtraBytes(byte cryptoAlgorithm) { extra[4] = (byte) KeyBlock.HASH_SHA256; return extra; } - - static final byte[] STANDARD_EXTRA = getExtraBytes(Key.ALGO_AES_PCFB_256_SHA256); - + + /** + * @return the provided argument + * @deprecated mutable data cannot safely be interned + */ public static byte[] internExtra(byte[] buf) { - if(Arrays.equals(buf, STANDARD_EXTRA)) return STANDARD_EXTRA; return buf; } diff --git a/src/freenet/keys/FreenetURI.java b/src/freenet/keys/FreenetURI.java index 8d6cc1a481c..f42ecdd4ba3 100644 --- a/src/freenet/keys/FreenetURI.java +++ b/src/freenet/keys/FreenetURI.java @@ -223,35 +223,14 @@ public FreenetURI(FreenetURI uri) { this.suggestedEdition = uri.suggestedEdition; if(logDEBUG) Logger.debug(this, "Copied: "+toString()+" from "+uri.toString(), new Exception("debug")); } - - boolean noCacheURI = false; - - /** Optimize for memory. */ + + /** + * @return this FreenetURI instance + * @deprecated mutable data cannot safely be interned + */ + @Deprecated public FreenetURI intern() { - boolean changedAnything = false; - byte[] x = extra; - if(keyType.equals("CHK")) - x = ClientCHK.internExtra(x); - else - x = ClientSSK.internExtra(x); - if(x != extra) changedAnything = true; - String[] newMetaStr = null; - if(metaStr != null) { - newMetaStr = new String[metaStr.length]; - for(int i=0;i Date: Sun, 17 Nov 2024 22:45:49 +0100 Subject: [PATCH 2/2] Make ByteArrayWrapper safe for use as key in Map The underlying byte[] is mutable and could accidentally be changed after instantiation of the wrapper object. To ensure that the hashCode or the (natural or fast) order of the object does not change, the data must be copied defensively. --- src/freenet/support/ByteArrayWrapper.java | 42 ++++++++++------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/freenet/support/ByteArrayWrapper.java b/src/freenet/support/ByteArrayWrapper.java index 23224354b1c..88a54c48b93 100644 --- a/src/freenet/support/ByteArrayWrapper.java +++ b/src/freenet/support/ByteArrayWrapper.java @@ -12,31 +12,24 @@ */ public class ByteArrayWrapper implements Comparable { - private final byte[] buf; - private int hashCode; + private final byte[] data; + private final int hashCode; - public static final Comparator FAST_COMPARATOR = new Comparator() { - - @Override - public int compare(ByteArrayWrapper o1, ByteArrayWrapper o2) { - if(o1.hashCode > o2.hashCode) return 1; - if(o1.hashCode < o2.hashCode) return -1; - return o1.compareTo(o2); - } - - }; + public static final Comparator FAST_COMPARATOR = Comparator + .comparingInt(ByteArrayWrapper::hashCode) + .thenComparing(Comparator.naturalOrder()); public ByteArrayWrapper(byte[] data) { - buf = data; - hashCode = Fields.hashCode(buf); + this.data = Arrays.copyOf(data, data.length); + this.hashCode = Arrays.hashCode(this.data); } @Override - public boolean equals(Object o) { - if(o instanceof ByteArrayWrapper) { - ByteArrayWrapper b = (ByteArrayWrapper) o; - if(b.buf == buf) return true; - return Arrays.equals(b.buf, buf); + public boolean equals(Object other) { + if (this == other) return true; + if (other instanceof ByteArrayWrapper) { + ByteArrayWrapper b = (ByteArrayWrapper) other; + return this.hashCode == b.hashCode && Arrays.equals(this.data, b.data); } return false; } @@ -46,14 +39,15 @@ public int hashCode() { return hashCode; } - /** DO NOT MODIFY THE RETURNED DATA! */ public byte[] get() { - return buf; + return Arrays.copyOf(data, data.length); } @Override - public int compareTo(ByteArrayWrapper arg) { - if(this == arg) return 0; - return Fields.compareBytes(buf, arg.buf); + public int compareTo(ByteArrayWrapper other) { + if (this == other) { + return 0; + } + return Fields.compareBytes(this.data, other.data); } }