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.
hyphanet · Nov 17, 2024 · f1a9c2e · f1a9c2e
1 parent e496dc1
commit f1a9c2e
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 54 deletions.
diff --git 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<ByteArrayWrapper> standardExtras = new HashSet<ByteArrayWrapper>();
-	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
@@ -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
@@ -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<metaStr.length;i++) {
-				newMetaStr[i] = metaStr[i].intern();
-				if(metaStr[i] != newMetaStr[i]) changedAnything = true;
-			}
-		}
-		String dn = docName == null ? null : docName.intern();
-		if(dn != docName) changedAnything = true;
-		if(!changedAnything) {
-			noCacheURI = true;
-			return this;
-		}
-		FreenetURI u = new FreenetURI(keyType, dn, newMetaStr, routingKey, cryptoKey, extra, suggestedEdition);
-		u.noCacheURI = true;
-		return u;
+		return this;
 	}
 
 	public FreenetURI(String keyType, String docName) {