Bug 573456 - [performance] Avoid File.getCanonicalPath
File.getCanonicalPath is much slower then URI.normalize under windows
Also avoid normalization if no ".." is involved.
Change-Id: I641a64d48f8acb9cf793c948d08c51ace45aec50
Signed-off-by: Joerg Kubitz <jkubitz-eclipse@gmx.de>
Reviewed-on: https://git.eclipse.org/r/c/equinox/rt.equinox.framework/+/180404
Tested-by: Equinox Bot <equinox-bot@eclipse.org>
Reviewed-by: Thomas Watson <tjwatson@us.ibm.com>
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/AllTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/AllTests.java
index beaa871..9ac4293 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/AllTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/AllTests.java
@@ -30,6 +30,7 @@
addTest(new TestSuite(ObjectPoolTestCase.class));
addTest(new JUnit4TestAdapter(ManifestElementTestCase.class));
addTest(new TestSuite(NLSTestCase.class));
+ addTest(new TestSuite(StorageUtilTestCase.class));
addBidiTests();
addLatinTests();
}
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/StorageUtilTestCase.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/StorageUtilTestCase.java
new file mode 100644
index 0000000..7491382
--- /dev/null
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/StorageUtilTestCase.java
@@ -0,0 +1,53 @@
+/*******************************************************************************
+ * Copyright (c) 2011, 2013 IBM Corporation and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * IBM Corporation - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.osgi.tests.util;
+
+import java.io.File;
+import org.eclipse.core.runtime.Platform;
+import org.eclipse.core.tests.harness.CoreTest;
+import org.eclipse.osgi.storage.StorageUtil;
+import org.junit.Test;
+
+public class StorageUtilTestCase extends CoreTest {
+
+ @Test
+ public void testRegularWindowsFileName() {
+ String[] validFilenames = { //
+ "something\\somethingelse", // normal file
+ "something/somethingelse", // normal file
+ "COM1anything", // normal file
+ "COM1/anything", // illegal directory name but normal file
+ ".temp", // It is acceptable to specify a period as the first character of a
+ // name. For example, ".temp".
+ "COM56", // there is no predefined NT namespace for COM56.
+ };
+ for (String validFilename : validFilenames) {
+ assertFalse(validFilename, StorageUtil.isReservedFileName(new File(validFilename)));
+ }
+ // test reserved names according to
+ // https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
+ String[] invalidFilenames = { //
+ "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9",
+ "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", // reserved names
+ "COM1.anything", "NUL.txt", // "Also avoid these names followed immediately by an extension; for
+ // example, NUL.txt is not recommended"
+ "COM1", "com1", "coM1"// case insensitive
+ };
+ boolean isWindows = Platform.getOS().equals(Platform.OS_WIN32);
+ for (String invalidFilename : invalidFilenames) {
+ assertTrue(invalidFilename, isWindows == StorageUtil.isReservedFileName(new File(invalidFilename)));
+ }
+ }
+
+}
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 f7be9b5..2f6d939 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
@@ -30,6 +30,7 @@
import java.net.URL;
import java.net.URLConnection;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.security.PrivilegedActionException;
@@ -1078,18 +1079,19 @@
}
// 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();
- if (!resultCanonical.startsWith(rootCanonical + File.separator) && !resultCanonical.equals(rootCanonical)) {
+ File rootBase = new File(root, base);
+ File result = new File(rootBase, path);
+ if (path.contains("..")) { //$NON-NLS-1$
+ // do the extra check to make sure the path did not escape the root path
+ Path resultNormalized = result.toPath().normalize();
+ Path rootBaseNormalized = rootBase.toPath().normalize();
+ if (!resultNormalized.startsWith(rootBaseNormalized)) {
throw new StorageException("Invalid path: " + path); //$NON-NLS-1$
}
- } catch (IOException e) {
- throw new StorageException("Invalid path: " + path, e); //$NON-NLS-1$
+ }
+ // Additional check if it is a special device instead of a regular file.
+ if (StorageUtil.isReservedFileName(result)) {
+ throw new StorageException("Invalid filename: " + path); //$NON-NLS-1$
}
return result;
}
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/StorageUtil.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/StorageUtil.java
index 6af3d74..4f4ebc3 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/StorageUtil.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/StorageUtil.java
@@ -25,7 +25,9 @@
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.StandardCopyOption;
+import java.util.Arrays;
import java.util.Dictionary;
+import java.util.HashSet;
import java.util.Hashtable;
import org.eclipse.osgi.internal.debug.Debug;
import org.osgi.framework.BundleContext;
@@ -256,4 +258,44 @@
Debug.println("Successfully moved file: " + from + " to " + to); //$NON-NLS-1$ //$NON-NLS-2$
}
}
+
+ private static final boolean IS_WINDOWS = "\\\\.\\NUL" //$NON-NLS-1$
+ .equals(getDeviceName(new File("NUL"))); //$NON-NLS-1$
+
+ private static String getDeviceName(File file) {
+ try {
+ return file.getCanonicalPath();
+ } catch (IOException e) {
+ return null;
+ }
+ }
+
+ // reserved names according to
+ // https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
+ private static HashSet<String> RESERVED_NAMES = new HashSet<>(
+ Arrays.asList(new String[] { "aux", "com1", "com2", "com3", "com4", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$
+ "com5", "com6", "com7", "com8", "com9", "con", "lpt1", "lpt2", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ //$NON-NLS-7$ //$NON-NLS-8$
+ "lpt3", "lpt4", "lpt5", "lpt6", "lpt7", "lpt8", "lpt9", "nul", "prn" })); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ //$NON-NLS-7$ //$NON-NLS-8$ //$NON-NLS-9$
+
+ /** Tests whether the filename can escape path into special device **/
+ public static boolean isReservedFileName(File file) {
+ // Directory names are not checked here because illegal directory names will be
+ // handled by OS.
+ String fileName = file.getName();
+ return isReservedFileName(fileName);
+ }
+
+ private static boolean isReservedFileName(String name) {
+ // Illegal characters are not checked here because they are check by both JDK
+ // and OS. This is only a check against technical allowed but unwanted device
+ // names.
+ if (!IS_WINDOWS) { // only windows has special file names which can escape any path
+ return false;
+ }
+ int dot = name.indexOf('.');
+ // on windows, filename suffixes are not relevant to name validity
+ String basename = dot == -1 ? name : name.substring(0, dot);
+ return RESERVED_NAMES.contains(basename.toLowerCase());
+ }
+
}
