From 98755ac911c90491b65ff46484728203b93ede79 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Thu, 7 Mar 2019 08:49:11 -0600 Subject: Bug 545149 - performance hit on loading from cached state Change-Id: Ibc62835672f6f62bd2ee44cfdc110e9124b676b3 Signed-off-by: Thomas Watson --- .../org.eclipse.osgi.tests/META-INF/MANIFEST.MF | 2 +- bundles/org.eclipse.osgi.tests/pom.xml | 2 +- .../tests/bundles/BundleInstallUpdateTests.java | 16 ++++++++-- .../tests/bundles/ClassLoadingBundleTests.java | 4 +++ .../osgi/tests/bundles/SystemBundleTests.java | 3 +- bundles/org.eclipse.osgi/META-INF/MANIFEST.MF | 2 +- .../src/org/eclipse/osgi/storage/Storage.java | 35 ++++++++++++++++++++-- bundles/org.eclipse.osgi/pom.xml | 2 +- 8 files changed, 55 insertions(+), 11 deletions(-) diff --git a/bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF b/bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF index 3ae62e2ec..58c45dd4e 100644 --- a/bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: Core OSGi Tests Bundle-SymbolicName: org.eclipse.osgi.tests;singleton:=true -Bundle-Version: 3.13.400.qualifier +Bundle-Version: 3.13.500.qualifier Bundle-Vendor: Eclipse.org Bundle-Localization: plugin Require-Bundle: diff --git a/bundles/org.eclipse.osgi.tests/pom.xml b/bundles/org.eclipse.osgi.tests/pom.xml index aac6bf942..b923070f8 100644 --- a/bundles/org.eclipse.osgi.tests/pom.xml +++ b/bundles/org.eclipse.osgi.tests/pom.xml @@ -19,7 +19,7 @@ org.eclipse.osgi org.eclipse.osgi.tests - 3.13.400-SNAPSHOT + 3.13.500-SNAPSHOT eclipse-test-plugin diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleInstallUpdateTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleInstallUpdateTests.java index 783a59c7b..eccc62878 100644 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleInstallUpdateTests.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleInstallUpdateTests.java @@ -18,8 +18,9 @@ import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.net.MalformedURLException; -import java.net.URISyntaxException; import java.net.URL; import java.net.URLEncoder; import java.util.Collection; @@ -37,6 +38,7 @@ import org.osgi.framework.Constants; import org.osgi.framework.InvalidSyntaxException; import org.osgi.framework.ServiceRegistration; import org.osgi.framework.hooks.bundle.CollisionHook; +import org.osgi.framework.wiring.BundleWiring; public class BundleInstallUpdateTests extends AbstractBundleTests { public static Test suite() { @@ -435,20 +437,23 @@ public class BundleInstallUpdateTests extends AbstractBundleTests { is.close(); } - public void testEscapeZipRoot() throws IOException, BundleException, InvalidSyntaxException, URISyntaxException { + public void testEscapeZipRoot() throws IOException, BundleException, InvalidSyntaxException, NoSuchMethodException, SecurityException, IllegalAccessException, IllegalArgumentException, InvocationTargetException { String entry1 = "../../escapedZipRoot1.txt"; String entry2 = "dir1/../../../escapedZipRoot2.txt"; String cp1 = "../../cp.jar"; + String nativeCode = "../../lib/nativeCode"; File bundlesDirectory = OSGiTestsActivator.getContext().getDataFile(getName()); bundlesDirectory.mkdirs(); Map headers = new HashMap(); headers.put(Constants.BUNDLE_MANIFESTVERSION, "2"); headers.put(Constants.BUNDLE_SYMBOLICNAME, getName()); - headers.put(Constants.BUNDLE_CLASSPATH, "., ../../cp.jar"); + headers.put(Constants.BUNDLE_CLASSPATH, "., " + cp1); + headers.put(Constants.BUNDLE_NATIVECODE, nativeCode); Map entries = new HashMap<>(); entries.put(entry1, "value"); entries.put(entry2, "value"); entries.put(cp1, "value"); + entries.put(nativeCode, "value"); File testBundleJarFile = SystemBundleTests.createBundle(bundlesDirectory, getName(), headers, entries); Bundle testBundle = getContext().installBundle(getName(), new FileInputStream(testBundleJarFile)); @@ -458,6 +463,11 @@ public class BundleInstallUpdateTests extends AbstractBundleTests { } catch (ClassNotFoundException e) { // expected } + ClassLoader cl = testBundle.adapt(BundleWiring.class).getClassLoader(); + Method findLibrary = ClassLoader.class.getDeclaredMethod("findLibrary", String.class); + findLibrary.setAccessible(true); + assertNull("Found library.", findLibrary.invoke(cl, "nativeCode")); + URLConverter bundleURLConverter = getContext().getService(getContext().getServiceReferences(URLConverter.class, "(protocol=bundleentry)").iterator().next()); URL dir1 = bundleURLConverter.toFileURL(testBundle.getEntry("dir1/")); diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java index f53778a2b..0246e2619 100644 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java @@ -1506,6 +1506,10 @@ public class ClassLoadingBundleTests extends AbstractBundleTests { } public void testUninstallInUse01() throws BundleException { + if (getContext().getServiceReference("org.eclipse.equinox.region.RegionDigraph") != null) { + System.out.println("Cannot test uninstall in use with RegionDigraph service"); + return; + } Bundle exporter1 = installer.installBundle("exporter.importer1"); //$NON-NLS-1$ BundleRevision iExporter1 = exporter1.adapt(BundleRevision.class); Bundle exporter2 = installer.installBundle("exporter.importer2"); //$NON-NLS-1$ diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java index 67607a729..8a4232659 100755 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java @@ -3312,7 +3312,8 @@ public class SystemBundleTests extends AbstractBundleTests { URI uri = new URI(frameworkLocation); File f = new File(uri); if (!f.isFile()) { - Assert.fail("Cannot test when framework location is a directory: " + f.getAbsolutePath()); + System.out.println("Cannot test when framework location is a directory: " + f.getAbsolutePath()); + return; } File testDestination = OSGiTestsActivator.getContext().getDataFile(getName() + ".framework.jar"); BaseSecurityTest.copy(new FileInputStream(f), testDestination); diff --git a/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF b/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF index 25ab3347b..dc7a293e5 100644 --- a/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF @@ -101,7 +101,7 @@ Bundle-Activator: org.eclipse.osgi.internal.framework.SystemBundleActivator Bundle-Description: %systemBundle Bundle-Copyright: %copyright Bundle-Vendor: %eclipse.org -Bundle-Version: 3.13.300.qualifier +Bundle-Version: 3.13.400.qualifier Bundle-Localization: systembundle Bundle-DocUrl: http://www.eclipse.org Eclipse-ExtensibleAPI: true diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java index 6dfe504b7..25c70ac8f 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java @@ -955,10 +955,35 @@ public class Storage { return bundleID + "/" + generationID + "/" + BUNDLE_FILE_NAME; //$NON-NLS-1$ //$NON-NLS-2$ } + /** + * Gets a file from storage and conditionally checks the parent storage area + * if the file does not exist in the child configuration. + * Note, this method does not check for escaping of paths from the root storage area. + * @param path the path relative to the root of the storage area + * @param checkParent if true then check the parent storage (if any) when the file + * does not exist in the child storage area + * @return the file being requested. A {@code null} value is never returned. The file + * returned may not exist. + * @throws StorageException if there was an issue getting the file + */ public File getFile(String path, boolean checkParent) throws StorageException { return getFile(null, path, checkParent); } + /** + * Same as {@link #getFile(String, boolean)} except takes a base parameter which is + * appended to the root storage area before looking for the path. If base is not + * null then additional checks are done to make sure the path does not escape out + * of the base path. + * @param base the additional base path to append to the root storage area. May be + * {@code null}, in which case no check is done for escaping out of the base path. + * @param path the path relative to the root + base storage area. + * @param checkParent if true then check the parent storage (if any) when the file + * does not exist in the child storage area + * @return the file being requested. A {@code null} value is never returned. The file + * returned may not exist. + * @throws StorageException if there was an issue getting the file + */ public File getFile(String base, String path, boolean checkParent) throws StorageException { // first check the child location File childPath = getFile(childRoot, base, path); @@ -978,12 +1003,16 @@ public class Storage { } private static File getFile(File root, String base, String path) { - if (base != null) { - // if base is not null then move root to include the base - root = new File(root, base); + if (base == null) { + // return quick; no need to check for path traversal + return new File(root, path); } + + // if base is not null then move root to include the base + root = new File(root, base); File result = new File(root, path); + // do the extra check to make sure the path did not escape the root path try { String resultCanonical = result.getCanonicalPath(); String rootCanonical = root.getCanonicalPath(); diff --git a/bundles/org.eclipse.osgi/pom.xml b/bundles/org.eclipse.osgi/pom.xml index 2d63f2897..92a400413 100644 --- a/bundles/org.eclipse.osgi/pom.xml +++ b/bundles/org.eclipse.osgi/pom.xml @@ -19,7 +19,7 @@ org.eclipse.osgi org.eclipse.osgi - 3.13.300-SNAPSHOT + 3.13.400-SNAPSHOT eclipse-plugin -- cgit v1.2.3