From 4cc11e3e1ca3e0b9e795f081feacdcac49671752 Mon Sep 17 00:00:00 2001 From: ghidra1 Date: Thu, 28 Mar 2024 10:50:16 -0400 Subject: [PATCH] GP-0 Improved error handling of bad project folder items locally and within server repository and updated javadocs. --- .../ghidra/server/store/RepositoryFile.java | 107 +++++++++--------- .../ghidra/server/store/RepositoryFolder.java | 88 ++++++++------ .../store/local/LocalFolderItem.java | 27 +++-- .../framework/data/GhidraFolderData.java | 24 +++- 4 files changed, 148 insertions(+), 98 deletions(-) diff --git a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/store/RepositoryFile.java b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/store/RepositoryFile.java index cb3294b9d1..3025f6f5ab 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/store/RepositoryFile.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/store/RepositoryFile.java @@ -49,27 +49,13 @@ public class RepositoryFile { * @param fileSystem local file-system which corresponds to repository. * @param parent parent repository folder * @param name item/file name - * @throws IOException */ RepositoryFile(Repository repository, LocalFileSystem fileSystem, RepositoryFolder parent, - String name) throws IOException { + String name) { this.repository = repository; this.fileSystem = fileSystem; this.parent = parent; this.name = name; -// LocalFolderItem folderItem = fileSystem.getItem(parent.getPathname(), name); -// if (folderItem == null || !folderItem.isVersioned() || -// !(folderItem instanceof LocalDatabaseItem)) { -// // must build pathname just in case folderItem does not exist -// String pathname = parent.getPathname(); -// if (pathname.length() != 1) { -// pathname += "/"; -// } -// pathname += name; -// RepositoryManager.log(repository.getName(), pathname, "file is corrupt", null); -// throw new FileNotFoundException(pathname + " is corrupt"); -// } -// this.databaseItem = (LocalDatabaseItem) folderItem; } /** @@ -104,6 +90,7 @@ public class RepositoryFile { /** * Returns item/file name + * @return file name */ public String getName() { return name; @@ -111,6 +98,7 @@ public class RepositoryFile { /** * Returns parent folder + * @return parent folder */ public RepositoryFolder getParent() { return parent; @@ -118,6 +106,7 @@ public class RepositoryFile { /** * Returns file/item path within repository. + * @return path within repository */ public String getPathname() { synchronized (fileSystem) { @@ -131,7 +120,7 @@ public class RepositoryFile { /** * Returns data pertaining to this file. - * @throws IOException + * @return Serializable {@link RepositoryItem} which corresponds to this file */ public RepositoryItem getItem() { synchronized (fileSystem) { @@ -157,9 +146,10 @@ public class RepositoryFile { * This method is only valid for an underlying FolderItem of type database. * @param version requested version or -1 for current version * @param minChangeDataVer minimum version to include within change data or -1 if not applicable. - * @param user + * @param user user who initiated the request * @return open BufferFile for read-only use. - * @throws IOException + * @throws UserAccessException if user is denied access + * @throws IOException if an IO error occurs */ public LocalManagedBufferFile openDatabase(int version, int minChangeDataVer, String user) throws IOException { @@ -177,8 +167,10 @@ public class RepositoryFile { /** * Open the current version for checkin use. * @param checkoutId checkout ID - * @param user + * @param user user who initiated the request * @return open BufferFile for update/checkin use + * @throws UserAccessException if user is denied write access + * @throws IOException if an IO error occurs */ public LocalManagedBufferFile openDatabase(long checkoutId, String user) throws IOException { synchronized (fileSystem) { @@ -199,7 +191,11 @@ public class RepositoryFile { } /** - * Returns list of all available versions. + * Returns all available versions. + * @param user user who initiated the request + * @return all available versions + * @throws UserAccessException if user is denied access + * @throws IOException if an IO error occurs */ public Version[] getVersions(String user) throws IOException { synchronized (fileSystem) { @@ -225,35 +221,36 @@ public class RepositoryFile { /** * Delete oldest or current version of this file/item. - * @param version oldest or current version, or -1 to remove + * @param deleteVersion oldest or current version, or -1 to remove * all versions. - * @param user - * @throws IOException + * @param user user who initiated the request + * @throws UserAccessException if user is denied ability to delete version(s) + * @throws IOException if an IO error occurs */ - public void delete(int version, String user) throws IOException { + public void delete(int deleteVersion, String user) throws IOException { synchronized (fileSystem) { validate(); User userObj = repository.validateWritePrivilege(user); if (!userObj.isAdmin()) { Version[] versions = databaseItem.getVersions(); - if (version == -1) { - for (int i = 0; i < versions.length; i++) { - if (!user.equals(versions[i].getUser())) { + if (deleteVersion == -1) { + for (Version version : versions) { + if (!user.equals(version.getUser())) { throw new UserAccessException(getName() + " version " + - versions[i].getVersion() + " owned by " + versions[i].getUser()); + version.getVersion() + " owned by " + version.getUser()); } } } - else if (version == versions[0].getVersion()) { + else if (deleteVersion == versions[0].getVersion()) { if (!user.equals(versions[0].getUser())) { - throw new UserAccessException(getName() + " version " + version + + throw new UserAccessException(getName() + " version " + deleteVersion + " owned by " + versions[0].getUser()); } } - else if (version == versions[versions.length - 1].getVersion()) { + else if (deleteVersion == versions[versions.length - 1].getVersion()) { if (!user.equals(versions[versions.length - 1].getUser())) { - throw new UserAccessException(getName() + " version " + version + + throw new UserAccessException(getName() + " version " + deleteVersion + " owned by " + versions[versions.length - 1].getUser()); } } @@ -267,7 +264,7 @@ public class RepositoryFile { } else { - databaseItem.delete(version, user); + databaseItem.delete(deleteVersion, user); } deleted = true; repositoryItem = null; @@ -284,9 +281,10 @@ public class RepositoryFile { * Move this file/item to a new folder and optionally change its name. * @param newParent new parent folder * @param newItemName new file/item name - * @param user + * @param user user who initiated the request * @throws InvalidNameException if name is invalid - * @throws IOException + * @throws UserAccessException if user is denied write access + * @throws IOException if an IO error occurs */ public void moveTo(RepositoryFolder newParent, String newItemName, String user) throws InvalidNameException, IOException { @@ -309,10 +307,12 @@ public class RepositoryFile { /** * Request a checkout of the underlying item. * @param checkoutType checkout type requested - * @param user + * @param user user who initiated the request + * @param projectPath user's project path which will own checkout * @return checkout data if successful. Null is returned if exclusive checkout * failed due to existing checkout(s). - * @throws IOException + * @throws UserAccessException if user is denied write access + * @throws IOException if an IO error occurs */ public ItemCheckoutStatus checkout(CheckoutType checkoutType, String user, String projectPath) throws IOException { @@ -332,8 +332,8 @@ public class RepositoryFile { * Update checkout version for an existing checkout. * @param checkoutId existing checkout ID * @param checkoutVersion newer version now associated with checkout - * @param user - * @throws IOException + * @param user user who initiated the request + * @throws IOException if an IO error occurs */ public void updateCheckoutVersion(long checkoutId, int checkoutVersion, String user) throws IOException { @@ -346,9 +346,9 @@ public class RepositoryFile { /** * Terminate an existing checkout * @param checkoutId existing checkout ID - * @param user + * @param user user who initiated the request * @param notify if true notify listeners of item change. - * @throws IOException + * @throws IOException if an IO error occurs */ public void terminateCheckout(long checkoutId, String user, boolean notify) throws IOException { synchronized (fileSystem) { @@ -368,8 +368,10 @@ public class RepositoryFile { /** * Returns checkout data for a specified checkout ID. * @param checkoutId existing checkout ID - * @param user - * @throws IOException + * @param user user who initiated the request + * @throws UserAccessException if user is denied access + * @throws IOException if an IO error occurs + * @return checkout data for a specified checkout ID. */ public ItemCheckoutStatus getCheckout(long checkoutId, String user) throws IOException { synchronized (fileSystem) { @@ -380,9 +382,11 @@ public class RepositoryFile { } /** - * Returns a list of all checkouts for this file/item. - * @param user - * @throws IOException + * Returns all checkouts for this file/item. + * @param user user who initiated the request + * @throws UserAccessException if user is denied access + * @throws IOException if an IO error occurs + * @return all checkouts for this file/item. */ public ItemCheckoutStatus[] getCheckouts(String user) throws IOException { synchronized (fileSystem) { @@ -394,7 +398,8 @@ public class RepositoryFile { /** * Returns true if one or more checkouts exist for this file/item. - * @throws IOException + * @throws IOException if an IO error occurs + * @return true if one or more checkouts exist for this file/item. */ public boolean hasCheckouts() throws IOException { synchronized (fileSystem) { @@ -405,7 +410,8 @@ public class RepositoryFile { /** * Returns true if checkin is currently in process. - * @throws IOException + * @throws IOException if an IO error occurs + * @return true if checkin is currently in process. */ public boolean isCheckinActive() throws IOException { synchronized (fileSystem) { @@ -419,15 +425,12 @@ public class RepositoryFile { */ public void itemChanged() { synchronized (fileSystem) { - // Nulling the repositoryItem deletes the cache information & gets new version info. repositoryItem = null; } } /** - * Reaquire associated folder item following a folder move or name change. - * @param newName items new name (which may be unchanged if path change was - * the result of a moved or renamed folder). + * Clear cached data as a result of a path change */ void pathChanged() { synchronized (fileSystem) { diff --git a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/store/RepositoryFolder.java b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/store/RepositoryFolder.java index 9d1c152710..c9d8a5b707 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/store/RepositoryFolder.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/store/RepositoryFolder.java @@ -23,8 +23,9 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import db.buffers.LocalManagedBufferFile; -import ghidra.framework.store.FileSystem; +import ghidra.framework.store.*; import ghidra.framework.store.local.LocalFileSystem; +import ghidra.framework.store.local.LocalFolderItem; import ghidra.server.Repository; import ghidra.server.RepositoryManager; import ghidra.util.InvalidNameException; @@ -62,12 +63,11 @@ public class RepositoryFolder { /** * Constructor for non-root folders - * @param fileSystem the private file system - * @param versionedFileSystem file system for storing version controlled items + * @param repository shared repository + * @param fileSystem local file system for storing version controlled items * @param parent parent folder * @param name name of this folder - * @param listener listener for DomainFolder changes. - * @throws IOException + * @throws IOException if an IO error occurs */ private RepositoryFolder(Repository repository, LocalFileSystem fileSystem, RepositoryFolder parent, String name) throws IOException { @@ -80,10 +80,9 @@ public class RepositoryFolder { /** * Constructor for the root folder - * @param fileSystem the private file system - * @param versionedFileSystem file system for storing version controlled items - * @param listener listener for DomainFolder changes. - * @throws IOException + * @param repository shared repository + * @param fileSystem local file system for storing version controlled items + * @throws IOException if an IO error occurs */ public RepositoryFolder(Repository repository, LocalFileSystem fileSystem) throws IOException { this.repository = repository; @@ -95,25 +94,30 @@ public class RepositoryFolder { private void init() throws IOException { String path = getPathname(); String[] names = fileSystem.getFolderNames(path); - for (int i = 0; i < names.length; i++) { - RepositoryFolder subfolder = - new RepositoryFolder(repository, fileSystem, this, names[i]); - folderMap.put(names[i], subfolder); + for (String name2 : names) { + RepositoryFolder subfolder = new RepositoryFolder(repository, fileSystem, this, name2); + folderMap.put(name2, subfolder); } names = fileSystem.getItemNames(path); - for (int i = 0; i < names.length; i++) { - try { - RepositoryFile rf = new RepositoryFile(repository, fileSystem, this, names[i]); - fileMap.put(names[i], rf); - } - catch (FileNotFoundException e) { - // Skip + int badItemCount = 0; + for (String name2 : names) { + LocalFolderItem item = fileSystem.getItem(path, name2); + if (item == null || !(item instanceof DatabaseItem)) { + ++badItemCount; + continue; } + RepositoryFile rf = new RepositoryFile(repository, fileSystem, this, name2); + fileMap.put(name2, rf); + } + if (badItemCount != 0) { + log.error("Repository '" + repository.getName() + "' contains " + badItemCount + + " bad items: " + getPathname()); } } /** * Returns folder name + * @return folder name */ public String getName() { return name; @@ -121,6 +125,7 @@ public class RepositoryFolder { /** * Returns parent folder or null if this is the root folder. + * @return parent folder or null */ public RepositoryFolder getParent() { return parent; @@ -128,6 +133,7 @@ public class RepositoryFolder { /** * Returns folder path within repository + * @return folder path */ public String getPathname() { synchronized (fileSystem) { @@ -141,7 +147,8 @@ public class RepositoryFolder { } /** - * Returns list of sub-folders contained within this folder + * Returns all sub-folders contained within this folder + * @return all sub-folders */ public RepositoryFolder[] getFolders() { synchronized (fileSystem) { @@ -156,6 +163,7 @@ public class RepositoryFolder { * Returns sub-folders with the specified name or null * if sub-folder not found within this folder. * @param folderName sub-folder name + * @return specified sub-folder or null if not found */ public RepositoryFolder getFolder(String folderName) { synchronized (fileSystem) { @@ -180,7 +188,8 @@ public class RepositoryFolder { } /** - * Returns list of files/items contained within this folder + * Returns all files/items contained within this folder + * @return all files/items contained within this folder */ public RepositoryFile[] getFiles() { synchronized (fileSystem) { @@ -195,6 +204,7 @@ public class RepositoryFolder { * Returns files/items with the specified name or null * if file/item not found within this folder. * @param fileName sub-folder name + * @return named file or null if not found */ public RepositoryFile getFile(String fileName) { synchronized (fileSystem) { @@ -202,8 +212,16 @@ public class RepositoryFolder { if (rf != null) { return rf; } + // NOTE: Uncertain what condition would lead to exiting file not already + // existing in fileMap if (fileSystem.fileExists(getPathname(), fileName)) { try { + LocalFolderItem item = fileSystem.getItem(getPathname(), fileName); + if (item == null || !(item instanceof DatabaseItem)) { + log.error("Repository '" + repository.getName() + "' contains bad item: " + + makePathname(getPathname(), fileName)); + return null; + } rf = new RepositoryFile(repository, fileSystem, this, fileName); fileMap.put(fileName, rf); return rf; @@ -220,10 +238,11 @@ public class RepositoryFolder { /** * Create a new sub-folder within this folder and the associated directory on the local file-system. * @param folderName new sub-folder name - * @param user + * @param user user who is initiating request * @return new folder * @throws InvalidNameException if folder name is invalid - * @throws IOException + * @throws DuplicateFileException if folder already exists with specified name + * @throws IOException if an IO error occurs */ public RepositoryFolder createFolder(String folderName, String user) throws InvalidNameException, IOException { @@ -246,13 +265,15 @@ public class RepositoryFolder { /** * Create a new database file/item within this folder. * @param itemName name of new database + * @param fileID file ID * @param bufferSize preferred database buffer size * @param contentType application content type - * @param user - * @param projectPath + * @param user user who is initiating request + * @param projectPath file path within repository * @return buffer file (contains checkoutId as checkinId) - * @throws InvalidNameException - * @throws IOException + * @throws InvalidNameException if itemName is invalid + * @throws DuplicateFileException if file already exists with specified name + * @throws IOException if an IO error occurs */ public LocalManagedBufferFile createDatabase(String itemName, String fileID, int bufferSize, String contentType, String user, String projectPath) @@ -275,7 +296,9 @@ public class RepositoryFolder { /** * Delete this empty folder. - * @throws IOException + * @throws FolderNotEmptyException Thrown if the folder is not empty. + * @throws FileNotFoundException if there is no folder with the given path name. + * @throws IOException if error occurred during delete. */ public void delete() throws IOException { synchronized (fileSystem) { @@ -325,6 +348,7 @@ public class RepositoryFolder { * Move child RepositoryItem into its new parent folder * after the underlying item has already been moved. * @param rf child RepositoryItem + * @param oldName old file name * @param newFolder new parent folder for rf */ void fileMoved(RepositoryFile rf, String oldName, RepositoryFolder newFolder) { @@ -336,11 +360,11 @@ public class RepositoryFolder { /** * Move this folder to a new parent folder and optionally change its name. - * @param newParentPath new parent folder + * @param newParent new parent folder * @param newFolderName new name for this folder - * @param user + * @param user user who is initiating request * @throws InvalidNameException if newFolderName is invalid - * @throws IOException + * @throws IOException if operation fails */ public void moveTo(RepositoryFolder newParent, String newFolderName, String user) throws InvalidNameException, IOException { diff --git a/Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/store/local/LocalFolderItem.java b/Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/store/local/LocalFolderItem.java index 6e361d8a41..695fdbd198 100644 --- a/Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/store/local/LocalFolderItem.java +++ b/Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/store/local/LocalFolderItem.java @@ -159,9 +159,10 @@ public abstract class LocalFolderItem implements FolderItem { File getDataDir() { synchronized (fileSystem) { // Use hidden DB directory - return new File(propertyFile.getFolder(), LocalFileSystem.HIDDEN_DIR_PREFIX + - LocalFileSystem.escapeHiddenDirPrefixChars(propertyFile.getStorageName()) + - DATA_DIR_EXTENSION); + return new File(propertyFile.getFolder(), + LocalFileSystem.HIDDEN_DIR_PREFIX + + LocalFileSystem.escapeHiddenDirPrefixChars(propertyFile.getStorageName()) + + DATA_DIR_EXTENSION); } } @@ -187,8 +188,8 @@ public abstract class LocalFolderItem implements FolderItem { throw new FileInUseException(getName() + " versioning error", e); } if (isCheckedOut) { - throw new FileInUseException(getName() + " version " + version + - " is checked out"); + throw new FileInUseException( + getName() + " version " + version + " is checked out"); } } else if (!isVersioned && getCheckoutId() != DEFAULT_CHECKOUT_ID) { @@ -373,7 +374,8 @@ public abstract class LocalFolderItem implements FolderItem { } finally { if (!success) { - if (useDataDir && !dataDir.exists() && chkDir.exists() && propertyFile.exists()) { + if (useDataDir && !dataDir.exists() && chkDir.exists() && + propertyFile.exists()) { chkDir.renameTo(dataDir); } } @@ -657,8 +659,7 @@ public abstract class LocalFolderItem implements FolderItem { synchronized (fileSystem) { ItemCheckoutStatus coStatus = - checkoutMgr.newCheckout(checkoutType, - user, getCurrentVersion(), projectPath); + checkoutMgr.newCheckout(checkoutType, user, getCurrentVersion(), projectPath); if (checkoutType != CheckoutType.NORMAL && coStatus != null && getFileID() == null) { // Establish missing fileID for on exclusive checkout resetFileID(); @@ -808,12 +809,18 @@ public abstract class LocalFolderItem implements FolderItem { else if (fileType == DATABASE_FILE_TYPE) { return new LocalDatabaseItem(fileSystem, propertyFile); } + else { + log.error("Item has unknown content type: " + + new File(propertyFile.getFolder(), propertyFile.getStorageName())); + } } catch (FileNotFoundException e) { - log.error("Item may be corrupt due to missing file: " + propertyFile.getPath(), e); + log.error("Item may be corrupt due to missing file: " + + new File(propertyFile.getFolder(), propertyFile.getStorageName()), e); } catch (IOException e) { - log.error("Item may be corrupt: " + propertyFile.getPath(), e); + log.error("Item may be corrupt: " + + new File(propertyFile.getFolder(), propertyFile.getStorageName()), e); } return new UnknownFolderItem(fileSystem, propertyFile); } diff --git a/Ghidra/Framework/Project/src/main/java/ghidra/framework/data/GhidraFolderData.java b/Ghidra/Framework/Project/src/main/java/ghidra/framework/data/GhidraFolderData.java index d1d1cd5056..58707b10e0 100644 --- a/Ghidra/Framework/Project/src/main/java/ghidra/framework/data/GhidraFolderData.java +++ b/Ghidra/Framework/Project/src/main/java/ghidra/framework/data/GhidraFolderData.java @@ -26,8 +26,7 @@ import ghidra.framework.protocol.ghidra.TransientProjectData; import ghidra.framework.store.FileSystem; import ghidra.framework.store.FolderItem; import ghidra.framework.store.FolderNotEmptyException; -import ghidra.framework.store.local.LocalFileSystem; -import ghidra.framework.store.local.LocalFolderItem; +import ghidra.framework.store.local.*; import ghidra.util.*; import ghidra.util.exception.*; import ghidra.util.task.TaskMonitor; @@ -615,10 +614,27 @@ class GhidraFolderData { private Map itemMapOf(T[] items) { Map map = new HashMap<>(); + int badItemCount = 0; + int nullNameCount = 0; for (T item : items) { - if (item != null) { - map.put(item.getName(), item); + if (item == null || item instanceof UnknownFolderItem) { + ++badItemCount; + continue; } + String itemName = item.getName(); + if (itemName == null) { + ++nullNameCount; + continue; + } + map.put(itemName, item); + } + if (badItemCount != 0) { + Msg.error(this, + "Project folder contains " + badItemCount + " bad items: " + getPathname()); + } + if (nullNameCount != 0) { + Msg.error(this, + "Project folder contains " + nullNameCount + " null items: " + getPathname()); } return map; }