Merge remote-tracking branch 'origin/GP-4632_dev747368_better_gfilesystem_symlinks'

Ghidra/Features/Base/src/main/java/ghidra/formats/gfilesystem/FileSystemIndexHelper.java

@@ -17,7 +17,6 @@ package ghidra.formats.gfilesystem;
 
 import java.io.IOException;
 import java.util.*;
-import java.util.stream.Collectors;
 
 import ghidra.util.Msg;
 
@@ -35,12 +34,25 @@ import ghidra.util.Msg;
  */
 public class FileSystemIndexHelper<METADATATYPE> {
 
-	private GFile rootDir;
+	private static final int MAX_SYMLINK_RECURSE_DEPTH = 10;
+	private FileData<METADATATYPE> rootDir;
 	
 	static class FileData<METADATATYPE> {
 		GFile file;
 		METADATATYPE metaData;
-		long fileIndex;
+		final long fileIndex;
+		final String symlinkPath;
+		
+		FileData(GFile file, METADATATYPE metaData, long fileIndex) {
+			this(file, metaData, fileIndex, null);
+		}
+
+		FileData(GFile file, METADATATYPE metaData, long fileIndex, String symlinkPath) {
+			this.file = file;
+			this.metaData = metaData;
+			this.fileIndex = fileIndex;
+			this.symlinkPath = symlinkPath;
+		}
 	}
 
 	protected Map<GFile, FileData<METADATATYPE>> fileToEntryMap = new HashMap<>();
@@ -58,18 +70,9 @@ public class FileSystemIndexHelper<METADATATYPE> {
 	 * back to the fs's {@link GFileSystem#getFSRL()} on a half-constructed filesystem.)
 	 */
 	public FileSystemIndexHelper(GFileSystem fs, FSRLRoot fsFSRL) {
-		this.rootDir = GFileImpl.fromFSRL(fs, null, fsFSRL.withPath("/"), true, -1);
-		initRootDir(null);
-	}
-
-	private void initRootDir(METADATATYPE metadata) {
-		FileData<METADATATYPE> fileData = new FileData<>();
-		fileData.file = rootDir;
-		fileData.fileIndex = -1;
-		fileData.metaData = metadata;
-
-		fileToEntryMap.put(rootDir, fileData);
-		directoryToListing.put(rootDir, new HashMap<>());
+		GFile rootGFile = GFileImpl.fromFSRL(fs, null, fsFSRL.withPath("/"), true, -1);
+		rootDir = new FileData<>(rootGFile, null, -1);
+		directoryToListing.put(rootDir.file, new HashMap<>());
 	}
 
 	/**
@@ -78,7 +81,7 @@ public class FileSystemIndexHelper<METADATATYPE> {
 	 * @return root {@link GFile} object.
 	 */
 	public GFile getRootDir() {
-		return rootDir;
+		return rootDir.file;
 	}
 
 	/**
@@ -87,6 +90,7 @@ public class FileSystemIndexHelper<METADATATYPE> {
 	public synchronized void clear() {
 		fileToEntryMap.clear();
 		directoryToListing.clear();
+		fileIndexToEntryMap.clear();
 	}
 
 	/**
@@ -113,14 +117,11 @@ public class FileSystemIndexHelper<METADATATYPE> {
 	 * Sets the associated metadata blob for the specified file.
 	 * 
 	 * @param f GFile to update
-	 * @param metaData new metdata blob
+	 * @param metaData new metadata blob
 	 * @throws IOException if unknown file
 	 */
 	public synchronized void setMetadata(GFile f, METADATATYPE metaData) throws IOException {
-		FileData<METADATATYPE> fileData = fileToEntryMap.get(f);
-		if ( fileData == null ) {
-			throw new IOException("Unknown file: " + f);
-		}
+		FileData<METADATATYPE> fileData = getFileData(f);
 		fileData.metaData = metaData;
 	}
 
@@ -150,7 +151,7 @@ public class FileSystemIndexHelper<METADATATYPE> {
 		return dirListing.values()
 				.stream()
 				.map(fd -> fd.file)
-				.collect(Collectors.toList());
+				.toList();
 	}
 
 	/**
@@ -174,19 +175,102 @@ public class FileSystemIndexHelper<METADATATYPE> {
 	 * @return {@link GFile} instance or null if no file was added to the index at that path
 	 */
 	public synchronized GFile lookup(GFile baseDir, String path, Comparator<String> nameComp) {
-		String[] nameparts = (path != null ? path : "").split("/");
-		GFile parent = lookupParent(baseDir, nameparts, false, nameComp);
-		if (parent == null) {
-			return null;
+		try {
+			return lookup(baseDir, path, false, nameComp);
 		}
-		String name = (nameparts.length > 0) ? nameparts[nameparts.length - 1] : null;
-		if (name == null || name.isEmpty()) {
-			return parent;
+		catch (IOException e) {
+			// shouldn't happen, fall thru
+		}
+		return null;
+	}
+
+	protected GFile lookup(GFile baseDir, String path, boolean followSymlinks,
+			Comparator<String> nameComp) throws IOException {
+		FileData<METADATATYPE> baseDirData = getFileData(baseDir);
+		FileData<METADATATYPE> fileData =
+			lookup(baseDirData, splitPath(path), -1, false, followSymlinks, 0, null, nameComp);
+		return (fileData != null) ? fileData.file : null;
+	}
+
+	protected FileData<METADATATYPE> lookup(FileData<METADATATYPE> baseDir, String[] nameparts,
+			int maxpart, boolean createIfMissing, boolean followSymlinks, int depth,
+			StringBuilder symlinkPathDebug, Comparator<String> nameComp) throws IOException {
+		symlinkPathDebug = Objects.requireNonNullElseGet(symlinkPathDebug, StringBuilder::new);
+		maxpart = maxpart < 0 ? nameparts.length : maxpart;
+
+		if (depth > MAX_SYMLINK_RECURSE_DEPTH) {
+			throw new IOException(
+				"Too many symlinks: %s, %s".formatted(symlinkPathDebug, Arrays.asList(nameparts)));
 		}
 
-		FileData<METADATATYPE> fileData =
-			lookupFileInDir(getDirectoryContents(parent, false), name, nameComp);
-		return (fileData != null) ? fileData.file : null;
+		symlinkPathDebug.append("[");
+		FileData<METADATATYPE> currentFile = Objects.requireNonNullElse(baseDir, rootDir);
+		for (int i = 0; i < maxpart && currentFile != null; i++) {
+			String name = nameparts[i];
+			symlinkPathDebug.append(i != 0 ? "," : "").append(name);
+			if (name.isEmpty()) {
+				continue;
+			}
+			if (followSymlinks) {
+				// otherwise "." and ".." are valid path elements that need to be matched exactly
+				if (".".equals(name)) {
+					continue;
+				}
+				if ("..".equals(name)) {
+					currentFile = getParentFileData(currentFile);
+					continue;
+				}
+			}
+
+			Map<String, FileData<METADATATYPE>> currentDirContents =
+				getDirectoryContents(currentFile.file, createIfMissing);
+			FileData<METADATATYPE> next = lookupFileInDir(currentDirContents, name, nameComp);
+			if (next == null && createIfMissing) {
+				next = doStoreMissingDir(name, currentFile.file);
+			}
+			if (next != null && next.symlinkPath != null && followSymlinks) {
+				next = lookup(currentFile, splitPath(next.symlinkPath), -1, createIfMissing,
+					followSymlinks, depth + 1, symlinkPathDebug, nameComp);
+			}
+			currentFile = next;
+		}
+
+		symlinkPathDebug.append("]");
+		return currentFile;
+	}
+
+	/**
+	 * If supplied file is a symlink, converts the supplied file into the targeted file, otherwise
+	 * just returns the original file.
+	 *   
+	 * @param file {@link GFile} to convert
+	 * @return symlink targeted {@link GFile}, or original file it not a symlink, or null if
+	 * symlink path was invalid or reached outside the bounds of this file system
+	 * @throws IOException if symlinks are nested too deeply
+	 */
+	public synchronized GFile resolveSymlinks(GFile file) throws IOException {
+		FileData<METADATATYPE> fd = getFileData(file);
+		if (fd.symlinkPath != null) {
+			fd = lookup(getParentFileData(fd), splitPath(fd.symlinkPath), -1, false, true, 0, null,
+				null);
+		}
+		return fd != null ? fd.file : null;
+	}
+
+	private FileData<METADATATYPE> getFileData(GFile f) throws IOException {
+		if (f == null) {
+			return rootDir;
+		}
+		FileData<METADATATYPE> fd = fileToEntryMap.get(f);
+		if (fd == null) {
+			throw new IOException("Unknown file: %s".formatted(f));
+		}
+		return fd;
+	}
+
+	private FileData<METADATATYPE> getParentFileData(FileData<METADATATYPE> file) {
+		GFile parentGFile = file.file.getParentFile();
+		return parentGFile != null ? fileToEntryMap.get(parentGFile) : null;
 	}
 
 	/**
@@ -213,12 +297,15 @@ public class FileSystemIndexHelper<METADATATYPE> {
 	public synchronized GFile storeFile(String path, long fileIndex, boolean isDirectory,
 			long length, METADATATYPE metadata) {
 
-		String[] nameparts = path.replaceAll("[\\\\]", "/").split("/");
-		GFile parent = lookupParent(rootDir, nameparts, true, null);
+		String[] nameparts = splitPath(path);
+		if (nameparts.length == 0) {
+			return rootDir.file;
+		}
 
+		GFile parent = lookupParent(nameparts, null);
 		String lastpart = nameparts[nameparts.length - 1];
 		FileData<METADATATYPE> fileData =
-			doStoreFile(lastpart, parent, fileIndex, isDirectory, length, metadata);
+			doStoreFile(lastpart, parent, fileIndex, isDirectory, length, null, metadata);
 		return fileData.file;
 	}
 
@@ -245,19 +332,84 @@ public class FileSystemIndexHelper<METADATATYPE> {
 	public synchronized GFile storeFileWithParent(String filename, GFile parent, long fileIndex,
 			boolean isDirectory, long length, METADATATYPE metadata) {
 		FileData<METADATATYPE> fileData =
-			doStoreFile(filename, parent, fileIndex, isDirectory, length, metadata);
+			doStoreFile(filename, parent, fileIndex, isDirectory, length, null, metadata);
+		return fileData.file;
+	}
+
+	/**
+	 * Creates and stores a file entry that is a symlink into in-memory indexes.
+	 * <p>
+	 * The string path will be normalized to forward slashes before being split into
+	 * directory components.
+	 * <p>
+	 * Filenames that are not unique in their directory will have a "[nnn]"
+	 * suffix added to the resultant GFile name, where nnn is the file's
+	 * order of occurrence in the container file.
+	 * <p>
+	 * 
+	 * @param path string path and filename of the file being added to the index.  Back
+	 * slashes are normalized to forward slashes
+	 * @param fileIndex the filesystem specific unique index for this file, or -1
+	 * if not available
+	 * @param symlinkPath destination of the symlink
+	 * @param length number of bytes in the file or -1 if not known or directory
+	 * @param metadata opaque blob that will be stored and associated with the new
+	 * GFile instance
+	 * @return new GFile instance
+	 */
+	public synchronized GFile storeSymlink(String path, long fileIndex, String symlinkPath,
+			long length, METADATATYPE metadata) {
+		String[] nameparts = splitPath(path);
+		if (nameparts.length == 0) {
+			Msg.warn(this,
+				"Unable to create invalid symlink file [%s] -> [%s]".formatted(path, symlinkPath));
+			return rootDir.file;
+		}
+		length = length != 0 ? length : symlinkPath.length();
+		GFile parent = lookupParent(nameparts, null);
+
+		String lastpart = nameparts[nameparts.length - 1];
+		FileData<METADATATYPE> fileData =
+			doStoreFile(lastpart, parent, fileIndex, false, length, symlinkPath, metadata);
+		return fileData.file;
+
+	}
+
+	/**
+	 * Creates and stores a file entry that is a symlink into in-memory indexes.
+	 * <p>
+	 * Use this when you already know the parent directory GFile object.
+	 * <p>
+	 * Filenames that are not unique in their directory will have a "[nnn]"
+	 * suffix added to the resultant GFile name, where nnn is the file's
+	 * order of occurrence in the container file.
+	 * <p>
+	 * 
+	 * @param filename the new file's name
+	 * @param parent the new file's parent directory
+	 * @param fileIndex the filesystem specific unique index for this file, or -1
+	 * if not available
+	 * @param symlinkPath destination of the symlink
+	 * @param length number of bytes in the file or -1 if not known or directory
+	 * @param metadata opaque blob that will be stored and associated with the new
+	 * GFile instance
+	 * @return new GFile instance
+	 */
+	public synchronized GFile storeSymlinkWithParent(String filename, GFile parent, long fileIndex,
+			String symlinkPath, long length, METADATATYPE metadata) {
+		length = length != 0 ? length : symlinkPath.length();
+		FileData<METADATATYPE> fileData =
+			doStoreFile(filename, parent, fileIndex, false, length, symlinkPath, metadata);
 		return fileData.file;
 	}
 
 	private FileData<METADATATYPE> doStoreMissingDir(String filename, GFile parent) {
-		parent = (parent == null) ? rootDir : parent;
+		parent = (parent == null) ? rootDir.file : parent;
 
 		Map<String, FileData<METADATATYPE>> dirContents = getDirectoryContents(parent, true);
 		GFile file = createNewFile(parent, filename, true, -1, null);
 
-		FileData<METADATATYPE> fileData = new FileData<>();
-		fileData.file = file;
-		fileData.fileIndex = -1;
+		FileData<METADATATYPE> fileData = new FileData<>(file, null, -1);
 		fileToEntryMap.put(file, fileData);
 		dirContents.put(filename, fileData);
 		getDirectoryContents(file, true);
@@ -266,11 +418,12 @@ public class FileSystemIndexHelper<METADATATYPE> {
 	}
 
 	private FileData<METADATATYPE> doStoreFile(String filename, GFile parent, long fileIndex,
-			boolean isDirectory, long length, METADATATYPE metadata) {
-		parent = (parent == null) ? rootDir : parent;
+			boolean isDirectory, long length, String symlinkPath, METADATATYPE metadata) {
+		parent = (parent == null) ? rootDir.file : parent;
 		long fileNum = (fileIndex != -1) ? fileIndex : fileToEntryMap.size();
 		if (fileIndexToEntryMap.containsKey(fileNum)) {
-			Msg.warn(this, "Duplicate fileNum for file " + parent.getPath() + "/" + filename);
+			Msg.warn(this, "Duplicate fileNum %d for file %s/%s".formatted(fileNum,
+				parent.getPath(), filename));
 		}
 
 		Map<String, FileData<METADATATYPE>> dirContents = getDirectoryContents(parent, true);
@@ -279,10 +432,7 @@ public class FileSystemIndexHelper<METADATATYPE> {
 
 		GFile file = createNewFile(parent, uniqueName, isDirectory, length, metadata);
 
-		FileData<METADATATYPE> fileData = new FileData<>();
-		fileData.file = file;
-		fileData.fileIndex = fileNum;
-		fileData.metaData = metadata;
+		FileData<METADATATYPE> fileData = new FileData<>(file, metadata, fileNum, symlinkPath);
 		fileToEntryMap.put(file, fileData);
 		fileIndexToEntryMap.put(fileNum, fileData);
 
@@ -297,13 +447,13 @@ public class FileSystemIndexHelper<METADATATYPE> {
 
 	private String makeUniqueFilename(boolean wasNameCollision, String filename, long fileIndex) {
 		return wasNameCollision
-				? filename + "[" + Long.toString(fileIndex) + "]"
+				? filename + "[%d]".formatted(fileIndex)
 				: filename;
 	}
 
 	private Map<String, FileData<METADATATYPE>> getDirectoryContents(GFile directoryFile,
 			boolean createIfMissing) {
-		directoryFile = (directoryFile != null) ? directoryFile : rootDir;
+		directoryFile = (directoryFile != null) ? directoryFile : rootDir.file;
 
 		Map<String, FileData<METADATATYPE>> dirContents = directoryToListing.get(directoryFile);
 		if (dirContents == null && createIfMissing) {
@@ -326,37 +476,28 @@ public class FileSystemIndexHelper<METADATATYPE> {
 	 * be represented as empty string elements in the nameparts array and will be skipped
 	 * as if they were not there.
 	 * <p>
-	 * @param baseDir optional starting directory to perform lookups
 	 * @param nameparts String[] containing the elements of a path
-	 * @param createIfMissing boolean flag, if true missing elements will have stub entries created
 	 * for them
 	 * @param nameComp optional comparator that will compare names, usually case-sensitive vs case
 	 * insensitive
-	 * @return GFile that represents the parent directory, or null if in read-only mode and not
-	 * found
+	 * @return GFile that represents the parent directory
 	 */
-	protected GFile lookupParent(GFile baseDir, String[] nameparts, boolean createIfMissing,
-			Comparator<String> nameComp) {
+	protected GFile lookupParent(String[] nameparts, Comparator<String> nameComp) {
 
-		GFile currentDir = Objects.requireNonNullElse(baseDir, rootDir);
-		for (int i = 0; i < nameparts.length - 1; i++) {
-			Map<String, FileData<METADATATYPE>> currentDirContents =
-				getDirectoryContents(currentDir, true);
-			String name = nameparts[i];
-			if (name.isEmpty()) {
-				continue;
-			}
-			FileData<METADATATYPE> fileData = lookupFileInDir(currentDirContents, name, nameComp);
-			if (fileData == null) {
-				if (!createIfMissing) {
-					return null;
-				}
-				fileData = doStoreMissingDir(name, currentDir);
-			}
-			currentDir = fileData.file;
+		try {
+			FileData<METADATATYPE> parent =
+				lookup(rootDir, nameparts, nameparts.length - 1, true, false, 0, null, nameComp);
+			return parent.file;
+		}
+		catch (IOException e) {
+			// fall thru, return rootdir
 		}
 
-		return currentDir;
+		return rootDir.file;
+	}
+
+	protected String[] splitPath(String path) {
+		return Objects.requireNonNullElse(path, "").replace('\\', '/').split("/");
 	}
 
 	protected FileData<METADATATYPE> lookupFileInDir(
@@ -400,8 +541,8 @@ public class FileSystemIndexHelper<METADATATYPE> {
 	protected GFileImpl createNewFile(GFile parentFile, String name, boolean isDirectory, long size,
 			METADATATYPE metadata) {
 		FSRL newFileFSRL = parentFile.getFSRL().appendPath(name);
-		return GFileImpl.fromFSRL(rootDir.getFilesystem(), parentFile, newFileFSRL, isDirectory,
-			size);
+		return GFileImpl.fromFSRL(rootDir.file.getFilesystem(), parentFile, newFileFSRL,
+			isDirectory, size);
 	}
 
 	/**
@@ -411,7 +552,7 @@ public class FileSystemIndexHelper<METADATATYPE> {
 	 * @param newFSRL the new FSRL the new file will be given
 	 */
 	public synchronized void updateFSRL(GFile file, FSRL newFSRL) {
-		GFileImpl newFile = GFileImpl.fromFSRL(rootDir.getFilesystem(), file.getParentFile(),
+		GFileImpl newFile = GFileImpl.fromFSRL(rootDir.file.getFilesystem(), file.getParentFile(),
 			newFSRL, file.isDirectory(), file.getLength());
 
 		FileData<METADATATYPE> fileData = fileToEntryMap.get(file);
@@ -438,7 +579,7 @@ public class FileSystemIndexHelper<METADATATYPE> {
 
 	@Override
 	public String toString() {
-		return "FileSystemIndexHelper for " + rootDir.getFilesystem();
+		return "FileSystemIndexHelper for " + rootDir.file.getFilesystem();
 	}
 
 }

Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/ext4/Ext4FileSystem.java

@@ -75,9 +75,8 @@ public class Ext4FileSystem extends AbstractFileSystem<Ext4File> {
 		monitor.setMessage("Reading inode tables");
 		Ext4GroupDescriptor[] groupDescriptors = new Ext4GroupDescriptor[numGroups];
 		for (int i = 0; i < numGroups; i++) {
-			monitor.checkCancelled();
 			groupDescriptors[i] = new Ext4GroupDescriptor(reader, superBlock.is64Bit());
-			monitor.incrementProgress(1);
+			monitor.increment();
 		}
 
 		Ext4Inode[] inodes = getInodes(reader, groupDescriptors, monitor);
@@ -125,12 +124,11 @@ public class Ext4FileSystem extends AbstractFileSystem<Ext4File> {
 		BinaryReader reader = new BinaryReader(directoryStream, true /* LE */);
 		Ext4DirEntry dirEnt;
 		while ((dirEnt = isdir2 ? Ext4DirEntry2.read(reader) : Ext4DirEntry.read(reader)) != null) {
-			monitor.checkCancelled();
 			if (dirEnt.isUnused()) {
 				continue;
 			}
 			processDirEntry(dirEnt, dirGFile, inodes, processedInodes, monitor);
-			monitor.incrementProgress(1);
+			monitor.increment();
 		}
 	}
 
@@ -147,8 +145,8 @@ public class Ext4FileSystem extends AbstractFileSystem<Ext4File> {
 			return;
 		}
 		if (!(inode.isDir() || inode.isFile() || inode.isSymLink())) {
-			throw new IOException("Inode " + inode + " has unhandled file type: " +
-				Integer.toHexString(inode.getFileType()));
+			throw new IOException("Inode %d has unhandled file type: 0x%x".formatted(inodeNumber,
+				inode.getFileType()));
 		}
 
 		String name = dirEntry.getName();
@@ -157,8 +155,11 @@ public class Ext4FileSystem extends AbstractFileSystem<Ext4File> {
 			return;
 		}
 
-		GFile gfile = fsIndex.storeFileWithParent(name, parentDir, -1, inode.isDir(),
-			inode.getSize(), new Ext4File(name, inode));
+		GFile gfile = !inode.isSymLink()
+				? fsIndex.storeFileWithParent(name, parentDir, -1, inode.isDir(), inode.getSize(),
+					new Ext4File(name, inode))
+				: fsIndex.storeSymlinkWithParent(name, parentDir, -1, readLink(inode, monitor),
+					inode.getSize(), new Ext4File(name, inode));
 		if (processedInodes.get(inodeNumber)) {
 			// this inode was already seen and handled earlier. adding a second filename to the fsih is
 			// okay, but don't try to process as a directory, which shouldn't normally be possible
@@ -184,7 +185,7 @@ public class Ext4FileSystem extends AbstractFileSystem<Ext4File> {
 			if (inode.isSymLink()) {
 				String symLinkDest = "unknown";
 				try {
-					symLinkDest = readLink(file, ext4File, monitor);
+					symLinkDest = readLink(inode, monitor);
 				}
 				catch (IOException e) {
 					// fall thru with default value
@@ -213,54 +214,14 @@ public class Ext4FileSystem extends AbstractFileSystem<Ext4File> {
 		return FileType.UNKNOWN;
 	}
 
-	private static final int MAX_SYMLINK_LOOKUP_COUNT = 100;
-
-	private Ext4Inode resolveSymLink(GFile file, TaskMonitor monitor) throws IOException {
-		int lookupCount = 0;
-		GFile currentFile = file;
-		StringBuilder symlinkDebugPath = new StringBuilder();
-		while (true) {
-			if (lookupCount++ > MAX_SYMLINK_LOOKUP_COUNT) {
-				throw new IOException(
-					"Symlink too long: " + file.getPath() + ", " + symlinkDebugPath);
-			}
-			Ext4File extFile = fsIndex.getMetadata(currentFile);
-			if (extFile == null) {
-				throw new IOException("Missing Ext4 metadata for " + currentFile.getPath());
-			}
-			Ext4Inode inode = extFile.getInode();
-			if (!inode.isSymLink()) {
-				return inode;
-			}
-			
-			String symlinkDestPath = readLink(file, extFile, monitor);
-			symlinkDebugPath.append(" -> ").append(symlinkDestPath);
-			if (!symlinkDestPath.startsWith("/")) {
-				if (currentFile.getParentFile() == null) {
-					throw new IOException("No parent file for " + currentFile);
-				}
-				// TODO: doesn't handle "../../" traversal yet
-				symlinkDestPath =
-					FSUtilities.appendPath(currentFile.getParentFile().getPath(),
-						symlinkDestPath);
-			}
-
-			currentFile = lookup(symlinkDestPath);
-			if (currentFile == null) {
-				throw new IOException("Missing symlink dest: " + file.getPath() +
-					", broken symlink: " + symlinkDebugPath);
-			}
-		}
-	}
-
-	private String readLink(GFile file, Ext4File extFile, TaskMonitor monitor) throws IOException {
-		try (ByteProvider bp = getInodeByteProvider(extFile.getInode(), file.getFSRL(), monitor)) {
+	private String readLink(Ext4Inode inode, TaskMonitor monitor) throws IOException {
+		try (ByteProvider bp = getInodeByteProvider(inode, null, monitor)) {
 			byte[] tmp = bp.readBytes(0, bp.length());
 			return new String(tmp, StandardCharsets.UTF_8);
 		}
 	}
 
-	private Ext4Inode getInodeFor(GFile file, TaskMonitor monitor) throws IOException {
+	private Ext4Inode getInodeFor(GFile file) {
 		Ext4File extFile = fsIndex.getMetadata(file);
 		if (extFile == null) {
 			return null;
@@ -270,9 +231,6 @@ public class Ext4FileSystem extends AbstractFileSystem<Ext4File> {
 			return null;
 		}
 
-		if (inode.isSymLink()) {
-			inode = resolveSymLink(file, monitor);
-		}
 		return inode;
 	}
 
@@ -287,8 +245,9 @@ public class Ext4FileSystem extends AbstractFileSystem<Ext4File> {
 	 */
 	@Override
 	public ByteProvider getByteProvider(GFile file, TaskMonitor monitor) throws IOException {
-		Ext4Inode inode = getInodeFor(file, monitor);
-		if (inode == null) {
+		file = fsIndex.resolveSymlinks(file);
+		Ext4Inode inode;
+		if (file == null || (inode = getInodeFor(file)) == null) {
 			return null;
 		}
 
@@ -329,11 +288,10 @@ public class Ext4FileSystem extends AbstractFileSystem<Ext4File> {
 			long offset = inodeTableBlockOffset * blockSize;
 			reader.setPointerIndex(offset);
 			monitor.setMessage(
-				"Reading inode table " + i + " of " + (groupDescriptors.length - 1) + "...");
+				"Reading inode table %d of %d...".formatted(i, groupDescriptors.length - 1));
 			monitor.initialize(inodesPerGroup);
 			for (int j = 0; j < inodesPerGroup; j++) {
-				monitor.checkCancelled();
-				monitor.incrementProgress(1);
+				monitor.increment();
 
 				Ext4Inode inode = new Ext4Inode(reader, superBlock.getS_inode_size());
 				offset = offset + superBlock.getS_inode_size();

Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/squashfs/SquashFileSystem.java

@@ -19,7 +19,8 @@ import static ghidra.formats.gfilesystem.fileinfo.FileAttributeType.*;
 
 import java.io.IOException;
 import java.io.OutputStream;
-import java.util.*;
+import java.util.Arrays;
+import java.util.Date;
 
 import ghidra.app.util.bin.BinaryReader;
 import ghidra.app.util.bin.ByteProvider;
@@ -106,7 +107,7 @@ public class SquashFileSystem extends AbstractFileSystem<SquashedFile> {
 			throws IOException, CancelledException {
 
 		// If the current file is a symlink, try to follow it
-		file = followSymLink(file, 0);
+		file = fsIndex.resolveSymlinks(file);
 
 		SquashedFile squashedFile = fsIndex.getMetadata(file);
 
@@ -141,106 +142,6 @@ public class SquashFileSystem extends AbstractFileSystem<SquashedFile> {
 		monitor.setProgress(totalUncompressedBytes);
 	}
 
-	/**
-	 * Given a GFile representing a symlink, return the destination GFile, recursing into referenced
-	 * symlinks as needed. If the given file is not a symlink, it will be returned
-	 * @param symLinkFile The file representing a symlink containing the target
-	 * @param depth The current recursion depth to prevent recursing too far
-	 * @return The destination file
-	 * @throws IOException Issues relating to locating a symlink target
-	 */
-	private GFile followSymLink(GFile symLinkFile, int depth) throws IOException {
-
-		// Check if a file was supplied properly
-		if (symLinkFile == null) {
-			return null;
-		}
-
-		// Get the path associated with the given symlink
-		String path = getSymLinkPath(symLinkFile);
-
-		// If path is null, then the given file is not a symlink and should be returned as the destination
-		if (path == null) {
-			return symLinkFile;
-		}
-
-		// Make sure to not follow symlinks too far
-		if (depth > SquashConstants.MAX_SYMLINK_DEPTH) {
-			throw new IOException("Did not find symlink destination after max traversal");
-		}
-
-		// Start with the parent at the root of the archive, as all paths will be absolute
-		GFile currentFile = symLinkFile.getParentFile();
-
-		// Split up the path into its parts
-		List<String> pathParts = new ArrayList<String>(Arrays.asList(path.split("/")));
-
-		// Future references to "." are redundant, so remove them along with any blank parts
-		pathParts.removeIf(part -> part.contentEquals(".") || part.isBlank());
-
-		// Iterate over all parts of the input path, removing portions as ".." appears
-		ListIterator<String> iterator = pathParts.listIterator();
-		while (iterator.hasNext()) {
-
-			// Get the next portion of the path
-			String currentPart = iterator.next();
-
-			// If the link references up a directory
-			if (currentPart.equals("..")) {
-
-				// Move up a directory
-				currentFile = currentFile.getParentFile();
-
-			}
-			else {
-
-				// Get the file representing the next portion of the path
-				currentFile = fsIndex.lookup(currentFile, currentPart, null);
-
-				// Determine if the current file is a symlink and follow it if so
-				currentFile = followSymLink(currentFile, depth + 1);
-			}
-
-			// Check if the lookup failed
-			if (currentFile == null) {
-				throw new IOException("Could not find file within the given parent directory");
-			}
-
-			// Keep track of the depth
-			depth++;
-		}
-
-		// Return GFile representing the destination of the symlink
-		return currentFile;
-	}
-
-	/**
-	 * If the given file is a symlink, return the path it points to (null if file is not a symlink)
-	 * @param file The file to check
-	 * @return The symlink path
-	 * @throws IOException There was no SquashedFile for the given file
-	 */
-	private String getSymLinkPath(GFile file) throws IOException {
-
-		// Get the associated SquashedFile and make sure it is not null
-		SquashedFile possibleSymLinkFile = fsIndex.getMetadata(file);
-		if (possibleSymLinkFile == null) {
-			throw new IOException("Cannot retrieve SquashedFile associated with the given file");
-		}
-
-		// Check if the current part is a symlink
-		if (possibleSymLinkFile.getInode().isSymLink()) {
-			// Get and convert the associated inode
-			SquashSymlinkInode symLinkInode = (SquashSymlinkInode) possibleSymLinkFile.getInode();
-
-			// Get the target path
-			return symLinkInode.getPath();
-		}
-
-		// If the file is not a symlink, return null
-		return null;
-	}
-
 	/**
 	 * Decompress (if needed) all data block associated with the given file and write to OutputStream
 	 * @param squashedFile The file to process

Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/squashfs/SquashUtils.java

@@ -273,10 +273,8 @@ public class SquashUtils {
 			// Treat as symbolic link inode
 			SquashSymlinkInode symLinkInode = (SquashSymlinkInode) inode;
 
-			// Store symlink as file. Lookup handled when getting ByteProvider
-			fsih.storeFileWithParent(entry.getFileName(), parentDir, symLinkInode.getNumber(),
-				false, symLinkInode.getPath().length(), new SquashedFile(symLinkInode, null));
-
+			fsih.storeSymlinkWithParent(entry.getFileName(), parentDir, symLinkInode.getNumber(),
+				symLinkInode.getPath(), 0, new SquashedFile(symLinkInode, null));
 		}
 		else {
 			Msg.info(SquashUtils.class,

Ghidra/Features/FileFormats/src/main/java/ghidra/file/formats/tar/TarFileSystem.java

@@ -68,10 +68,15 @@ public class TarFileSystem extends AbstractFileSystem<TarMetadata> {
 				monitor.checkCancelled();
 
 				int fileNum = fileCount++;
-				GFile newFile = fsIndex.storeFile(tarEntry.getName(), fileCount,
-					tarEntry.isDirectory(), tarEntry.getSize(), new TarMetadata(tarEntry, fileNum));
+				String linkName = tarEntry.getLinkName();
+				GFile newFile = !tarEntry.isSymbolicLink()
+						? fsIndex.storeFile(tarEntry.getName(), fileCount, tarEntry.isDirectory(),
+							tarEntry.getSize(), new TarMetadata(tarEntry, fileNum))
+						: fsIndex.storeSymlink(tarEntry.getName(), fileCount,
+							linkName, linkName.length(), new TarMetadata(tarEntry, fileNum));
 
-				if (tarEntry.getSize() < FileCache.MAX_INMEM_FILESIZE) {
+				if (!tarEntry.isSymbolicLink() &&
+					tarEntry.getSize() < FileCache.MAX_INMEM_FILESIZE) {
 					// because tar files are sequential access, we cache smaller files if they
 					// will fit in a in-memory ByteProvider
 					try (ByteProvider bp =
@@ -116,7 +121,10 @@ public class TarFileSystem extends AbstractFileSystem<TarMetadata> {
 			FileAttribute.create(GROUP_NAME_ATTR, blob.getGroupName()),
 			FileAttribute.create(USER_ID_ATTR, blob.getLongUserId()),
 			FileAttribute.create(GROUP_ID_ATTR, blob.getLongGroupId()),
-			FileAttribute.create(UNIX_ACL_ATTR, (long) blob.getMode()));
+			FileAttribute.create(UNIX_ACL_ATTR, (long) blob.getMode()),
+			blob.isSymbolicLink()
+					? FileAttribute.create(SYMLINK_DEST_ATTR, blob.getLinkName())
+					: null);
 	}
 
 	private FileType tarToFileType(TarArchiveEntry tae) {
@@ -136,13 +144,14 @@ public class TarFileSystem extends AbstractFileSystem<TarMetadata> {
 	public ByteProvider getByteProvider(GFile file, TaskMonitor monitor)
 			throws IOException, CancelledException {
 
-		TarMetadata tmd = fsIndex.getMetadata(file);
+		GFile resolvedFile = fsIndex.resolveSymlinks(file);
+		TarMetadata tmd = fsIndex.getMetadata(resolvedFile);
 		if (tmd == null) {
 			throw new IOException("Unknown file " + file);
 		}
 
-		ByteProvider fileBP = fsService.getDerivedByteProvider(provider.getFSRL(), file.getFSRL(),
-			file.getPath(), tmd.tarArchiveEntry.getSize(), () -> {
+		ByteProvider fileBP = fsService.getDerivedByteProvider(provider.getFSRL(),
+			resolvedFile.getFSRL(), resolvedFile.getPath(), tmd.tarArchiveEntry.getSize(), () -> {
 				TarArchiveInputStream tarInput = new TarArchiveInputStream(provider.getInputStream(0));
 
 				int fileNum = 0;
@@ -151,13 +160,13 @@ public class TarFileSystem extends AbstractFileSystem<TarMetadata> {
 					if (fileNum == tmd.fileNum) {
 						if (!tmd.tarArchiveEntry.getName().equals(tarEntry.getName())) {
 							throw new IOException(
-								"Mismatch between filenum and tarEntry for " + file);
+								"Mismatch between filenum and tarEntry for " + resolvedFile);
 						}
 						return tarInput;
 					}
 					fileNum++;
 				}
-				throw new IOException("Could not find requested file " + file);
+				throw new IOException("Could not find requested file " + resolvedFile);
 			}, monitor);
 
 		return fileBP;