From a83ce4fa1a907cce97403a7ba4183dfd402c6048 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 13 Jul 2026 16:39:14 -0500 Subject: [PATCH] [Microsoft.Android.Build.BaseTasks] Guard LogCodedError overloads PR #11994 fixed the XA0145 emulator boot-timeout message rendering literal `{0}`/`{1}` placeholders instead of substituting the values. The root cause was overload resolution: a call like LogCodedError ("XA0145", Resources.XA0145, Device, BootTimeoutSeconds); binds to `LogCodedError (code, message, string? file, int lineNumber)` rather than the `params object[]` formatting overload, because a non-`params` method that matches the trailing `(string, int)` exactly is always preferred. The message was logged verbatim and `Device` / `BootTimeoutSeconds` were silently reinterpreted as a source file and line number. This makes the whole class of bug a compile error instead of a silent runtime failure: * Extract the real logging body into private `LogCodedErrorCore` / `LogCodedWarningCore` sinks; every public overload now delegates to them, so nothing routes through the trap overload internally. * Mark the `(code, message, string? file, int lineNumber)` overloads on both `LogCodedError` and `LogCodedWarning` `[Obsolete (error: true)]`, with a message pointing callers at the formatting overload or the file-first overload. Any accidental `(string, int)` tail now fails to build with CS0619. * Add a `FormatMessage` helper that returns the message verbatim when there are no arguments, so pre-formatted messages containing literal `{`/`}` are never passed through `string.Format`. * Migrate the two intentional `Aapt2` callers to the file-first overload `LogCodedError (code, file, lineNumber, message)`. The `Log`/`log` extension-method versions in `MSBuildExtensions` were never affected: that overload set has no ambiguous trailing overload. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f03fd34e-2c48-48fe-a2af-b42ed2b4905c --- .../AsyncTask.cs | 33 +++++++++++++------ .../Tasks/Aapt2.cs | 4 +-- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/AsyncTask.cs b/external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/AsyncTask.cs index 5ff29a9275c..982ab455a0b 100644 --- a/external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/AsyncTask.cs +++ b/external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/AsyncTask.cs @@ -152,21 +152,25 @@ public void LogMessage (string message, MessageImportance importance = MessageIm } public void LogError (string message) - => LogCodedError (code: null, message: message, file: null, lineNumber: 0); + => LogCodedErrorCore (code: null, message: message, file: null, lineNumber: 0); public void LogError (string message, params object [] messageArgs) - => LogCodedError (code: null, message: string.Format (message, messageArgs)); + => LogCodedErrorCore (code: null, message: FormatMessage (message, messageArgs), file: null, lineNumber: 0); public void LogCodedError (string? code, string message) - => LogCodedError (code: code, message: message, file: null, lineNumber: 0); + => LogCodedErrorCore (code: code, message: message, file: null, lineNumber: 0); public void LogCodedError (string? code, string message, params object [] messageArgs) - => LogCodedError (code: code, message: string.Format (message, messageArgs), file: null, lineNumber: 0); + => LogCodedErrorCore (code: code, message: FormatMessage (message, messageArgs), file: null, lineNumber: 0); public void LogCodedError (string? code, string? file, int lineNumber, string message, params object [] messageArgs) - => LogCodedError (code: code, message: string.Format (message, messageArgs), file: file, lineNumber: lineNumber); + => LogCodedErrorCore (code: code, message: FormatMessage (message, messageArgs), file: file, lineNumber: lineNumber); + [Obsolete ("This overload binds ambiguously when the trailing arguments are (string, int): a call meant to format the message, e.g. LogCodedError (\"XA0000\", Resources.XA0000, someString, someInt), silently binds here and treats the arguments as a source file and line number instead of substituting them into the message. To format the message use LogCodedError (code, message, params object[]); to attach a source location use LogCodedError (code, file, lineNumber, message, params object[]).", error: true)] public void LogCodedError (string? code, string message, string? file, int lineNumber) + => LogCodedErrorCore (code: code, message: message, file: file, lineNumber: lineNumber); + + void LogCodedErrorCore (string? code, string message, string? file, int lineNumber) { if (uiThreadId == Thread.CurrentThread.ManagedThreadId) { #pragma warning disable 618 @@ -201,21 +205,25 @@ public void LogCodedError (string? code, string message, string? file, int lineN } public void LogWarning (string message) - => LogCodedWarning (code: null, message: message, file: null, lineNumber: 0); + => LogCodedWarningCore (code: null, message: message, file: null, lineNumber: 0); public void LogWarning (string message, params object [] messageArgs) - => LogCodedWarning (code: null, message: string.Format (message, messageArgs)); + => LogCodedWarningCore (code: null, message: FormatMessage (message, messageArgs), file: null, lineNumber: 0); public void LogCodedWarning (string? code, string message) - => LogCodedWarning (code: code, message: message, file: null, lineNumber: 0); + => LogCodedWarningCore (code: code, message: message, file: null, lineNumber: 0); public void LogCodedWarning (string? code, string message, params object [] messageArgs) - => LogCodedWarning (code: code, message: string.Format (message, messageArgs), file: null, lineNumber: 0); + => LogCodedWarningCore (code: code, message: FormatMessage (message, messageArgs), file: null, lineNumber: 0); public void LogCodedWarning (string? code, string? file, int lineNumber, string message, params object [] messageArgs) - => LogCodedWarning (code: code, message: string.Format (message, messageArgs), file: file, lineNumber: lineNumber); + => LogCodedWarningCore (code: code, message: FormatMessage (message, messageArgs), file: file, lineNumber: lineNumber); + [Obsolete ("This overload binds ambiguously when the trailing arguments are (string, int): a call meant to format the message, e.g. LogCodedWarning (\"XA0000\", Resources.XA0000, someString, someInt), silently binds here and treats the arguments as a source file and line number instead of substituting them into the message. To format the message use LogCodedWarning (code, message, params object[]); to attach a source location use LogCodedWarning (code, file, lineNumber, message, params object[]).", error: true)] public void LogCodedWarning (string? code, string message, string? file, int lineNumber) + => LogCodedWarningCore (code: code, message: message, file: file, lineNumber: lineNumber); + + void LogCodedWarningCore (string? code, string message, string? file, int lineNumber) { if (uiThreadId == Thread.CurrentThread.ManagedThreadId) { #pragma warning disable 618 @@ -248,6 +256,11 @@ public void LogCodedWarning (string? code, string message, string? file, int lin EnqueueMessage (warningMessageQueue, data, warningDataAvailable); } + // Returns the message verbatim when there are no arguments, so pre-formatted messages + // that contain literal '{' or '}' characters are not passed through string.Format. + static string FormatMessage (string message, object [] messageArgs) + => messageArgs is null || messageArgs.Length == 0 ? message : string.Format (message, messageArgs); + public void LogCustomBuildEvent (CustomBuildEventArgs e) { if (uiThreadId == Thread.CurrentThread.ManagedThreadId) { diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/Aapt2.cs b/src/Xamarin.Android.Build.Tasks/Tasks/Aapt2.cs index 8efd2a10497..3514887c6c2 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/Aapt2.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/Aapt2.cs @@ -186,9 +186,9 @@ protected bool LogAapt2EventsFromOutput (string singleLine, MessageImportance me if (level.Contains ("error") || (line != 0 && !file.IsNullOrEmpty ())) { var errorCode = GetErrorCodeForFile (message, file); if (manifestError) - LogCodedError (errorCode, string.Format (Xamarin.Android.Tasks.Properties.Resources.AAPTManifestError, message.TrimEnd('.')), AndroidManifestFile?.ItemSpec ?? "", 0); + LogCodedError (errorCode, AndroidManifestFile?.ItemSpec ?? "", 0, string.Format (Xamarin.Android.Tasks.Properties.Resources.AAPTManifestError, message.TrimEnd('.'))); else - LogCodedError (errorCode, AddAdditionalErrorText (errorCode, message), file, line); + LogCodedError (errorCode, file, line, AddAdditionalErrorText (errorCode, message)); return true; } }