From 0316a84b03151be971162fa50596b7abfcc73a9c Mon Sep 17 00:00:00 2001 From: Spiderbuttons Date: Sun, 22 Jun 2025 22:06:52 -0400 Subject: [PATCH 1/5] Enable single directory climbing to root content folder for tilesheet image sources in mod-loaded maps. --- .../ContentManagers/ModContentManager.cs | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs index f11ce9648..adb095565 100644 --- a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs @@ -413,8 +413,19 @@ private void FixTilesheetPaths(Map map, string relativeMapPath, bool fixEagerPat // validate tilesheet path string errorPrefix = $"{this.ModName} loaded map '{relativeMapPath}' with invalid tilesheet path '{imageSource}'."; - if (Path.IsPathRooted(imageSource) || PathUtilities.GetSegments(imageSource).Contains("..")) - throw new SContentLoadException(ContentLoadErrorType.InvalidData, $"{errorPrefix} Tilesheet paths must be a relative path without directory climbing (../)."); + if (Path.IsPathRooted(imageSource)) + throw new SContentLoadException(ContentLoadErrorType.InvalidData, $"{errorPrefix} Tilesheet paths must not be an absolute path ({Path.GetPathRoot(imageSource)})."); + + string[] imageSourcePathSegments = PathUtilities.GetSegments(imageSource); + if (imageSourcePathSegments.Contains("..")) + { + int climbingCount = imageSourcePathSegments.Count(segment => segment.Equals("..", StringComparison.OrdinalIgnoreCase)); + if (climbingCount > 1) + { + string[] offendingSegments = imageSourcePathSegments.TakeWhile(seg => seg.Equals("..")).ToArray(); + throw new SContentLoadException(ContentLoadErrorType.InvalidData, $"{errorPrefix} Tilesheet paths must not climb more than one directory ({string.Join('/', offendingSegments)}/)."); + } + } // load best match try @@ -467,8 +478,8 @@ private bool TryGetTilesheetAssetName(string modRelativeMapFolder, string relati relativePath = Path.Combine(Path.GetDirectoryName(relativePath) ?? "", filename.TrimStart('.')); } - // get relative to map file - { + // get relative to map file unless path has directory climbing + if (!PathUtilities.GetSegments(relativePath).Contains("..")) { string localKey = Path.Combine(modRelativeMapFolder, relativePath); if (this.GetModFile(localKey).Exists) { @@ -524,8 +535,12 @@ private string GetContentKeyForTilesheetImageSource(string relativePath) string key = relativePath; string topFolder = PathUtilities.GetSegments(key, limit: 2)[0]; - // convert image source relative to map file into asset key - if (!topFolder.Equals("Maps", StringComparison.OrdinalIgnoreCase)) + // remove previously validated directory climbing if it exists + if (topFolder.Equals("..", StringComparison.OrdinalIgnoreCase)) + key = key[2..]; + + // else convert image source relative to map file into asset key + else if (!topFolder.Equals("Maps", StringComparison.OrdinalIgnoreCase)) key = Path.Combine("Maps", key); // remove file extension from unpacked file From cb53d1861b452212cbe1d0be2eddd29c92962607 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Fri, 26 Dec 2025 13:03:25 -0500 Subject: [PATCH 2/5] avoid unneeded allocations --- .../ContentManagers/ModContentManager.cs | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs index adb095565..2ee2d6160 100644 --- a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs @@ -412,9 +412,8 @@ private void FixTilesheetPaths(Map map, string relativeMapPath, bool fixEagerPat imageSource = imageSource[(relativeMapFolder.Length + 1)..]; // validate tilesheet path - string errorPrefix = $"{this.ModName} loaded map '{relativeMapPath}' with invalid tilesheet path '{imageSource}'."; if (Path.IsPathRooted(imageSource)) - throw new SContentLoadException(ContentLoadErrorType.InvalidData, $"{errorPrefix} Tilesheet paths must not be an absolute path ({Path.GetPathRoot(imageSource)})."); + throw this.GetPathError(relativeMapFolder, imageSource, $"Tilesheet paths must not be an absolute path ({Path.GetPathRoot(imageSource)})."); string[] imageSourcePathSegments = PathUtilities.GetSegments(imageSource); if (imageSourcePathSegments.Contains("..")) @@ -422,8 +421,8 @@ private void FixTilesheetPaths(Map map, string relativeMapPath, bool fixEagerPat int climbingCount = imageSourcePathSegments.Count(segment => segment.Equals("..", StringComparison.OrdinalIgnoreCase)); if (climbingCount > 1) { - string[] offendingSegments = imageSourcePathSegments.TakeWhile(seg => seg.Equals("..")).ToArray(); - throw new SContentLoadException(ContentLoadErrorType.InvalidData, $"{errorPrefix} Tilesheet paths must not climb more than one directory ({string.Join('/', offendingSegments)}/)."); + var offendingSegments = imageSourcePathSegments.TakeWhile(seg => seg.Equals("..")); + throw this.GetPathError(relativeMapFolder, imageSource, $"Tilesheet paths must not climb more than one directory ({string.Join('/', offendingSegments)}/)."); } } @@ -431,7 +430,7 @@ private void FixTilesheetPaths(Map map, string relativeMapPath, bool fixEagerPat try { if (!this.TryGetTilesheetAssetName(relativeMapFolder, imageSource, out IAssetName? assetName, out string? error)) - throw new SContentLoadException(ContentLoadErrorType.InvalidData, $"{errorPrefix} {error}"); + throw this.GetPathError(relativeMapFolder, imageSource, error); if (assetName is not null) { @@ -446,7 +445,7 @@ private void FixTilesheetPaths(Map map, string relativeMapPath, bool fixEagerPat if (ex is SContentLoadException) throw; - throw new SContentLoadException(ContentLoadErrorType.InvalidData, $"{errorPrefix} The tilesheet couldn't be loaded.", ex); + throw this.GetPathError(relativeMapFolder, imageSource, "The tilesheet couldn't be loaded.", ex); } } } @@ -455,10 +454,10 @@ private void FixTilesheetPaths(Map map, string relativeMapPath, bool fixEagerPat /// The folder path containing the map, relative to the mod folder. /// The tilesheet path to load. /// The found asset name. - /// A message indicating why the file couldn't be loaded. + /// A message indicating why the file couldn't be loaded, if applicable. /// Returns whether the asset name was found. /// See remarks on . - private bool TryGetTilesheetAssetName(string modRelativeMapFolder, string relativePath, out IAssetName? assetName, out string? error) + private bool TryGetTilesheetAssetName(string modRelativeMapFolder, string relativePath, out IAssetName? assetName, [NotNullWhen(false)] out string? error) { error = null; @@ -479,7 +478,8 @@ private bool TryGetTilesheetAssetName(string modRelativeMapFolder, string relati } // get relative to map file unless path has directory climbing - if (!PathUtilities.GetSegments(relativePath).Contains("..")) { + if (!PathUtilities.GetSegments(relativePath).Contains("..")) + { string localKey = Path.Combine(modRelativeMapFolder, relativePath); if (this.GetModFile(localKey).Exists) { @@ -549,4 +549,14 @@ private string GetContentKeyForTilesheetImageSource(string relativePath) return key; } + + /// Get an exception indicating that a map asset can't be loaded because one of its tilesheets has an invalid path. + /// The relative path to the folder which contains the map asset, relative to the mod folder. + /// The tilesheet's path to the image file. + /// The error message indicating why loading it failed. (Basic metadata like the image source is prepended automatically.) + /// The inner exception which caused the error, if applicable. + private SContentLoadException GetPathError(string relativeMapPath, string imageSource, string error, Exception? ex = null) + { + return new SContentLoadException(ContentLoadErrorType.InvalidData, $"{this.ModName} loaded map '{relativeMapPath}' with invalid tilesheet path '{imageSource}'. {error}", ex); + } } From 28bc30953d3b1faeda5febdb1c0228c6fa4768b2 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Fri, 26 Dec 2025 13:03:25 -0500 Subject: [PATCH 3/5] avoid unneeded case-insensitive equals for ".." --- .../Framework/ContentManagers/ModContentManager.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs index 2ee2d6160..66672f239 100644 --- a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs @@ -415,14 +415,16 @@ private void FixTilesheetPaths(Map map, string relativeMapPath, bool fixEagerPat if (Path.IsPathRooted(imageSource)) throw this.GetPathError(relativeMapFolder, imageSource, $"Tilesheet paths must not be an absolute path ({Path.GetPathRoot(imageSource)})."); - string[] imageSourcePathSegments = PathUtilities.GetSegments(imageSource); - if (imageSourcePathSegments.Contains("..")) { - int climbingCount = imageSourcePathSegments.Count(segment => segment.Equals("..", StringComparison.OrdinalIgnoreCase)); - if (climbingCount > 1) + string[] imageSourcePathSegments = PathUtilities.GetSegments(imageSource); + if (imageSourcePathSegments.Contains("..")) { - var offendingSegments = imageSourcePathSegments.TakeWhile(seg => seg.Equals("..")); - throw this.GetPathError(relativeMapFolder, imageSource, $"Tilesheet paths must not climb more than one directory ({string.Join('/', offendingSegments)}/)."); + int climbingCount = imageSourcePathSegments.Count(segment => segment == ".."); + if (climbingCount > 1) + { + var offendingSegments = imageSourcePathSegments.TakeWhile(segment => segment == ".."); + throw this.GetPathError(relativeMapFolder, imageSource, $"Tilesheet paths must not climb more than one directory ({string.Join('/', offendingSegments)}/)."); + } } } From d9ccf08b71b9c223bbf042d5b4e30e71cdd86298 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Fri, 26 Dec 2025 13:03:25 -0500 Subject: [PATCH 4/5] adjust validation to account for climbing after first path segment --- .../ContentManagers/ModContentManager.cs | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs index 66672f239..87e2809c4 100644 --- a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs @@ -399,32 +399,33 @@ private void FixTilesheetPaths(Map map, string relativeMapPath, bool fixEagerPat this.Monitor.VerboseLog($"Fixing tilesheet paths for map '{relativeMapPath}' from mod '{this.ModName}'..."); foreach (TileSheet tilesheet in map.TileSheets) { - // get image source + // get image path tilesheet.ImageSource = this.NormalizePathSeparators(tilesheet.ImageSource); string imageSource = tilesheet.ImageSource; + if (fixEagerPathPrefixes && relativeMapFolder.Length > 0 && imageSource?.StartsWith(relativeMapFolder) is true) + imageSource = imageSource[(relativeMapFolder.Length + 1)..]; - // validate image source + // ensure path isn't empty if (string.IsNullOrWhiteSpace(imageSource)) throw new SContentLoadException(ContentLoadErrorType.InvalidData, $"{this.ModName} loaded map '{relativeMapPath}' with invalid tilesheet '{tilesheet.Id}'. This tilesheet has no image source."); - // reverse incorrect eager tilesheet path prefixing - if (fixEagerPathPrefixes && relativeMapFolder.Length > 0 && imageSource.StartsWith(relativeMapFolder)) - imageSource = imageSource[(relativeMapFolder.Length + 1)..]; - - // validate tilesheet path + // ensure path is relative if (Path.IsPathRooted(imageSource)) throw this.GetPathError(relativeMapFolder, imageSource, $"Tilesheet paths must not be an absolute path ({Path.GetPathRoot(imageSource)})."); + // ensure any directory climbing is valid + // Tilesheet paths are relative to either the `Content/Maps` folder or the map file. Directory climbing is + // restricted for safety and simplicity: + // 1. Can only climb at the start of the path (e.g. `../LooseSprites/Cursors` but not `Mines/../Barn`). + // 2. Can only climb once (to avoid escaping the `Content` folder). + // 3. Always relative to the `Content/Maps` folder (not the map file). { - string[] imageSourcePathSegments = PathUtilities.GetSegments(imageSource); - if (imageSourcePathSegments.Contains("..")) + const string climbSegment = ".."; + string[] pathSegments = PathUtilities.GetSegments(imageSource); + if (pathSegments.Contains(climbSegment)) { - int climbingCount = imageSourcePathSegments.Count(segment => segment == ".."); - if (climbingCount > 1) - { - var offendingSegments = imageSourcePathSegments.TakeWhile(segment => segment == ".."); - throw this.GetPathError(relativeMapFolder, imageSource, $"Tilesheet paths must not climb more than one directory ({string.Join('/', offendingSegments)}/)."); - } + if (pathSegments[0] != climbSegment || pathSegments.Count(segment => segment == climbSegment) > 1) + throw this.GetPathError(relativeMapFolder, imageSource, $"Directory climbing ({climbSegment}/) is only permitted once at the start of the path."); } } @@ -480,7 +481,7 @@ private bool TryGetTilesheetAssetName(string modRelativeMapFolder, string relati } // get relative to map file unless path has directory climbing - if (!PathUtilities.GetSegments(relativePath).Contains("..")) + if (!relativePath.StartsWith("..") && PathUtilities.GetSegments(relativePath, 2)[0] != "..") // directory climbing can only be at the start of the path { string localKey = Path.Combine(modRelativeMapFolder, relativePath); if (this.GetModFile(localKey).Exists) From 29deb0a019efd211cf5c3328fc7f83ef9f6c6ce2 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Fri, 26 Dec 2025 13:25:18 -0500 Subject: [PATCH 5/5] add constant for climb segment to simplify tracking related logic --- .../ContentManagers/ModContentManager.cs | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs index 87e2809c4..52287445a 100644 --- a/src/SMAPI/Framework/ContentManagers/ModContentManager.cs +++ b/src/SMAPI/Framework/ContentManagers/ModContentManager.cs @@ -30,6 +30,9 @@ internal sealed class ModContentManager : BaseContentManager /********* ** Fields *********/ + /// A path segment which navigates to the parent directory. + private const string DirectoryClimbingPathSegment = ".."; + /// Encapsulates SMAPI's JSON file parsing. private readonly JsonHelper JsonHelper; @@ -420,7 +423,7 @@ private void FixTilesheetPaths(Map map, string relativeMapPath, bool fixEagerPat // 2. Can only climb once (to avoid escaping the `Content` folder). // 3. Always relative to the `Content/Maps` folder (not the map file). { - const string climbSegment = ".."; + const string climbSegment = ModContentManager.DirectoryClimbingPathSegment; string[] pathSegments = PathUtilities.GetSegments(imageSource); if (pathSegments.Contains(climbSegment)) { @@ -481,7 +484,8 @@ private bool TryGetTilesheetAssetName(string modRelativeMapFolder, string relati } // get relative to map file unless path has directory climbing - if (!relativePath.StartsWith("..") && PathUtilities.GetSegments(relativePath, 2)[0] != "..") // directory climbing can only be at the start of the path + const string climbSegment = ModContentManager.DirectoryClimbingPathSegment; + if (!relativePath.StartsWith(climbSegment) && PathUtilities.GetSegments(relativePath, 2)[0] != climbSegment) // directory climbing can only be at the start of the path { string localKey = Path.Combine(modRelativeMapFolder, relativePath); if (this.GetModFile(localKey).Exists) @@ -536,15 +540,17 @@ private bool GetContentFolderFileExists(string key) private string GetContentKeyForTilesheetImageSource(string relativePath) { string key = relativePath; - string topFolder = PathUtilities.GetSegments(key, limit: 2)[0]; - // remove previously validated directory climbing if it exists - if (topFolder.Equals("..", StringComparison.OrdinalIgnoreCase)) - key = key[2..]; + // make path relative to Content folder + { + string topFolder = PathUtilities.GetSegments(key, limit: 2)[0]; + const string climbSegment = ModContentManager.DirectoryClimbingPathSegment; - // else convert image source relative to map file into asset key - else if (!topFolder.Equals("Maps", StringComparison.OrdinalIgnoreCase)) - key = Path.Combine("Maps", key); + if (topFolder == climbSegment) + key = key[(climbSegment.Length + 1)..]; + else if (!topFolder.Equals("Maps", StringComparison.OrdinalIgnoreCase)) + key = Path.Combine("Maps", key); + } // remove file extension from unpacked file if (key.EndsWith(".png", StringComparison.OrdinalIgnoreCase))