Bug 576643 - Clean up and unify Bundle resource classes

Change-Id: I25cd5a9b67cf601bdaf09f69ec33420bf15264f5
Signed-off-by: Hannes Wellmann <wellmann.hannes1@gmx.net>
Reviewed-on: https://git.eclipse.org/r/c/equinox/rt.equinox.framework/+/186524
Tested-by: Equinox Bot <equinox-bot@eclipse.org>
Reviewed-by: Thomas Watson <tjwatson@us.ibm.com>

4 files changed, 105 insertions, 110 deletions

diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java
index e23951d91..ff88bd15e 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/BundleInfo.java
@@ -10,6 +10,7 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Hannes Wellmann - Bug 576643: Clean up and unify Bundle resource classes
  *******************************************************************************/
 package org.eclipse.osgi.storage;
 
@@ -18,7 +19,6 @@ import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
-import java.net.MalformedURLException;
 import java.net.URL;
 import java.security.ProtectionDomain;
 import java.util.Collection;
@@ -31,7 +31,9 @@ import java.util.Map;
 import java.util.ResourceBundle;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+import java.util.jar.JarFile;
 import org.eclipse.osgi.container.Module;
+import org.eclipse.osgi.container.ModuleContainer;
 import org.eclipse.osgi.container.ModuleContainerAdaptor.ModuleEvent;
 import org.eclipse.osgi.container.ModuleRevision;
 import org.eclipse.osgi.container.ModuleRevisionBuilder;
@@ -60,7 +62,7 @@ import org.osgi.framework.Constants;
 import org.osgi.framework.connect.ConnectModule;
 
 public final class BundleInfo {
-	public static final String OSGI_BUNDLE_MANIFEST = "META-INF/MANIFEST.MF"; //$NON-NLS-1$
+	public static final String OSGI_BUNDLE_MANIFEST = JarFile.MANIFEST_NAME;
 	public static final String MULTI_RELEASE_HEADER = "Multi-Release"; //$NON-NLS-1$
 	public static final String MULTI_RELEASE_VERSIONS = "META-INF/versions/"; //$NON-NLS-1$
 	public static final Collection<String> MULTI_RELEASE_FILTER_PREFIXES = Collections.singleton("META-INF/"); //$NON-NLS-1$
@@ -153,7 +155,7 @@ public final class BundleInfo {
 							// For MRJARs only replace Import-Package and Require-Capability if the versioned values are non-null
 							if (Boolean.parseBoolean(merged.get(MULTI_RELEASE_HEADER))) {
 								for (int i = getStorage().getRuntimeVersion().getMajor(); i > 8; i--) {
-									String versionManifest = "META-INF/versions/" + i + "/OSGI-INF/MANIFEST.MF"; //$NON-NLS-1$ //$NON-NLS-2$
+									String versionManifest = MULTI_RELEASE_VERSIONS + i + "/OSGI-INF/MANIFEST.MF"; //$NON-NLS-1$
 									BundleEntry versionEntry = getBundleFile().getEntry(versionManifest);
 									if (versionEntry != null) {
 										Map<String, String> versioned = ManifestElement.parseBundleManifest(versionEntry.getInputStream(), new CaseInsensitiveDictionaryMap<>());
@@ -171,10 +173,9 @@ public final class BundleInfo {
 								}
 							}
 							rawHeaders = Collections.unmodifiableMap(merged);
+						} catch (RuntimeException e) {
+							throw e;
 						} catch (Exception e) {
-							if (e instanceof RuntimeException) {
-								throw (RuntimeException) e;
-							}
 							throw new RuntimeException("Error occurred getting the bundle manifest.", e); //$NON-NLS-1$
 						}
 					}
@@ -263,7 +264,7 @@ public final class BundleInfo {
 			}
 			synchronized (this.genMonitor) {
 				this.content = content;
-				this.isDirectory = content == null ? false : Storage.secureAction.isDirectory(content);
+				this.isDirectory = content != null && Storage.secureAction.isDirectory(content);
 				this.contentType = contentType;
 				setLastModified(content);
 			}
@@ -276,7 +277,7 @@ public final class BundleInfo {
 				return;
 			}
 			if (isDirectory)
-				content = new File(content, "META-INF/MANIFEST.MF"); //$NON-NLS-1$
+				content = new File(content, OSGI_BUNDLE_MANIFEST); // $NON-NLS-1$
 			lastModified = Storage.secureAction.lastModified(content);
 		}
 
@@ -421,15 +422,14 @@ public final class BundleInfo {
 
 		public URL getEntry(String path) {
 			BundleEntry entry = getBundleFile().getEntry(path);
-			if (entry == null)
-				return null;
-			path = BundleFile.fixTrailingSlash(path, entry);
-			try {
-				//use the constant string for the protocol to prevent duplication
-				return Storage.secureAction.getURL(BundleResourceHandler.OSGI_ENTRY_URL_PROTOCOL, Long.toString(getBundleId()) + BundleResourceHandler.BID_FWKID_SEPARATOR + Integer.toString(getStorage().getModuleContainer().hashCode()), 0, path, new Handler(getStorage().getModuleContainer(), entry));
-			} catch (MalformedURLException e) {
+			if (entry == null) {
 				return null;
 			}
+			// use the constant string for the protocol to prevent duplication
+			String protocol = BundleResourceHandler.OSGI_ENTRY_URL_PROTOCOL;
+			ModuleContainer container = getStorage().getModuleContainer();
+			Handler handler = new Handler(container, entry);
+			return BundleFile.createURL(protocol, getBundleId(), container, entry, 0, path, handler);
 		}
 
 		public String findLibrary(String libname) {
@@ -500,8 +500,7 @@ public final class BundleInfo {
 			if (!lockedID) {
 				throw new BundleException("Failed to obtain id locks for generation.", BundleException.STATECHANGE_ERROR, new ThreadInfoReport(generationLocks.getLockInfo(nextGenerationId))); //$NON-NLS-1$
 			}
-			Generation newGeneration = new Generation(nextGenerationId++);
-			return newGeneration;
+			return new Generation(nextGenerationId++);
 		}
 	}
 
@@ -516,8 +515,7 @@ public final class BundleInfo {
 
 	Generation restoreGeneration(long generationId, File content, boolean isDirectory, Type contentType, boolean hasPackageInfo, Map<String, String> cached, long lastModified, boolean isMRJar) {
 		synchronized (this.infoMonitor) {
-			Generation restoredGeneration = new Generation(generationId, content, isDirectory, contentType, hasPackageInfo, cached, lastModified, isMRJar);
-			return restoredGeneration;
+			return new Generation(generationId, content, isDirectory, contentType, hasPackageInfo, cached, lastModified, isMRJar);
 		}
 	}
 
@@ -568,34 +566,27 @@ public final class BundleInfo {
 		if (manifest == null) {
 			return false;
 		}
-		BufferedReader br = null;
-		try {
-			br = new BufferedReader(new InputStreamReader(manifest.getInputStream()));
+		try (BufferedReader br = new BufferedReader(new InputStreamReader(manifest.getInputStream()))) {
 			String line;
 			while ((line = br.readLine()) != null) {
 				if (line.length() < 20)
 					continue;
 				switch (line.charAt(0)) {
-					case 'S' :
-						if (line.charAt(1) == 'p')
-							if (line.startsWith("Specification-Title: ") || line.startsWith("Specification-Version: ") || line.startsWith("Specification-Vendor: ")) //$NON-NLS-1$ //$NON-NLS-2$//$NON-NLS-3$
-								return true;
-						break;
-					case 'I' :
-						if (line.startsWith("Implementation-Title: ") || line.startsWith("Implementation-Version: ") || line.startsWith("Implementation-Vendor: ")) //$NON-NLS-1$ //$NON-NLS-2$//$NON-NLS-3$
-							return true;
-						break;
+				case 'S':
+					if (line.charAt(1) == 'p' && (line.startsWith("Specification-Title: ") //$NON-NLS-1$
+							|| line.startsWith("Specification-Version: ") //$NON-NLS-1$
+							|| line.startsWith("Specification-Vendor: "))) //$NON-NLS-1$
+						return true;
+					break;
+				case 'I':
+					if (line.startsWith("Implementation-Title: ") || line.startsWith("Implementation-Version: ") //$NON-NLS-1$ //$NON-NLS-2$
+							|| line.startsWith("Implementation-Vendor: ")) //$NON-NLS-1$
+						return true;
+					break;
 				}
 			}
 		} catch (IOException ioe) {
 			// do nothing
-		} finally {
-			if (br != null)
-				try {
-					br.close();
-				} catch (IOException e) {
-					// do nothing
-				}
 		}
 		return false;
 	}
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/BundleFile.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/BundleFile.java
index 7a964a21d..598782c3f 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/BundleFile.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/BundleFile.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2004, 2015 IBM Corporation and others.
+ * Copyright (c) 2004, 2021 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -10,6 +10,7 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Hannes Wellmann - Bug 576643: Clean up and unify Bundle resource classes
  *******************************************************************************/
 
 package org.eclipse.osgi.storage.bundlefile;
@@ -18,9 +19,11 @@ import java.io.File;
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.net.URLStreamHandler;
 import java.security.AccessController;
 import java.util.Enumeration;
 import org.eclipse.osgi.container.Module;
+import org.eclipse.osgi.container.ModuleContainer;
 import org.eclipse.osgi.framework.util.SecureAction;
 import org.eclipse.osgi.storage.url.BundleResourceHandler;
 import org.eclipse.osgi.storage.url.bundleresource.Handler;
@@ -46,7 +49,7 @@ abstract public class BundleFile {
 	 * @param basefile The File object where this BundleFile is
 	 * persistently stored.
 	 */
-	public BundleFile(File basefile) {
+	protected BundleFile(File basefile) {
 		this.basefile = basefile;
 	}
 
@@ -151,14 +154,12 @@ abstract public class BundleFile {
 	 * @return a URL to access the contents of the specified entry
 	 */
 	protected URL createResourceURL(BundleEntry bundleEntry, Module hostModule, int index, String path) {
-		long hostBundleID = hostModule.getId();
-		path = fixTrailingSlash(path, bundleEntry);
-		try {
-			//use the constant string for the protocol to prevent duplication
-			return secureAction.getURL(BundleResourceHandler.OSGI_RESOURCE_URL_PROTOCOL, Long.toString(hostBundleID) + BundleResourceHandler.BID_FWKID_SEPARATOR + Integer.toString(hostModule.getContainer().hashCode()), index, path, new Handler(hostModule.getContainer(), bundleEntry));
-		} catch (MalformedURLException e) {
-			return null;
-		}
+		// use the constant string for the protocol to prevent duplication
+		String protocol = BundleResourceHandler.OSGI_RESOURCE_URL_PROTOCOL;
+		long bundleId = hostModule.getId();
+		ModuleContainer container = hostModule.getContainer();
+		Handler handler = new Handler(container, bundleEntry);
+		return createURL(protocol, bundleId, container, bundleEntry, index, path, handler);
 	}
 
 	/**
@@ -182,6 +183,17 @@ abstract public class BundleFile {
 		return String.valueOf(basefile);
 	}
 
+	public static URL createURL(String protocol, long bundleId, ModuleContainer container, BundleEntry entry, int index,
+			String path, URLStreamHandler handler) {
+		path = fixTrailingSlash(path, entry);
+		try {
+			String host = BundleResourceHandler.createURLHostForBundleID(container, bundleId);
+			return secureAction.getURL(protocol, host, index, path, handler);
+		} catch (MalformedURLException e) {
+			return null;
+		}
+	}
+
 	public static String fixTrailingSlash(String path, BundleEntry entry) {
 		if (path.length() == 0)
 			return "/"; //$NON-NLS-1$
@@ -200,5 +212,4 @@ abstract public class BundleFile {
 		}
 		return path;
 	}
-
 }
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/url/BundleResourceHandler.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/url/BundleResourceHandler.java
index d3770493f..8ffd0f501 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/url/BundleResourceHandler.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/url/BundleResourceHandler.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2004, 2013 IBM Corporation and others.
+ * Copyright (c) 2004, 2021 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -10,12 +10,18 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Hannes Wellmann - Bug 576643: Clean up and unify Bundle resource classes
  *******************************************************************************/
 
 package org.eclipse.osgi.storage.url;
 
 import java.io.IOException;
-import java.net.*;
+import java.net.InetAddress;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLConnection;
+import java.net.URLStreamHandler;
+import java.util.Objects;
 import org.eclipse.osgi.container.Module;
 import org.eclipse.osgi.container.ModuleContainer;
 import org.eclipse.osgi.internal.messages.Msg;
@@ -40,7 +46,7 @@ public abstract class BundleResourceHandler extends URLStreamHandler {
 	protected final ModuleContainer container;
 	protected BundleEntry bundleEntry;
 
-	public BundleResourceHandler(ModuleContainer container, BundleEntry bundleEntry) {
+	protected BundleResourceHandler(ModuleContainer container, BundleEntry bundleEntry) {
 		this.container = container;
 		this.bundleEntry = bundleEntry;
 	}
@@ -60,7 +66,7 @@ public abstract class BundleResourceHandler extends URLStreamHandler {
 		if (start < end)
 			spec = str.substring(start, end);
 		end -= start;
-		//Default is to use path and bundleId from context
+		// Default is to use path and bundleId from context
 		String path = url.getPath();
 		String host = url.getHost();
 		int resIndex = url.getPort();
@@ -99,7 +105,7 @@ public abstract class BundleResourceHandler extends URLStreamHandler {
 		}
 		if (path == null)
 			path = ""; //$NON-NLS-1$
-		//modify path if there's any relative references
+		// modify path if there's any relative references
 		// see RFC2396 Section 5.2
 		// Note: For ".." references above the root the approach taken is removing them from the resolved path
 		if (path.endsWith("/.") || path.endsWith("/..")) //$NON-NLS-1$ //$NON-NLS-2$
@@ -119,15 +125,15 @@ public abstract class BundleResourceHandler extends URLStreamHandler {
 		// Check the permission of the caller to see if they
 		// are allowed access to the resource.
 		String authorized = SECURITY_UNCHECKED;
-		long bundleId = getBundleID(host);
+		long bundleId = parseBundleIDFromURLHost(host);
 		Module module = getModule(bundleId);
 		if (checkAuthorization(module))
 			authorized = SECURITY_CHECKED;
 		// Always force the use of the hash from the adaptor
-		host = Long.toString(bundleId) + BID_FWKID_SEPARATOR + Integer.toString(container.hashCode());
+		host = createURLHostForBundleID(container, bundleId);
 		// Setting the authority portion of the URL to SECURITY_ATHORIZED
 		// ensures that this URL was created by using this parseURL
-		// method.  The openConnection method will only open URLs
+		// method. The openConnection method will only open URLs
 		// that have the authority set to this.
 		setURL(url, url.getProtocol(), host, resIndex, authorized, null, path, null, url.getRef());
 	}
@@ -148,30 +154,31 @@ public abstract class BundleResourceHandler extends URLStreamHandler {
 	 */
 	@Override
 	protected URLConnection openConnection(URL url) throws IOException {
-		if (bundleEntry != null) // if the bundleEntry is not null then return quick
-			return (new BundleURLConnection(url, bundleEntry));
-
+		if (bundleEntry != null) { // if the bundleEntry is not null then return quick
+			return new BundleURLConnection(url, bundleEntry);
+		}
 		String host = url.getHost();
 		if (host == null) {
 			throw new IOException(NLS.bind(Msg.URL_NO_BUNDLE_ID, url.toExternalForm()));
 		}
 		long bundleID;
 		try {
-			bundleID = getBundleID(host);
+			bundleID = parseBundleIDFromURLHost(host);
 		} catch (NumberFormatException nfe) {
 			throw (MalformedURLException) new MalformedURLException(NLS.bind(Msg.URL_INVALID_BUNDLE_ID, host)).initCause(nfe);
 		}
 		Module module = getModule(bundleID);
-		if (module == null)
+		if (module == null) {
 			throw new IOException(NLS.bind(Msg.URL_NO_BUNDLE_FOUND, url.toExternalForm()));
+		}
 		// check to make sure that this URL was created using the
-		// parseURL method.  This ensures the security check was done
+		// parseURL method. This ensures the security check was done
 		// at URL construction.
 		if (!url.getAuthority().equals(SECURITY_CHECKED)) {
 			// No admin security check was made better check now.
 			checkAuthorization(module);
 		}
-		return (new BundleURLConnection(url, findBundleEntry(url, module)));
+		return new BundleURLConnection(url, findBundleEntry(url, module));
 	}
 
 	/**
@@ -197,26 +204,25 @@ public abstract class BundleResourceHandler extends URLStreamHandler {
 		result.append("://"); //$NON-NLS-1$
 
 		String host = url.getHost();
-		if ((host != null) && (host.length() > 0))
+		if (host != null && host.length() > 0) {
 			result.append(host);
+		}
 		int index = url.getPort();
-		if (index > 0)
+		if (index > 0) {
 			result.append(':').append(index);
-
+		}
 		String path = url.getPath();
 		if (path != null) {
-			if ((path.length() > 0) && (path.charAt(0) != '/')) /* if name doesn't have a leading slash */
-			{
+			if (path.length() > 0 && path.charAt(0) != '/') { // if name doesn't have a leading slash
 				result.append("/"); //$NON-NLS-1$
 			}
-
 			result.append(path);
 		}
 		String ref = url.getRef();
-		if (ref != null && ref.length() > 0)
+		if (ref != null && ref.length() > 0) {
 			result.append('#').append(ref);
-
-		return (result.toString());
+		}
+		return result.toString();
 	}
 
 	@Override
@@ -252,11 +258,7 @@ public abstract class BundleResourceHandler extends URLStreamHandler {
 
 	@Override
 	protected boolean hostsEqual(URL url1, URL url2) {
-		String host1 = url1.getHost();
-		String host2 = url2.getHost();
-		if (host1 != null && host2 != null)
-			return host1.equalsIgnoreCase(host2);
-		return (host1 == null && host2 == null);
+		return equalsIgnoreCase(url1.getHost(), url2.getHost());
 	}
 
 	@Override
@@ -264,23 +266,10 @@ public abstract class BundleResourceHandler extends URLStreamHandler {
 		// do a hashcode test to allow each handler to check the adaptor first
 		if (url1.hashCode() != url2.hashCode())
 			return false;
-		String p1 = url1.getProtocol();
-		String p2 = url2.getProtocol();
-		if (!((p1 == p2) || (p1 != null && p1.equalsIgnoreCase(p2))))
-			return false;
-
-		if (!hostsEqual(url1, url2))
-			return false;
-
-		if (url1.getPort() != url2.getPort())
-			return false;
-
-		String path1 = url1.getPath();
-		String path2 = url2.getPath();
-		if (!((path1 == path2) || (path1 != null && path1.equals(path2))))
-			return false;
-
-		return true;
+		return equalsIgnoreCase(url1.getProtocol(), url2.getProtocol())
+				&& hostsEqual(url1, url2)
+				&& url1.getPort() == url2.getPort()
+				&& Objects.equals(url1.getPath(), url2.getPath());
 		// note that the authority is not checked here because it can be different for two
 		// URLs depending on how they were constructed.
 	}
@@ -289,15 +278,23 @@ public abstract class BundleResourceHandler extends URLStreamHandler {
 		SecurityManager sm = System.getSecurityManager();
 		if (sm == null)
 			return true;
-		Bundle bundle = module == null ? null : module.getBundle();
-		if (bundle == null)
+		Bundle moduleBundle = module == null ? null : module.getBundle();
+		if (moduleBundle == null)
 			return false;
-		sm.checkPermission(new AdminPermission(bundle, AdminPermission.RESOURCE));
+		sm.checkPermission(new AdminPermission(moduleBundle, AdminPermission.RESOURCE));
 		return true;
 	}
 
-	private long getBundleID(String host) {
-		int dotIndex = host.indexOf('.');
+	private static boolean equalsIgnoreCase(String s1, String s2) {
+		return s1 != null ? s1.equalsIgnoreCase(s2) : s2 == null;
+	}
+
+	public static String createURLHostForBundleID(ModuleContainer container, long bundleId) {
+		return bundleId + BID_FWKID_SEPARATOR + container.hashCode();
+	}
+
+	private static long parseBundleIDFromURLHost(String host) {
+		int dotIndex = host.indexOf(BID_FWKID_SEPARATOR);
 		return (dotIndex >= 0 && dotIndex < host.length() - 1) ? Long.parseLong(host.substring(0, dotIndex)) : Long.parseLong(host);
 	}
 }
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/url/BundleURLConnection.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/url/BundleURLConnection.java
index d6a68fa7e..9040649da 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/url/BundleURLConnection.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/url/BundleURLConnection.java
@@ -25,7 +25,6 @@ import org.eclipse.osgi.util.NLS;
 /**
  * URLConnection for BundleClassLoader resources.
  */
-
 public class BundleURLConnection extends URLConnection {
 	/** BundleEntry that the URL is associated. */
 	protected final BundleEntry bundleEntry;
@@ -77,7 +76,7 @@ public class BundleURLConnection extends URLConnection {
 					try {
 						connect();
 					} catch (IOException e) {
-						return (null);
+						return null;
 					}
 				}
 				try {
@@ -88,18 +87,17 @@ public class BundleURLConnection extends URLConnection {
 				}
 			}
 		}
-
-		return (contentType);
+		return contentType;
 	}
 
 	@Override
 	public boolean getDoInput() {
-		return (true);
+		return true;
 	}
 
 	@Override
 	public boolean getDoOutput() {
-		return (false);
+		return false;
 	}
 
 	@Override
@@ -107,8 +105,7 @@ public class BundleURLConnection extends URLConnection {
 		if (!connected) {
 			connect();
 		}
-
-		return (in);
+		return in;
 	}
 
 	@Override
@@ -118,8 +115,7 @@ public class BundleURLConnection extends URLConnection {
 		if (lastModified == -1) {
 			return (0);
 		}
-
-		return (lastModified);
+		return lastModified;
 	}
 
 	/**