GP-3914 Corrected ProjectLocator bug for projects in root directory

Ghidra/Framework/Project/src/main/java/ghidra/framework/model/ProjectLocator.java

@@ -17,6 +17,7 @@ package ghidra.framework.model;
 
 import java.io.File;
 import java.net.URL;
+import java.util.Set;
 
 import org.apache.commons.lang3.StringUtils;
 
@@ -37,16 +38,30 @@ public class ProjectLocator {
 
 	private URL url;
 
+	/**
+	 * Set of characters specifically disallowed in project name or path.
+	 * These characters may interfere with path and URL parsing.
+	 */
+	public static Set<Character> DISALLOWED_CHARS = Set.of(':', ';', '&', '?', '#');
+
 	/**
 	 * Construct a project locator object.
-	 * @param path path to parent directory (may or may not exist).  The user's temp directory
-	 * will be used if this value is null or blank.
+	 * @param path absolute path to parent directory (may or may not exist).  The user's temp directory
+	 * will be used if this value is null or blank.  The use of "\" characters will always be replaced 
+	 * with "/".
 	 * WARNING: Use of a relative paths should be avoided (e.g., on a windows platform
-	 * an absolute path should start with a drive letter specification such as C:\path,
-	 * while this same path on a Linux platform would be treated as relative).
-	 * @param name name of the project
+	 * an absolute path should start with a drive letter specification such as C:\path).
+	 * A path such as "/path" on windows will utilize the current default drive and will
+	 * not throw an exception.  If a drive letter is specified it must specify an absolute
+	 * path (e.g., C:\, C:\path).
+	 * @param name name of the project (may only contain alphanumeric characters or   
+	 * @throws IllegalArgumentException if an absolute path is not specified or invalid project name
 	 */
 	public ProjectLocator(String path, String name) {
+		if (name.contains("/") || name.contains("\\")) {
+			throw new IllegalArgumentException("name contains path separator character: " + name);
+		}
+		checkInvalidChar("name", name, 0);
 		if (name.endsWith(PROJECT_FILE_SUFFIX)) {
 			name = name.substring(0, name.length() - PROJECT_FILE_SUFFIX.length());
 		}
@@ -59,25 +74,75 @@ public class ProjectLocator {
 	}
 
 	/**
-	 * Ensure that absolute path is specified.
+	 * Check for characters explicitly disallowed in path or project name.
+	 * @param type type of string to include in exception
+	 * @param str string to check
+	 * @param startIndex index at which to start checking
+	 * @throws IllegalArgumentException if str contains invalid character
+	 */
+	private static void checkInvalidChar(String type, String str, int startIndex) {
+		for (int i = startIndex; i < str.length(); i++) {
+			char c = str.charAt(i);
+			if (DISALLOWED_CHARS.contains(c)) {
+				throw new IllegalArgumentException(
+					type + " contains invalid character: '" + c + "'");
+			}
+		}
+	}
+
+	/**
+	 * Ensure that absolute path is specified and normalize its format.
+	 * An absolute path may start with a windows drive letter (e.g., c:/a/b, /c:/a/b)
+	 * or without (e.g., /a/b).  Although for Windows the lack of a drive letter is
+	 * not absolute, for consistency with Linux we permit this form which on
+	 * Windows will use the default drive for the process. The resulting path
+	 * may be transormed to always end with a "/" and if started with a drive letter
+	 * (e.g., "c:/") it will have a "/" prepended (e.g., "/c:/", both forms
+	 * are treated the same by the {@link File} class under Windows).
 	 * @param path path to be checked and possibly modified.
 	 * @return path to be used
+	 * @throws IllegalArgumentException if an invalid path is specified
 	 */
 	private static String checkAbsolutePath(String path) {
-		if (path.startsWith("/") && path.length() >= 4 && path.indexOf(":/") == 2 &&
-			Character.isLetter(path.charAt(1))) {
-			// strip leading "/" on Windows paths (e.g., /C:/mydir) and transform separators to '\'
-			path = path.substring(1);
-			path = path.replace('/', '\\');
+		int scanIndex = 0;
+		path = path.replace('\\', '/');
+		int len = path.length();
+		if (!path.startsWith("/")) {
+			// Allow paths to start with windows drive letter (e.g., c:/a/b)
+			if (len >= 3 && hasAbsoluteDriveLetter(path, 0)) {
+				path = "/" + path;
+			}
+			else {
+				throw new IllegalArgumentException("absolute path required");
+			}
+			scanIndex = 3;
 		}
-		if (path.endsWith("/") || path.endsWith("\\")) {
-			path = path.substring(0, path.length() - 1);
+		else if (len >= 3 && hasDriveLetter(path, 1)) {
+			if (len < 4 || path.charAt(3) != '/') {
+				// path such as "/c:" not permitted
+				throw new IllegalArgumentException("absolute path required");
+			}
+			scanIndex = 4;
+		}
+		checkInvalidChar("path", path, scanIndex);
+		if (!path.endsWith(File.separator)) {
+			path += File.separator;
 		}
 		return path;
 	}
 
+	private static boolean hasDriveLetter(String path, int index) {
+		return Character.isLetter(path.charAt(index++)) && path.charAt(index) == ':';
+	}
+
+	private static boolean hasAbsoluteDriveLetter(String path, int index) {
+		int pathIndex = index + 2;
+		return path.length() > pathIndex && hasDriveLetter(path, index) &&
+			path.charAt(pathIndex) == '/';
+	}
+
 	/**
-	 * Returns true if this project URL corresponds to a transient project
+	 * @returns true if this project URL corresponds to a transient project
 	 * (e.g., corresponds to remote Ghidra URL)
 	 */
 	public boolean isTransient() {
@@ -85,7 +150,7 @@ public class ProjectLocator {
 	}
 
 	/**
-	 * Returns the URL associated with this local project.  If using a temporary transient
+	 * @returns the URL associated with this local project.  If using a temporary transient
 	 * project location this URL should not be used.
 	 */
 	public URL getURL() {
@@ -93,7 +158,7 @@ public class ProjectLocator {
 	}
 
 	/**
-	 * Get the name of the project identified by this project info.
+	 * @returns the name of the project identified by this project info.
 	 */
 	public String getName() {
 		return name;
@@ -110,21 +175,21 @@ public class ProjectLocator {
 	}
 
 	/**
-	 * Returns the project directory
+	 * @returns the project directory
 	 */
 	public File getProjectDir() {
 		return new File(location, name + PROJECT_DIR_SUFFIX);
 	}
 
 	/**
-	 * Returns the file that indicates a Ghidra project.
+	 * @returns the file that indicates a Ghidra project.
 	 */
 	public File getMarkerFile() {
 		return new File(location, name + PROJECT_FILE_SUFFIX);
 	}
 
 	/**
-	 * Returns project lock file to prevent multiple accesses to the
+	 * @returns project lock file to prevent multiple accesses to the
 	 * same project at once.
 	 */
 	public File getProjectLockFile() {
@@ -132,7 +197,7 @@ public class ProjectLocator {
 	}
 
 	/**
-	 * Returns the project directory file extension.
+	 * @returns the project directory file extension.
 	 */
 	public static String getProjectDirExtension() {
 		return PROJECT_DIR_SUFFIX;
@@ -164,7 +229,7 @@ public class ProjectLocator {
 	}
 
 	/**
-	 * Returns the file extension suitable for creating file filters for the file chooser.
+	 * @returns the file extension suitable for creating file filters for the file chooser.
 	 */
 	public static String getProjectExtension() {
 		return PROJECT_FILE_SUFFIX;
@@ -180,7 +245,7 @@ public class ProjectLocator {
 	}
 
 	/**
-	 * Returns true if project storage exists
+	 * @returns true if project storage exists
 	 */
 	public boolean exists() {
 		return getMarkerFile().isFile() && getProjectDir().isDirectory();

Ghidra/Framework/Project/src/main/java/ghidra/framework/protocol/ghidra/GhidraURL.java

@@ -15,6 +15,7 @@
  */
 package ghidra.framework.protocol.ghidra;
 
+import java.io.File;
 import java.net.*;
 import java.util.Objects;
 import java.util.regex.Pattern;
@@ -33,6 +34,8 @@ import ghidra.framework.remote.GhidraServerHandle;
  */
 public class GhidraURL {
 
+	// TODO: URL encoding/decoding should be used
+
 	public static final String PROTOCOL = "ghidra";
 
 	private static final String PROTOCOL_URL_START = PROTOCOL + ":/";
@@ -191,24 +194,67 @@ public class GhidraURL {
 	}
 
 	/**
-	 * Ensure that absolute path is specified.  Any use of Windows 
-	 * separator (back-slash) will be converted to a forward-slash. 
+	 * Ensure that absolute path is specified and normalize its format.
+	 * An absolute path may start with a windows drive letter (e.g., c:/a/b, /c:/a/b)
+	 * or without (e.g., /a/b).  Although for Windows the lack of a drive letter is
+	 * not absolute, for consistency with Linux we permit this form which on
+	 * Windows will use the default drive for the process. If path starts with a drive 
+	 * letter (e.g., "c:/") it will have a "/" prepended (e.g., "/c:/", both forms
+	 * are treated the same by the {@link File} class under Windows).
 	 * @param path path to be checked and possibly modified.
 	 * @return path to be used
+	 * @throws IllegalArgumentException if an invalid path is specified
 	 */
 	private static String checkAbsolutePath(String path) {
+		int scanIndex = 0;
 		path = path.replace('\\', '/');
+		int len = path.length();
 		if (!path.startsWith("/")) {
-			if (path.length() >= 3 && path.indexOf(":/") == 1 &&
-				Character.isLetter(path.charAt(0))) {
-				// prepend a "/" on Windows paths (e.g., C:/mydir)
+			// Allow paths to start with windows drive letter (e.g., c:/a/b)
+			if (len >= 3 && hasAbsoluteDriveLetter(path, 0)) {
 				path = "/" + path;
 			}
-			else { // absence of drive letter is tolerated even if not absolute on windows
-				throw new IllegalArgumentException("Absolute directory path required");
+			else {
+				throw new IllegalArgumentException("absolute path required");
+			}
+			scanIndex = 3;
+		}
+		else if (len >= 3 && hasDriveLetter(path, 1)) {
+			if (len < 4 || path.charAt(3) != '/') {
+				// path such as "/c:" not permitted
+				throw new IllegalArgumentException("absolute path required");
+			}
+			scanIndex = 4;
+		}
+		checkInvalidChar("path", path, scanIndex);
+		return path;
+	}
+
+	private static boolean hasDriveLetter(String path, int index) {
+		return Character.isLetter(path.charAt(index++)) && path.charAt(index) == ':';
+	}
+
+	private static boolean hasAbsoluteDriveLetter(String path, int index) {
+		int pathIndex = index + 2;
+		return path.length() > pathIndex && hasDriveLetter(path, index) &&
+			path.charAt(pathIndex) == '/';
+	}
+
+	/**
+	 * Check for characters explicitly disallowed in path or project name.
+	 * @param type type of string to include in exception
+	 * @param str string to check
+	 * @param startIndex index at which to start checking
+	 * @throws IllegalArgumentException if str contains invalid character
+	 */
+	private static void checkInvalidChar(String type, String str, int startIndex) {
+		for (int i = startIndex; i < str.length(); i++) {
+			char c = str.charAt(i);
+			if (ProjectLocator.DISALLOWED_CHARS.contains(c)) {
+				throw new IllegalArgumentException(
+					type + " contains invalid character: '" + c + "'");
 			}
 		}
-		return path;
 	}
 
 	/**
@@ -231,8 +277,15 @@ public class GhidraURL {
 				throw new IllegalArgumentException("Unsupported query/ref used with project path");
 			}
 			projectPathOrURL = checkAbsolutePath(projectPathOrURL);
-			String[] splitName = splitOffName(projectPathOrURL);
-			return makeURL(splitName[0], splitName[1]);
+			int minSplitIndex = projectPathOrURL.charAt(2) == ':' ? 3 : 0;
+			int splitIndex = projectPathOrURL.lastIndexOf('/');
+			if (splitIndex < minSplitIndex || projectPathOrURL.length() == (splitIndex + 1)) {
+				throw new IllegalArgumentException("Absolute project path is missing project name");
+			}
+			++splitIndex;
+			String location = projectPathOrURL.substring(0, splitIndex);
+			String projectName = projectPathOrURL.substring(splitIndex);
+			return makeURL(location, projectName);
 		}
 		try {
 			return new URL(projectPathOrURL);
@@ -442,11 +495,12 @@ public class GhidraURL {
 	 * @param projectFilePath file path (e.g., /a/b/c, may be null)
 	 * @param ref location reference (may be null)
 	 * @return local Ghidra project URL
+	 * @throws IllegalArgumentException if an absolute projectLocation path is not specified
 	 */
 	public static URL makeURL(String projectLocation, String projectName, String projectFilePath,
 			String ref) {
 		if (StringUtils.isBlank(projectLocation) || StringUtils.isBlank(projectName)) {
-			throw new IllegalArgumentException("Inavlid project location and/or name");
+			throw new IllegalArgumentException("Invalid project location and/or name");
 		}
 		String path = checkAbsolutePath(projectLocation);
 		if (!path.endsWith("/")) {

Ghidra/Framework/Project/src/test/java/ghidra/framework/model/ProjectLocatorTest.java

@@ -0,0 +1,182 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.framework.model;
+
+import static org.junit.Assert.*;
+
+import java.io.File;
+import java.net.MalformedURLException;
+import java.net.URL;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import generic.test.AbstractGenericTest;
+import ghidra.framework.OperatingSystem;
+import ghidra.framework.protocol.ghidra.Handler;
+
+public class ProjectLocatorTest extends AbstractGenericTest {
+
+	@Before
+	public void setUp() {
+		Handler.registerHandler();
+	}
+
+	//
+	// Behavior of test differs when run on Windows vs Linux/Mac
+	//
+
+	@Test
+	public void testPaths() throws MalformedURLException {
+
+		ProjectLocator pl = new ProjectLocator("c:\\", "bob");
+		assertEquals(new URL("ghidra:/c:/bob"), pl.getURL());
+		assertEquals("/c:/", pl.getLocation());
+		assertEquals(new File("/c:/bob.rep"), pl.getProjectDir());
+		assertEquals(new File("/c:/bob.gpr"), pl.getMarkerFile());
+		assertEquals("bob", pl.getName());
+
+		if (OperatingSystem.CURRENT_OPERATING_SYSTEM == OperatingSystem.WINDOWS) {
+			assertEquals("c:/bob.rep", pl.getProjectDir().getAbsolutePath());
+			assertEquals("c:/bob.gpr", pl.getMarkerFile().getAbsolutePath());
+		}
+
+		pl = new ProjectLocator("/c:/", "bob");
+		assertEquals(new URL("ghidra:/c:/bob"), pl.getURL());
+		assertEquals("/c:/", pl.getLocation());
+		assertEquals(new File("/c:/bob.rep"), pl.getProjectDir());
+		assertEquals(new File("/c:/bob.gpr"), pl.getMarkerFile());
+		assertEquals("bob", pl.getName());
+
+		if (OperatingSystem.CURRENT_OPERATING_SYSTEM == OperatingSystem.WINDOWS) {
+			assertEquals("c:/bob.rep", pl.getProjectDir().getAbsolutePath());
+			assertEquals("c:/bob.gpr", pl.getMarkerFile().getAbsolutePath());
+		}
+
+		pl = new ProjectLocator("c:\\a", "bob");
+		assertEquals(new URL("ghidra:/c:/a/bob"), pl.getURL());
+		assertEquals("/c:/a/", pl.getLocation());
+		assertEquals(new File("/c:/a/bob.rep"), pl.getProjectDir());
+		assertEquals(new File("/c:/a/bob.gpr"), pl.getMarkerFile());
+		assertEquals("bob", pl.getName());
+
+		if (OperatingSystem.CURRENT_OPERATING_SYSTEM == OperatingSystem.WINDOWS) {
+			assertEquals("c:/a/bob.rep", pl.getProjectDir().getAbsolutePath());
+			assertEquals("c:/a/bob.gpr", pl.getMarkerFile().getAbsolutePath());
+		}
+
+		pl = new ProjectLocator("c:\\a\\", "bob");
+		assertEquals(new URL("ghidra:/c:/a/bob"), pl.getURL());
+		assertEquals("/c:/a/", pl.getLocation());
+		assertEquals(new File("/c:/a/bob.rep"), pl.getProjectDir());
+		assertEquals(new File("/c:/a/bob.gpr"), pl.getMarkerFile());
+		assertEquals("bob", pl.getName());
+
+		if (OperatingSystem.CURRENT_OPERATING_SYSTEM == OperatingSystem.WINDOWS) {
+			assertEquals("c:/a/bob.rep", pl.getProjectDir().getAbsolutePath());
+			assertEquals("c:/a/bob.gpr", pl.getMarkerFile().getAbsolutePath());
+		}
+
+		pl = new ProjectLocator("\\", "bob");
+		assertEquals(new URL("ghidra:/bob"), pl.getURL());
+		assertEquals("/", pl.getLocation());
+		assertEquals(new File("/bob.rep"), pl.getProjectDir());
+		assertEquals(new File("/bob.gpr"), pl.getMarkerFile());
+		assertEquals("bob", pl.getName());
+
+		if (OperatingSystem.CURRENT_OPERATING_SYSTEM == OperatingSystem.WINDOWS) {
+			// NOTE: Sensitive to default drive for process
+			assertEquals("c:/bob.rep", pl.getProjectDir().getAbsolutePath());
+			assertEquals("c:/bob.gpr", pl.getMarkerFile().getAbsolutePath());
+		}
+
+		pl = new ProjectLocator("\\a\\", "bob");
+		assertEquals(new URL("ghidra:/a/bob"), pl.getURL());
+		assertEquals("/a/", pl.getLocation());
+		assertEquals(new File("/a/bob.rep"), pl.getProjectDir());
+		assertEquals(new File("/a/bob.gpr"), pl.getMarkerFile());
+		assertEquals("bob", pl.getName());
+
+		if (OperatingSystem.CURRENT_OPERATING_SYSTEM == OperatingSystem.WINDOWS) {
+			// NOTE: Sensitive to default drive for process
+			assertEquals("c:/a/bob.rep", pl.getProjectDir().getAbsolutePath());
+			assertEquals("c:/a/bob.gpr", pl.getMarkerFile().getAbsolutePath());
+		}
+	}
+
+	@Test
+	public void testTempPath() throws MalformedURLException {
+
+		String tmpPath = System.getProperty("java.io.tmpdir").replace("\\", "/");
+		if (!tmpPath.startsWith("/")) {
+			tmpPath = "/" + tmpPath;
+		}
+		if (!tmpPath.endsWith("/")) {
+			tmpPath += "/";
+		}
+
+		ProjectLocator pl = new ProjectLocator("", "bob");
+		assertEquals(tmpPath, pl.getLocation());
+		assertEquals(new URL("ghidra:" + tmpPath + "bob"), pl.getURL());
+		assertEquals(new File(pl.getLocation() + "bob.rep"), pl.getProjectDir());
+		assertEquals(new File(pl.getLocation() + "bob.gpr"), pl.getMarkerFile());
+		assertEquals("bob", pl.getName());
+
+		pl = new ProjectLocator(null, "bob");
+		assertEquals(tmpPath, pl.getLocation());
+		assertEquals(new URL("ghidra:" + tmpPath + "bob"), pl.getURL());
+		assertEquals(new File(pl.getLocation() + "bob.rep"), pl.getProjectDir());
+		assertEquals(new File(pl.getLocation() + "bob.gpr"), pl.getMarkerFile());
+		assertEquals("bob", pl.getName());
+	}
+
+	@Test
+	public void testBadPaths() {
+
+		// relative paths
+		doTestBadPath("c:", "bob");
+		doTestBadPath("/c:", "bob");
+		doTestBadPath("a/b", "bob");
+
+		// bad paths chars
+		doTestBadPath("/a?/b", "bob");
+		doTestBadPath("/a#/b", "bob");
+		doTestBadPath("/a/:b", "bob");
+		doTestBadPath("/a;/b", "bob");
+		doTestBadPath("/a&/b", "bob");
+
+		// bad name chars
+		doTestBadPath("/a/b", "b?ob");
+		doTestBadPath("/a/b", "b#ob");
+		doTestBadPath("/a/b", "b:ob");
+		doTestBadPath("/a/b", "b;ob");
+		doTestBadPath("/a/b", "b?ob");
+		doTestBadPath("/a/b", "b&ob");
+		doTestBadPath("/a/b", "b/ob");
+		doTestBadPath("/a/b", "b\\ob");
+
+	}
+
+	private void doTestBadPath(String path, String name) {
+		try {
+			new ProjectLocator(path, name);
+			fail("expected absolute path error");
+		}
+		catch (IllegalArgumentException e) {
+			// expected
+		}
+	}
+}