Attachment 229500 Details for Bug 395274 – Patch proposal

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] Patch proposal

patch2.patch (text/plain), 7.54 KB, created by Violeta Georgieva

on 2013-04-09 08:40:12 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Violeta Georgieva

Created: 2013-04-09 08:40:12 EDT

Size: 7.54 KB

patch

obsolete

>diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleResourceTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleResourceTests.java
>index b97c1a0..617738e 100644
>--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleResourceTests.java
>+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleResourceTests.java
>@@ -15,6 +15,7 @@ import java.util.Enumeration;
> import junit.framework.Test;
> import junit.framework.TestSuite;
> import org.eclipse.core.tests.harness.CoreTest;
>+import org.eclipse.osgi.framework.internal.core.FrameworkProperties;
> import org.eclipse.osgi.tests.OSGiTestsActivator;
> import org.osgi.framework.*;
> 
>@@ -65,6 +66,19 @@ public class BundleResourceTests extends CoreTest {
> 		assertNotNull("Did not find resource!", paths);
> 	}
> 
>+	public void testBug395274() throws Exception {
>+		FrameworkProperties.setProperty("osgi.strictBundleEntryPath", "true");
>+		Bundle bundle = installer.installBundle("test"); //$NON-NLS-1$
>+		URL path = bundle.getEntry("META-INF./MANIFEST.MF");
>+		assertNull("found resource!", path);
>+		path = bundle.getEntry("META-INF/MANIFEST.MF");
>+		assertNotNull("Did not find resource!", path);
>+		path = bundle.getEntry("folder/file1.TXT");
>+		assertNull("found resource!", path);
>+		path = bundle.getEntry("folder/file1.txt");
>+		assertNotNull("Did not find resource!", path);
>+	}
>+
> 	public void testBug328795() throws BundleException {
> 		Bundle bundle = installer.installBundle("test"); //$NON-NLS-1$
> 		checkEntries(bundle, "notFound\\", 0); // this results in invalid syntax exception which is logged because of trailing escape
>@@ -97,6 +111,10 @@ public class BundleResourceTests extends CoreTest {
> 		checkEntries(bundle, "*(*", 2);
> 		checkEntries(bundle, "*\\)*", 2);
> 		checkEntries(bundle, "*\\(*", 2);
>+		checkEntries(bundle, "/./file1.txt", 1);
>+		checkEntries(bundle, "//file1.txt", 1);
>+		checkEntries(bundle, "/", 1);
>+		checkEntries(bundle, "/.", 1);
> 	}
> 
> 	public void testBug338081() throws BundleException {
>diff --git a/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/baseadaptor/bundlefile/DirBundleFile.java b/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/baseadaptor/bundlefile/DirBundleFile.java
>index 64c3f71..1f4ab16 100644
>--- a/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/baseadaptor/bundlefile/DirBundleFile.java
>+++ b/bundles/org.eclipse.osgi/defaultAdaptor/src/org/eclipse/osgi/baseadaptor/bundlefile/DirBundleFile.java
>@@ -15,6 +15,7 @@ import java.io.File;
> import java.io.IOException;
> import java.util.Enumeration;
> import java.util.NoSuchElementException;
>+import org.eclipse.osgi.framework.internal.core.FrameworkProperties;
> import org.eclipse.osgi.internal.baseadaptor.AdaptorMsg;
> import org.eclipse.osgi.util.NLS;
> 
>@@ -24,6 +25,16 @@ import org.eclipse.osgi.util.NLS;
>  */
> public class DirBundleFile extends BundleFile {
> 
>+	private static final String POINTER_SAME_DIRECTORY_1 = "/.";//$NON-NLS-1$
>+	private static final String POINTER_SAME_DIRECTORY_2 = "//";//$NON-NLS-1$
>+	private static final String POINTER_UPPER_DIRECTORY = "..";//$NON-NLS-1$
>+
>+	protected static final String PROPERTY_STRICT_BUNDLE_ENTRY_PATH = "osgi.strictBundleEntryPath";//$NON-NLS-1$
>+	protected static final String PROPERTY_STRICT_BUNDLE_ENTRY_PATH_DEFAULT_VALUE = "false";//$NON-NLS-1$
>+
>+	private final String baseFileCanonicalPath;
>+	private final boolean enabledStrictBundleEntryPath;
>+
> 	/**
> 	 * Constructs a DirBundleFile
> 	 * @param basefile the base file
>@@ -34,22 +45,52 @@ public class DirBundleFile extends BundleFile {
> 		if (!BundleFile.secureAction.exists(basefile) || !BundleFile.secureAction.isDirectory(basefile)) {
> 			throw new IOException(NLS.bind(AdaptorMsg.ADAPTOR_DIRECTORY_EXCEPTION, basefile));
> 		}
>+		this.baseFileCanonicalPath = BundleFile.secureAction.getCanonicalPath(basefile);
>+		this.enabledStrictBundleEntryPath = Boolean.parseBoolean(FrameworkProperties.getProperty(PROPERTY_STRICT_BUNDLE_ENTRY_PATH, PROPERTY_STRICT_BUNDLE_ENTRY_PATH_DEFAULT_VALUE));
> 	}
> 
> 	public File getFile(String path, boolean nativeCode) {
>-		boolean checkInBundle = path != null && path.indexOf("..") >= 0; //$NON-NLS-1$
>-		File file = new File(basefile, path);
>+		File file = new File(this.baseFileCanonicalPath, path);
> 		if (!BundleFile.secureAction.exists(file)) {
> 			return null;
> 		}
>+		boolean checkInBundle = false;
>+		boolean normalize = false;
>+		boolean isBundleRoot = false;
>+		if (path != null) {
>+			isBundleRoot = path.equals("/");//$NON-NLS-1$
>+			if (!isBundleRoot) {
>+				checkInBundle = path.indexOf(POINTER_UPPER_DIRECTORY) >= 0;
>+				if (this.enabledStrictBundleEntryPath) {
>+					normalize = checkInBundle || path.indexOf(POINTER_SAME_DIRECTORY_1) >= 0 || path.indexOf(POINTER_SAME_DIRECTORY_2) >= 0;
>+				}
>+			}
>+		}
>+		File canonicalFile;
>+		try {
>+			canonicalFile = BundleFile.secureAction.getCanonicalFile(file);
>+			if (this.enabledStrictBundleEntryPath && !isBundleRoot) {
>+				File absoluteFile = BundleFile.secureAction.getAbsoluteFile(file);
>+				String canonicalPath;
>+				String absolutePath;
>+				if (normalize) {
>+					canonicalPath = canonicalFile.toURI().getPath();
>+					absolutePath = absoluteFile.toURI().normalize().getPath();
>+				} else {
>+					canonicalPath = canonicalFile.getPath();
>+					absolutePath = absoluteFile.getPath();
>+				}
>+				if (!canonicalPath.equals(absolutePath)) {
>+					return null;
>+				}
>+			}
>+		} catch (IOException e) {
>+			return null;
>+		}
> 		// must do an extra check to make sure file is within the bundle (bug 320546)
> 		if (checkInBundle) {
>-			try {
>-				if (!BundleFile.secureAction.getCanonicalPath(file).startsWith(BundleFile.secureAction.getCanonicalPath(basefile)))
>-					return null;
>-			} catch (IOException e) {
>+			if (!canonicalFile.getPath().startsWith(this.baseFileCanonicalPath))
> 				return null;
>-			}
> 		}
> 		return file;
> 	}
>diff --git a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/util/SecureAction.java b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/util/SecureAction.java
>index 2e654b1..2ff8a2c 100644
>--- a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/util/SecureAction.java
>+++ b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/util/SecureAction.java
>@@ -198,6 +198,44 @@ public class SecureAction {
> 	}
> 
> 	/**
>+	 * Returns the absolute file.  Same as calling
>+	 * file.getAbsoluteFile().
>+	 * @param file a file object
>+	 * @return the absolute file.
>+	 */
>+	public File getAbsoluteFile(final File file) {
>+		if (System.getSecurityManager() == null)
>+			return file.getAbsoluteFile();
>+		return AccessController.doPrivileged(new PrivilegedAction<File>() {
>+			public File run() {
>+				return file.getAbsoluteFile();
>+			}
>+		}, controlContext);
>+	}
>+
>+	/**
>+	 * Returns the canonical file.  Same as calling
>+	 * file.getCanonicalFile().
>+	 * @param file a file object
>+	 * @return the canonical file.
>+	 */
>+	public File getCanonicalFile(final File file) throws IOException {
>+		if (System.getSecurityManager() == null)
>+			return file.getCanonicalFile();
>+		try {
>+			return AccessController.doPrivileged(new PrivilegedExceptionAction<File>() {
>+				public File run() throws IOException {
>+					return file.getCanonicalFile();
>+				}
>+			}, controlContext);
>+		} catch (PrivilegedActionException e) {
>+			if (e.getException() instanceof IOException)
>+				throw (IOException) e.getException();
>+			throw (RuntimeException) e.getException();
>+		}
>+	}
>+
>+	/**
> 	 * Returns true if a file exists, otherwise false is returned.  Same as calling
> 	 * file.exists().
> 	 * @param file a file object

Actions: View | Diff

Attachments on bug 395274: 224051 | 226730 | 229125 | 229500 | 229529 | 229550