Merge #4194 Tests for safety of string format calls

CHANGELOG.md

@@ -87,6 +87,7 @@ All notable changes to this project will be documented in this file.
 - [Netkan] Inflation error for multi-hosted mods with mismatched versions (#4178 by: HebaruSan)
 - [Multiple] Include version in useragent string, make it readonly (#4184, #4185 by: HebaruSan)
 - [Policy] indexing policy: add link to the code of conduct (#4183 by: JonnyOThan; reviewed: HebaruSan)
+- [Build] Tests for safety of string format calls (#4194 by: HebaruSan; reviewed: techman83)
 
 ## v1.34.4 (Niven)
 

Cmdline/Action/Available.cs

@@ -22,8 +22,9 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options)
                 .Where(m => !m.IsDLC)
                 .OrderBy(m => m.identifier);
 
-            user.RaiseMessage(string.Format(Properties.Resources.AvailableHeader,
-                instance.game.ShortName, instance.Version()));
+            user.RaiseMessage(Properties.Resources.AvailableHeader,
+                              instance.game.ShortName,
+                              instance.Version()?.ToString() ?? "");
             user.RaiseMessage("");
 
             if (opts.detail)

Cmdline/Action/Cache.cs

@@ -224,7 +224,7 @@ private int ShowCacheSizeLimit()
             IConfiguration cfg = ServiceLocator.Container.Resolve<IConfiguration>();
             if (cfg.CacheSizeLimit.HasValue)
             {
-                user?.RaiseMessage(CkanModule.FmtSize(cfg.CacheSizeLimit.Value));
+                user?.RaiseMessage("{0}", CkanModule.FmtSize(cfg.CacheSizeLimit.Value));
             }
             else
             {
@@ -258,7 +258,7 @@ private void PrintUsage(string verb)
         {
             foreach (var h in CacheSubOptions.GetHelp(verb))
             {
-                user?.RaiseError(h);
+                user?.RaiseError("{0}", h);
             }
         }
 

Cmdline/Action/Compare.cs

@@ -27,7 +27,7 @@ public int RunCommand(object rawOptions)
 
                 if (options.machine_readable)
                 {
-                    user.RaiseMessage(compareResult.ToString());
+                    user.RaiseMessage("{0}", compareResult.ToString());
                 }
                 else if (compareResult == 0)
                 {
@@ -51,7 +51,7 @@ public int RunCommand(object rawOptions)
                 user.RaiseError(Properties.Resources.ArgumentMissing);
                 foreach (var h in Actions.GetHelp("compare"))
                 {
-                    user.RaiseError(h);
+                    user.RaiseError("{0}", h);
                 }
                 return Exit.BADOPT;
             }

Cmdline/Action/Compat.cs

@@ -295,7 +295,7 @@ private void PrintUsage(string verb)
         {
             foreach (var h in CompatSubOptions.GetHelp(verb))
             {
-                user?.RaiseError(h);
+                user?.RaiseError("{0}", h);
             }
         }
 

Cmdline/Action/Filter.cs

@@ -224,7 +224,7 @@ private void PrintUsage(string verb)
         {
             foreach (var h in FilterSubOptions.GetHelp(verb))
             {
-                user?.RaiseError(h);
+                user?.RaiseError("{0}", h);
             }
         }
 

Cmdline/Action/GameInstance.cs

@@ -690,7 +690,7 @@ private void PrintUsage(string verb)
         {
             foreach (var h in InstanceSubOptions.GetHelp(verb))
             {
-                user?.RaiseError(h);
+                user?.RaiseError("{0}", h);
             }
         }
 

Cmdline/Action/Import.cs

@@ -43,7 +43,7 @@ public int RunCommand(CKAN.GameInstance instance, object options)
                     user.RaiseError(Properties.Resources.ArgumentMissing);
                     foreach (var h in Actions.GetHelp("import"))
                     {
-                        user.RaiseError(h);
+                        user.RaiseError("{0}", h);
                     }
                     return Exit.ERROR;
                 }

Cmdline/Action/Install.cs

@@ -38,7 +38,7 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options)
                 user.RaiseError(Properties.Resources.ArgumentMissing);
                 foreach (var h in Actions.GetHelp("install"))
                 {
-                    user.RaiseError(h);
+                    user.RaiseError("{0}", h);
                 }
                 return Exit.BADOPT;
             }
@@ -164,7 +164,7 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options)
                     }
                     catch (Kraken e)
                     {
-                        user.RaiseMessage(e.Message);
+                        user.RaiseMessage("{0}", e.Message);
                         return Exit.ERROR;
                     }
 

Cmdline/Action/Mark.cs

@@ -147,7 +147,7 @@ private static void PrintUsage(IUser user, string verb)
         {
             foreach (var h in MarkSubOptions.GetHelp(verb))
             {
-                user.RaiseError(h);
+                user.RaiseError("{0}", h);
             }
         }
 

Cmdline/Action/Remove.cs

@@ -113,7 +113,7 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options)
                 user.RaiseError(Properties.Resources.ArgumentMissing);
                 foreach (var h in Actions.GetHelp("remove"))
                 {
-                    user.RaiseError(h);
+                    user.RaiseError("{0}", h);
                 }
                 return Exit.BADOPT;
             }

Cmdline/Action/Replace.cs

@@ -30,7 +30,7 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options)
                 user.RaiseError(Properties.Resources.ArgumentMissing);
                 foreach (var h in Actions.GetHelp("replace"))
                 {
-                    user.RaiseError(h);
+                    user.RaiseError("{0}", h);
                 }
                 return Exit.BADOPT;
             }

Cmdline/Action/Repo.cs

@@ -451,7 +451,7 @@ private void PrintUsage(string verb)
         {
             foreach (var h in RepoSubOptions.GetHelp(verb))
             {
-                user?.RaiseError(h);
+                user?.RaiseError("{0}", h);
             }
         }
 

Cmdline/Action/Search.cs

@@ -27,7 +27,7 @@ public int RunCommand(CKAN.GameInstance ksp, object raw_options)
                 user.RaiseError(Properties.Resources.ArgumentMissing);
                 foreach (var h in Actions.GetHelp("search"))
                 {
-                    user.RaiseError(h);
+                    user.RaiseError("{0}", h);
                 }
                 return Exit.BADOPT;
             }
@@ -117,7 +117,7 @@ public int RunCommand(CKAN.GameInstance ksp, object raw_options)
 
                 foreach (CkanModule mod in matching)
                 {
-                    user.RaiseMessage(mod.identifier);
+                    user.RaiseMessage("{0}", mod.identifier);
                 }
             }
 

Cmdline/Action/Show.cs

@@ -25,7 +25,7 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options)
                 user.RaiseError(Properties.Resources.ArgumentMissing);
                 foreach (var h in Actions.GetHelp("show"))
                 {
-                    user.RaiseError(h);
+                    user.RaiseError("{0}", h);
                 }
                 return Exit.BADOPT;
             }

Cmdline/Action/Update.cs

@@ -96,7 +96,7 @@ public int RunCommand(object raw_options)
             catch (MissingCertificateKraken kraken)
             {
                 // Handling the kraken means we have prettier output.
-                user.RaiseMessage(kraken.ToString());
+                user.RaiseMessage("{0}", kraken.ToString());
                 return Exit.ERROR;
             }
 
@@ -162,7 +162,7 @@ private void PrintModules(string message, IEnumerable<CkanModule> modules)
                 throw new BadCommandKraken("List of modules cannot be null.");
             }
 
-            user.RaiseMessage(message);
+            user.RaiseMessage("{0}", message);
 
             foreach (CkanModule module in modules)
             {

Cmdline/Action/Upgrade.cs

@@ -50,7 +50,7 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options)
                 user.RaiseError(Properties.Resources.ArgumentMissing);
                 foreach (var h in Actions.GetHelp("upgrade"))
                 {
-                    user.RaiseError(h);
+                    user.RaiseError("{0}", h);
                 }
                 return Exit.BADOPT;
             }
@@ -93,7 +93,7 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options)
                                           latestVersion?.ToString() ?? "");
                         if (update.ReleaseNotes != null)
                         {
-                            user.RaiseMessage(update.ReleaseNotes);
+                            user.RaiseMessage("{0}", update.ReleaseNotes);
                         }
                         user.RaiseMessage("");
                         user.RaiseMessage("");
@@ -157,7 +157,7 @@ public int RunCommand(CKAN.GameInstance instance, object raw_options)
             }
             catch (InconsistentKraken kraken)
             {
-                user.RaiseMessage(kraken.ToString());
+                user.RaiseMessage("{0}", kraken.ToString());
                 return Exit.ERROR;
             }
             catch (ModuleIsDLCKraken kraken)

Cmdline/Main.cs

@@ -317,7 +317,7 @@ private static int ConsoleUi(GameInstanceManager manager, ConsoleUIOptions opts)
 
         private static int Version(IUser user)
         {
-            user.RaiseMessage(Meta.GetVersion(VersionFormat.Full));
+            user.RaiseMessage("{0}", Meta.GetVersion(VersionFormat.Full));
 
             return Exit.OK;
         }

ConsoleUI/CKAN-ConsoleUI.csproj

@@ -38,6 +38,7 @@
     <Reference Include="System.Core" />
     <Reference Include="System.ServiceModel" />
     <Reference Include="System.Transactions" />
+    <PackageReference Include="StringSyntaxAttribute" Version="1.0.0" />
   </ItemGroup>
   <ItemGroup Condition=" '$(TargetFramework)' == 'net8.0' ">
     <PackageReference Include="System.ServiceModel.Primitives" Version="6.2.0" />

ConsoleUI/GameInstanceListScreen.cs

@@ -94,7 +94,7 @@ public GameInstanceListScreen(ConsoleTheme          theme,
                         } catch (Exception ex) {
                             // This can throw if the previous current instance had an error,
                             // since it gets destructed when it's replaced.
-                            RaiseError(ex.Message);
+                            RaiseError("{0}", ex.Message);
                         }
                         return false;
                     } else {

ConsoleUI/InstallScreen.cs

@@ -123,9 +123,9 @@ public override void Run(Action? process = null)
                             // Don't need to tell the user they just cancelled out.
                         } catch (FileNotFoundKraken ex) {
                             // Possible file corruption
-                            RaiseError(ex.Message);
+                            RaiseError("{0}", ex.Message);
                         } catch (DirectoryNotFoundKraken ex) {
-                            RaiseError(ex.Message);
+                            RaiseError("{0}", ex.Message);
                         } catch (FileExistsKraken ex) {
                             if (ex.owningModule != null) {
                                 RaiseMessage(Properties.Resources.InstallOwnedFileConflict, ex.installingModule?.ToString() ?? "", ex.filename, ex.owningModule);
@@ -134,9 +134,9 @@ public override void Run(Action? process = null)
                             }
                             RaiseError(Properties.Resources.InstallFilesReverted);
                         } catch (DownloadErrorsKraken ex) {
-                            RaiseError(ex.ToString());
+                            RaiseError("{0}", ex.ToString());
                         } catch (ModuleDownloadErrorsKraken ex) {
-                            RaiseError(ex.ToString());
+                            RaiseError("{0}", ex.ToString());
                         } catch (DownloadThrottledKraken ex) {
                             if (RaiseYesNoDialog(string.Format(Properties.Resources.InstallAuthTokenPrompt, ex.ToString()))) {
                                 if (ex.infoUrl != null) {
@@ -145,9 +145,9 @@ public override void Run(Action? process = null)
                                 LaunchSubScreen(new AuthTokenScreen(theme));
                             }
                         } catch (MissingCertificateKraken ex) {
-                            RaiseError(ex.ToString());
+                            RaiseError("{0}", ex.ToString());
                         } catch (InconsistentKraken ex) {
-                            RaiseError(ex.Message);
+                            RaiseError("{0}", ex.Message);
                         } catch (TooManyModsProvideKraken ex) {
 
                             var ch = new ConsoleChoiceDialog<CkanModule>(
@@ -174,7 +174,7 @@ public override void Run(Action? process = null)
                         } catch (ModNotInstalledKraken ex) {
                             RaiseError(Properties.Resources.InstallNotInstalled, ex.mod);
                         } catch (DllLocationMismatchKraken ex) {
-                            RaiseError(ex.Message);
+                            RaiseError("{0}", ex.Message);
                         }
                     } while (retry);
                 }

ConsoleUI/ModListScreen.cs

@@ -455,7 +455,7 @@ private bool ViewSuggestions()
                     RaiseError(Properties.Resources.ModListAuditNotFound);
                 }
             } catch (ModuleNotFoundKraken k) {
-                RaiseError($"{k.module} {k.version}: {k.Message}");
+                RaiseError("{0} {1}: {2}", k.module, k.version ?? "", k.Message);
             }
             return true;
         }
@@ -496,7 +496,7 @@ private bool UpdateRegistry(bool showNewModsPrompt = true)
                                         userAgent);
                     } catch (Exception ex) {
                         // There can be errors while you re-install mods with changed metadata
-                        ps.RaiseError(ex.Message + ex.StackTrace);
+                        ps.RaiseError("{0}", ex.Message + ex.StackTrace);
                     }
                     // Update recent with mods that were updated in this pass
                     foreach (CkanModule mod in registry.CompatibleModules(
@@ -517,11 +517,9 @@ private bool UpdateRegistry(bool showNewModsPrompt = true)
         }
 
         private static string newModPrompt(int howMany)
-        {
-            return howMany == 1
+            => howMany == 1
                 ? string.Format(Properties.Resources.ModListNewMod,  howMany)
                 : string.Format(Properties.Resources.ModListNewMods, howMany);
-        }
 
         private bool ScanForMods()
         {

ConsoleUI/Toolkit/ConsoleScreen.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Diagnostics.CodeAnalysis;
 
 namespace CKAN.ConsoleUI.Toolkit {
 
@@ -141,7 +142,8 @@ public int RaiseSelectionDialog(string message, params object[] args)
         /// </summary>
         /// <param name="message">Format string for the message</param>
         /// <param name="args">Values to be interpolated into the format string</param>
-        public void RaiseError(string message, params object[] args)
+        public void RaiseError([StringSyntax(StringSyntaxAttribute.CompositeFormat)]
+                               string message, params object[] args)
         {
             var d = new ConsoleMessageDialog(
                 theme,
@@ -159,7 +161,8 @@ public void RaiseError(string message, params object[] args)
         /// </summary>
         /// <param name="message">Format string for the message</param>
         /// <param name="args">Values to be interpolated into the format string</param>
-        public void RaiseMessage(string message, params object[] args)
+        public void RaiseMessage([StringSyntax(StringSyntaxAttribute.CompositeFormat)]
+                                 string message, params object[] args)
         {
             Message(message, args);
             Draw();

Core/CKAN-core.csproj

@@ -59,6 +59,7 @@
     <PackageReference Include="Mono.Cecil" Version="0.11.5" />
     <PackageReference Include="Nullable" Version="1.3.1" PrivateAssets="all" />
     <PackageReference Include="IndexRange" Version="1.0.3" />
+    <PackageReference Include="StringSyntaxAttribute" Version="1.0.0" />
   </ItemGroup>
   <ItemGroup>
     <Compile Include="..\_build\meta\GlobalAssemblyVersionInfo.cs">

Core/Net/NetAsyncDownloader.cs

@@ -62,7 +62,7 @@ public static void DownloadWithProgress(IList<DownloadTarget> downloadTargets,
             {
                 if (error != null)
                 {
-                    user?.RaiseError(error.ToString());
+                    user?.RaiseError("{0}", error.ToString());
                 }
             };
             downloader.DownloadAndWait(downloadTargets);

Core/Net/NetAsyncModulesDownloader.cs

@@ -161,7 +161,7 @@ private void ModuleDownloadComplete(NetAsyncDownloader.DownloadTarget target,
                     }
                     catch (InvalidModuleFileKraken kraken)
                     {
-                        User.RaiseError(kraken.ToString());
+                        User.RaiseError("{0}", kraken.ToString());
                         if (module != null)
                         {
                             // Finish out the progress bar
@@ -176,7 +176,7 @@ private void ModuleDownloadComplete(NetAsyncDownloader.DownloadTarget target,
                     catch (OperationCanceledException exc)
                     {
                         log.WarnFormat("Cancellation token threw, validation incomplete: {0}", filename);
-                        User.RaiseMessage(exc.Message);
+                        User.RaiseMessage("{0}", exc.Message);
                         if (module != null)
                         {
                             // Finish out the progress bar