Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests for safety of string format calls #4194

Merged
merged 1 commit into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Cmdline/Action/Available.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions Cmdline/Action/Cache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -258,7 +258,7 @@ private void PrintUsage(string verb)
{
foreach (var h in CacheSubOptions.GetHelp(verb))
{
user?.RaiseError(h);
user?.RaiseError("{0}", h);
}
}

Expand Down
4 changes: 2 additions & 2 deletions Cmdline/Action/Compare.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion Cmdline/Action/Compat.cs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ private void PrintUsage(string verb)
{
foreach (var h in CompatSubOptions.GetHelp(verb))
{
user?.RaiseError(h);
user?.RaiseError("{0}", h);
}
}

Expand Down
2 changes: 1 addition & 1 deletion Cmdline/Action/Filter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ private void PrintUsage(string verb)
{
foreach (var h in FilterSubOptions.GetHelp(verb))
{
user?.RaiseError(h);
user?.RaiseError("{0}", h);
}
}

Expand Down
2 changes: 1 addition & 1 deletion Cmdline/Action/GameInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ private void PrintUsage(string verb)
{
foreach (var h in InstanceSubOptions.GetHelp(verb))
{
user?.RaiseError(h);
user?.RaiseError("{0}", h);
}
}

Expand Down
2 changes: 1 addition & 1 deletion Cmdline/Action/Import.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions Cmdline/Action/Install.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion Cmdline/Action/Mark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion Cmdline/Action/Remove.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion Cmdline/Action/Replace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion Cmdline/Action/Repo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ private void PrintUsage(string verb)
{
foreach (var h in RepoSubOptions.GetHelp(verb))
{
user?.RaiseError(h);
user?.RaiseError("{0}", h);
}
}

Expand Down
4 changes: 2 additions & 2 deletions Cmdline/Action/Search.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion Cmdline/Action/Show.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions Cmdline/Action/Update.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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)
{
Expand Down
6 changes: 3 additions & 3 deletions Cmdline/Action/Upgrade.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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("");
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion Cmdline/Main.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions ConsoleUI/CKAN-ConsoleUI.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
2 changes: 1 addition & 1 deletion ConsoleUI/GameInstanceListScreen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 7 additions & 7 deletions ConsoleUI/InstallScreen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) {
Expand All @@ -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>(
Expand All @@ -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);
}
Expand Down
8 changes: 3 additions & 5 deletions ConsoleUI/ModListScreen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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(
Expand All @@ -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()
{
Expand Down
7 changes: 5 additions & 2 deletions ConsoleUI/Toolkit/ConsoleScreen.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Diagnostics.CodeAnalysis;

namespace CKAN.ConsoleUI.Toolkit {

Expand Down Expand Up @@ -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,
Expand All @@ -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();
Expand Down
1 change: 1 addition & 0 deletions Core/CKAN-core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
2 changes: 1 addition & 1 deletion Core/Net/NetAsyncDownloader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions Core/Net/NetAsyncModulesDownloader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 6 additions & 2 deletions Core/User.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Diagnostics.CodeAnalysis;

namespace CKAN
{
/// <summary>
Expand All @@ -18,11 +20,13 @@ public interface IUser
/// </summary>
/// <returns>The index of the item selected from the array or -1 if cancelled</returns>
int RaiseSelectionDialog(string message, params object[] args);
void RaiseError(string message, params object[] args);
void RaiseError([StringSyntax(StringSyntaxAttribute.CompositeFormat)]
string message, params object[] args);

void RaiseProgress(string message, int percent);
void RaiseProgress(int percent, long bytesPerSecond, long bytesLeft);
void RaiseMessage(string message, params object[] args);
void RaiseMessage([StringSyntax(StringSyntaxAttribute.CompositeFormat)]
string message, params object[] args);
}

/// <summary>
Expand Down
Loading