From 985f6fb02ea3bf38fe9e2948d611ebf058464488 Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Wed, 25 Feb 2026 18:56:25 +0000 Subject: [PATCH 01/24] Add JDK installer with discovery, download, verification, and extraction Implements JdkInstaller class for automated JDK installation: - JDK discovery via Microsoft OpenJDK release API (supports JDK 21) - Platform-aware download (Windows .zip/.msi, macOS .tar.gz/.pkg, Linux .tar.gz) - SHA-256 checksum verification with Zip Slip protection - Elevated install mode: uses native installers (.msi/.pkg) when running as admin/root - Non-elevated mode: extracts archives to user-writable paths - Atomic extraction with rollback on failure - Cancellation support throughout the pipeline - Progress reporting via IProgress Models organized in Models/Jdk/ folder: - JdkVersionInfo, JdkReleaseInfo, JdkPlatformAsset - JdkInstallProgress (record type for progress reporting) Includes comprehensive tests for discovery, download, extraction, and error handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 13 + .../DownloadUtils.cs | 131 +++++ .../IsExternalInit.cs | 8 + .../JdkInstaller.cs | 501 ++++++++++++++++++ .../Models/Jdk/JdkInstallPhase.cs | 17 + .../Models/Jdk/JdkInstallProgress.cs | 10 + .../Models/Jdk/JdkVersionInfo.cs | 44 ++ .../Xamarin.Android.Tools.AndroidSdk.csproj | 5 +- .../JdkInstallerTests.cs | 263 +++++++++ 9 files changed, 990 insertions(+), 2 deletions(-) create mode 100644 src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs create mode 100644 src/Xamarin.Android.Tools.AndroidSdk/IsExternalInit.cs create mode 100644 src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs create mode 100644 src/Xamarin.Android.Tools.AndroidSdk/Models/Jdk/JdkInstallPhase.cs create mode 100644 src/Xamarin.Android.Tools.AndroidSdk/Models/Jdk/JdkInstallProgress.cs create mode 100644 src/Xamarin.Android.Tools.AndroidSdk/Models/Jdk/JdkVersionInfo.cs create mode 100644 tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInstallerTests.cs diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index f972e466..5a936e49 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -24,8 +24,21 @@ dotnet test tests/Microsoft.Android.Build.BaseTasks-Tests/Microsoft.Android.Buil Output: `bin\$(Configuration)\` (redistributables), `bin\Test$(Configuration)\` (tests). `$(DotNetTargetFrameworkVersion)` = `10.0` in `Directory.Build.props`. Versioning: `nuget.version` has `major.minor`; patch = git commit count since file changed. +## Android Environment Variables + +Per the [official Android docs](https://developer.android.com/tools/variables#envar): + +- **`ANDROID_HOME`** — the canonical variable for the Android SDK root path. Use this everywhere. +- **`ANDROID_SDK_ROOT`** — **deprecated**. Do not introduce new usages. Existing code may still read it for backward compatibility but always prefer `ANDROID_HOME`. +- **`ANDROID_USER_HOME`** — user-level config/AVD storage (defaults to `~/.android`). +- **`ANDROID_EMULATOR_HOME`** — emulator config (defaults to `$ANDROID_USER_HOME`). +- **`ANDROID_AVD_HOME`** — AVD data (defaults to `$ANDROID_USER_HOME/avd`). + +When setting environment variables for SDK tools (e.g. `sdkmanager`, `avdmanager`), set `ANDROID_HOME`. The `EnvironmentVariableNames` class in this repo defines the constants. + ## Conventions +- **One type per file**: each public class, struct, enum, or interface must be in its own `.cs` file named after the type (e.g. `JdkVersionInfo` → `JdkVersionInfo.cs`). Do not combine multiple top-level types in a single file. - [Mono Coding Guidelines](http://www.mono-project.com/community/contributing/coding-guidelines/): tabs, K&R braces, `PascalCase` public members. - Nullable enabled in `AndroidSdk`. `NullableAttributes.cs` excluded on `net10.0+`. - Strong-named via `product.snk`. In the AndroidSdk project, tests use `InternalsVisibleTo` with full public key (`Properties/AssemblyInfo.cs`). diff --git a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs new file mode 100644 index 00000000..64c4469d --- /dev/null +++ b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs @@ -0,0 +1,131 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.IO; +using System.IO.Compression; +using System.Net.Http; +using System.Security.Cryptography; +using System.Threading; +using System.Threading.Tasks; + +namespace Xamarin.Android.Tools +{ + /// + /// Shared helpers for downloading files, verifying checksums, and extracting archives. + /// + static class DownloadUtils + { + /// Downloads a file from the given URL with optional progress reporting. + public static async Task DownloadFileAsync (HttpClient client, string url, string destinationPath, long expectedSize, IProgress<(double percent, string message)>? progress, CancellationToken cancellationToken) + { + using var response = await client.GetAsync (url, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait (false); + response.EnsureSuccessStatusCode (); + + var totalBytes = response.Content.Headers.ContentLength ?? expectedSize; + + using var contentStream = await response.Content.ReadAsStreamAsync ().ConfigureAwait (false); + + var dirPath = Path.GetDirectoryName (destinationPath); + if (!string.IsNullOrEmpty (dirPath)) + Directory.CreateDirectory (dirPath); + + using var fileStream = new FileStream (destinationPath, FileMode.Create, FileAccess.Write, FileShare.None, 81920, useAsync: true); + + var buffer = new byte [81920]; + long totalRead = 0; + int bytesRead; + + while ((bytesRead = await contentStream.ReadAsync (buffer, 0, buffer.Length, cancellationToken).ConfigureAwait (false)) > 0) { + await fileStream.WriteAsync (buffer, 0, bytesRead, cancellationToken).ConfigureAwait (false); + totalRead += bytesRead; + + if (progress is not null && totalBytes > 0) { + var pct = (double) totalRead / totalBytes * 100; + progress.Report ((pct, $"Downloaded {totalRead / (1024 * 1024)} MB / {totalBytes / (1024 * 1024)} MB")); + } + } + } + + /// Verifies a file's SHA-256 hash against an expected value. + public static void VerifyChecksum (string filePath, string expectedChecksum) + { + using var sha256 = SHA256.Create (); + using var stream = File.OpenRead (filePath); + + var hash = sha256.ComputeHash (stream); + var actual = BitConverter.ToString (hash).Replace ("-", "").ToLowerInvariant (); + + if (!string.Equals (actual, expectedChecksum, StringComparison.OrdinalIgnoreCase)) + throw new InvalidOperationException ($"Checksum verification failed. Expected: {expectedChecksum}, Actual: {actual}"); + } + + /// Extracts a ZIP archive with Zip Slip protection. + public static void ExtractZipSafe (string archivePath, string destinationPath, CancellationToken cancellationToken) + { + using var archive = ZipFile.OpenRead (archivePath); + var fullExtractRoot = Path.GetFullPath (destinationPath); + + foreach (var entry in archive.Entries) { + cancellationToken.ThrowIfCancellationRequested (); + + if (string.IsNullOrEmpty (entry.Name)) + continue; + + var destinationFile = Path.GetFullPath (Path.Combine (fullExtractRoot, entry.FullName)); + + // Zip Slip protection + if (!destinationFile.StartsWith (fullExtractRoot + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase) && + destinationFile != fullExtractRoot) { + throw new InvalidOperationException ($"Archive entry '{entry.FullName}' would extract outside target directory."); + } + + var entryDir = Path.GetDirectoryName (destinationFile); + if (!string.IsNullOrEmpty (entryDir)) + Directory.CreateDirectory (entryDir); + + entry.ExtractToFile (destinationFile, overwrite: true); + } + } + + /// Extracts a tar.gz archive using the system tar command. + public static async Task ExtractTarGzAsync (string archivePath, string destinationPath, Action logger, CancellationToken cancellationToken) + { + var psi = new ProcessStartInfo { + FileName = "/usr/bin/tar", + UseShellExecute = false, + CreateNoWindow = true, + }; +#if NET5_0_OR_GREATER + psi.ArgumentList.Add ("-xzf"); + psi.ArgumentList.Add (archivePath); + psi.ArgumentList.Add ("-C"); + psi.ArgumentList.Add (destinationPath); +#else + psi.Arguments = $"-xzf \"{archivePath}\" -C \"{destinationPath}\""; +#endif + + var stdout = new StringWriter (); + var stderr = new StringWriter (); + var exitCode = await ProcessUtils.StartProcess (psi, stdout: stdout, stderr: stderr, cancellationToken).ConfigureAwait (false); + + if (exitCode != 0) { + var errorOutput = stderr.ToString (); + logger (TraceLevel.Error, $"tar extraction failed (exit code {exitCode}): {errorOutput}"); + throw new IOException ($"Failed to extract archive '{archivePath}': {errorOutput}"); + } + } + + /// Parses "hash filename" or just "hash" from .sha256sum.txt content. + public static string? ParseChecksumFile (string content) + { + if (string.IsNullOrWhiteSpace (content)) + return null; + + var line = content.Trim ().Split ('\n') [0].Trim (); + var parts = line.Split (new[] { ' ', '\t' }, StringSplitOptions.RemoveEmptyEntries); + return parts.Length > 0 ? parts [0] : null; + } + } +} diff --git a/src/Xamarin.Android.Tools.AndroidSdk/IsExternalInit.cs b/src/Xamarin.Android.Tools.AndroidSdk/IsExternalInit.cs new file mode 100644 index 00000000..4f2414e6 --- /dev/null +++ b/src/Xamarin.Android.Tools.AndroidSdk/IsExternalInit.cs @@ -0,0 +1,8 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Runtime.CompilerServices +{ + // Polyfill for netstandard2.0 to support C# 9+ records and init-only properties. + internal static class IsExternalInit { } +} diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs new file mode 100644 index 00000000..d29cdca1 --- /dev/null +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -0,0 +1,501 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Net.Http; +using System.Runtime.InteropServices; +#if NET5_0_OR_GREATER +using System.Security.Principal; +#endif +using System.Threading; +using System.Threading.Tasks; + +namespace Xamarin.Android.Tools +{ + /// + /// Provides JDK installation capabilities using the Microsoft Build of OpenJDK. + /// Downloads from https://aka.ms/download-jdk with SHA-256 verification. + /// See https://www.microsoft.com/openjdk for more information. + /// + public class JdkInstaller : IDisposable + { + const string DownloadUrlBase = "https://aka.ms/download-jdk"; + + /// Gets the recommended JDK major version for .NET Android development. + public static int RecommendedMajorVersion => 21; + + /// Gets the supported JDK major versions available for installation. + public static IReadOnlyList SupportedVersions { get; } = [ RecommendedMajorVersion ]; + + readonly HttpClient httpClient = new(); + readonly Action logger; + + public JdkInstaller (Action? logger = null) + { + this.logger = logger ?? AndroidSdkInfo.DefaultConsoleLogger; + } + + public void Dispose () => httpClient.Dispose (); + + /// Discovers available Microsoft OpenJDK versions for the current platform. + public async Task> DiscoverAsync (CancellationToken cancellationToken = default) + { + var results = new List (); + + foreach (var version in SupportedVersions) { + cancellationToken.ThrowIfCancellationRequested (); + try { + var info = BuildVersionInfo (version); + + // Verify the download URL is valid with a HEAD request + using var request = new HttpRequestMessage (HttpMethod.Head, info.DownloadUrl); + using var response = await httpClient.SendAsync (request, cancellationToken).ConfigureAwait (false); + + if (!response.IsSuccessStatusCode) { + logger (TraceLevel.Warning, $"JDK {version} not available: HEAD {info.DownloadUrl} returned {(int) response.StatusCode}"); + continue; + } + + if (response.Content.Headers.ContentLength.HasValue) + info.Size = response.Content.Headers.ContentLength.Value; + + if (response.RequestMessage?.RequestUri is not null) + info.ResolvedUrl = response.RequestMessage.RequestUri.ToString (); + + info.Checksum = await FetchChecksumAsync (info.ChecksumUrl, $"JDK {version}", cancellationToken).ConfigureAwait (false); + + results.Add (info); + logger (TraceLevel.Info, $"Discovered {info.DisplayName} (size={info.Size})"); + } + catch (Exception ex) { + logger (TraceLevel.Warning, $"Failed to discover JDK {version}: {ex.Message}"); + logger (TraceLevel.Verbose, ex.ToString ()); + } + } + + return results.AsReadOnly (); + } + + /// Downloads and installs a Microsoft OpenJDK to the target path. + public async Task InstallAsync (int majorVersion, string targetPath, IProgress? progress = null, CancellationToken cancellationToken = default) + { + if (!SupportedVersions.Contains (majorVersion)) + throw new ArgumentException ($"JDK version {majorVersion} is not supported. Supported versions: {string.Join (", ", SupportedVersions)}", nameof (majorVersion)); + + if (string.IsNullOrEmpty (targetPath)) + throw new ArgumentNullException (nameof (targetPath)); + + // When elevated and a platform installer is available, use it and let the installer handle paths + if (IsElevated () && GetInstallerExtension () is not null) { + logger (TraceLevel.Info, "Running elevated — using platform installer (.msi/.pkg)."); + await InstallWithPlatformInstallerAsync (majorVersion, progress, cancellationToken).ConfigureAwait (false); + return; + } + + if (!IsTargetPathWritable (targetPath)) { + logger (TraceLevel.Error, $"Target path '{targetPath}' is not writable or is in a restricted location."); + throw new ArgumentException ($"Target path '{targetPath}' is not writable or is in a restricted location.", nameof (targetPath)); + } + + var versionInfo = BuildVersionInfo (majorVersion); + + // Fetch checksum - required for supply-chain integrity + var checksum = await FetchChecksumAsync (versionInfo.ChecksumUrl, $"JDK {majorVersion}", cancellationToken).ConfigureAwait (false); + if (string.IsNullOrEmpty (checksum)) + throw new InvalidOperationException ($"Failed to fetch SHA-256 checksum for JDK {majorVersion}. Cannot verify download integrity."); + versionInfo.Checksum = checksum; + + var tempArchivePath = Path.Combine (Path.GetTempPath (), $"microsoft-jdk-{majorVersion}-{Guid.NewGuid ()}{GetArchiveExtension ()}"); + + try { + // Download + logger (TraceLevel.Info, $"Downloading Microsoft OpenJDK {majorVersion} from {versionInfo.DownloadUrl}"); + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Downloading, 0, $"Downloading Microsoft OpenJDK {majorVersion}...")); + await DownloadUtils.DownloadFileAsync (httpClient, versionInfo.DownloadUrl, tempArchivePath, versionInfo.Size, + progress is null ? null : new Progress<(double pct, string msg)> (p => progress.Report (new JdkInstallProgress (JdkInstallPhase.Downloading, p.pct, p.msg))), + cancellationToken).ConfigureAwait (false); + logger (TraceLevel.Info, $"Download complete: {tempArchivePath}"); + + // Verify checksum + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 0, "Verifying SHA-256 checksum...")); + DownloadUtils.VerifyChecksum (tempArchivePath, versionInfo.Checksum!); + logger (TraceLevel.Info, "Checksum verified."); + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 100, "Checksum verified.")); + + // Extract + logger (TraceLevel.Info, $"Extracting JDK to {targetPath}"); + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Extracting, 0, "Extracting JDK...")); + await ExtractArchiveAsync (tempArchivePath, targetPath, cancellationToken).ConfigureAwait (false); + logger (TraceLevel.Info, "Extraction complete."); + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Extracting, 100, "Extraction complete.")); + + // Validate + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Validating, 0, "Validating installation...")); + if (!IsValid (targetPath)) { + logger (TraceLevel.Error, $"JDK installation at '{targetPath}' failed validation."); + TryDeleteDirectory (targetPath, "invalid installation"); + throw new InvalidOperationException ($"JDK installation at '{targetPath}' failed validation. The extracted files may be corrupted."); + } + logger (TraceLevel.Info, $"Microsoft OpenJDK {majorVersion} installed successfully at {targetPath}"); + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Validating, 100, "Validation complete.")); + + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Complete, 100, $"Microsoft OpenJDK {majorVersion} installed successfully.")); + } + catch (Exception ex) when (!(ex is ArgumentException) && !(ex is ArgumentNullException)) { + logger (TraceLevel.Error, $"JDK installation failed: {ex.Message}"); + logger (TraceLevel.Verbose, ex.ToString ()); + throw; + } + finally { + TryDeleteFile (tempArchivePath); + } + } + + /// Validates whether the path contains a valid JDK installation. + public bool IsValid (string jdkPath) + { + if (string.IsNullOrEmpty (jdkPath) || !Directory.Exists (jdkPath)) + return false; + + try { + var jdk = new JdkInfo (jdkPath, logger: logger); + return jdk.Version is not null; + } + catch (Exception ex) { + logger (TraceLevel.Warning, $"JDK validation failed for '{jdkPath}': {ex.Message}"); + logger (TraceLevel.Verbose, ex.ToString ()); + return false; + } + } + + async Task InstallWithPlatformInstallerAsync (int majorVersion, IProgress? progress, CancellationToken cancellationToken) + { + var installerExt = GetInstallerExtension ()!; + var info = BuildVersionInfo (majorVersion, installerExt); + + var tempInstallerPath = Path.Combine (Path.GetTempPath (), $"microsoft-jdk-{majorVersion}-{Guid.NewGuid ()}{installerExt}"); + + try { + // Download installer + logger (TraceLevel.Info, $"Downloading installer from {info.DownloadUrl}"); + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Downloading, 0, $"Downloading Microsoft OpenJDK {majorVersion} installer...")); + await DownloadUtils.DownloadFileAsync (httpClient, info.DownloadUrl, tempInstallerPath, info.Size, + progress is null ? null : new Progress<(double pct, string msg)> (p => progress.Report (new JdkInstallProgress (JdkInstallPhase.Downloading, p.pct, p.msg))), + cancellationToken).ConfigureAwait (false); + + // Verify checksum if available + if (!string.IsNullOrEmpty (info.Checksum)) { + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 0, "Verifying SHA-256 checksum...")); + DownloadUtils.VerifyChecksum (tempInstallerPath, info.Checksum!); + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 100, "Checksum verified.")); + } + + // Run the installer silently + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Extracting, 0, "Running platform installer...")); + await RunPlatformInstallerAsync (tempInstallerPath, cancellationToken).ConfigureAwait (false); + + logger (TraceLevel.Info, $"Microsoft OpenJDK {majorVersion} installed successfully via platform installer."); + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Complete, 100, $"Microsoft OpenJDK {majorVersion} installed successfully.")); + } + finally { + TryDeleteFile (tempInstallerPath); + } + } + + async Task RunPlatformInstallerAsync (string installerPath, CancellationToken cancellationToken) + { + ProcessStartInfo psi; + if (OS.IsWindows) { + psi = new ProcessStartInfo { + FileName = "msiexec", + UseShellExecute = false, + CreateNoWindow = true, + }; +#if NET5_0_OR_GREATER + psi.ArgumentList.Add ("/i"); + psi.ArgumentList.Add (installerPath); + psi.ArgumentList.Add ("/quiet"); + psi.ArgumentList.Add ("/norestart"); +#else + psi.Arguments = $"/i \"{installerPath}\" /quiet /norestart"; +#endif + } + else { + psi = new ProcessStartInfo { + FileName = "/usr/sbin/installer", + UseShellExecute = false, + CreateNoWindow = true, + }; +#if NET5_0_OR_GREATER + psi.ArgumentList.Add ("-pkg"); + psi.ArgumentList.Add (installerPath); + psi.ArgumentList.Add ("-target"); + psi.ArgumentList.Add ("/"); +#else + psi.Arguments = $"-pkg \"{installerPath}\" -target /"; +#endif + } + + var stdout = new StringWriter (); + var stderr = new StringWriter (); + var exitCode = await ProcessUtils.StartProcess (psi, stdout: stdout, stderr: stderr, cancellationToken).ConfigureAwait (false); + + if (exitCode != 0) { + var errorOutput = stderr.ToString (); + logger (TraceLevel.Error, $"Installer failed (exit code {exitCode}): {errorOutput}"); + throw new InvalidOperationException ($"Platform installer failed with exit code {exitCode}: {errorOutput}"); + } + } + + /// Checks if the target path is writable and not in a restricted location. + public bool IsTargetPathWritable (string targetPath) + { + if (string.IsNullOrEmpty (targetPath)) + return false; + + // Normalize the path to prevent path traversal bypasses (e.g., "C:\Program Files\..\Users") + string normalizedPath; + try { + normalizedPath = Path.GetFullPath (targetPath); + } + catch { + normalizedPath = targetPath; + } + + if (OS.IsWindows) { + var programFiles = Environment.GetFolderPath (Environment.SpecialFolder.ProgramFiles); + var programFilesX86 = Environment.GetFolderPath (Environment.SpecialFolder.ProgramFilesX86); + if ((!string.IsNullOrEmpty (programFiles) && normalizedPath.StartsWith (programFiles, StringComparison.OrdinalIgnoreCase)) || + (!string.IsNullOrEmpty (programFilesX86) && normalizedPath.StartsWith (programFilesX86, StringComparison.OrdinalIgnoreCase))) { + logger (TraceLevel.Warning, $"Target path '{targetPath}' is in Program Files which typically requires elevation."); + return false; + } + } + + try { + Directory.CreateDirectory (targetPath); + var testFile = Path.Combine (targetPath, $".write-test-{Guid.NewGuid ()}"); + using (File.Create (testFile, 1, FileOptions.DeleteOnClose)) { } + return true; + } + catch (Exception ex) { + logger (TraceLevel.Warning, $"Target path '{targetPath}' is not writable: {ex.Message}"); + return false; + } + } + + /// Removes a JDK installation at the specified path. + public bool Remove (string jdkPath) + { + if (string.IsNullOrEmpty (jdkPath) || !Directory.Exists (jdkPath)) + return false; + + try { + Directory.Delete (jdkPath, recursive: true); + logger (TraceLevel.Info, $"Removed JDK at '{jdkPath}'."); + return true; + } + catch (Exception ex) { + logger (TraceLevel.Error, $"Failed to remove JDK at '{jdkPath}': {ex.Message}"); + logger (TraceLevel.Verbose, ex.ToString ()); + return false; + } + } + + static JdkVersionInfo BuildVersionInfo (int majorVersion, string? extensionOverride = null) + { + var os = GetMicrosoftOpenJDKOSName (); + var arch = GetArchitectureName (); + var ext = extensionOverride ?? GetArchiveExtension (); + + var filename = $"microsoft-jdk-{majorVersion}-{os}-{arch}{ext}"; + var downloadUrl = $"{DownloadUrlBase}/{filename}"; + var checksumUrl = $"{downloadUrl}.sha256sum.txt"; + var displayName = extensionOverride is not null + ? $"Microsoft OpenJDK {majorVersion} ({ext})" + : $"Microsoft OpenJDK {majorVersion}"; + + return new JdkVersionInfo ( + majorVersion: majorVersion, + displayName: displayName, + downloadUrl: downloadUrl, + checksumUrl: checksumUrl + ); + } + + async Task ExtractArchiveAsync (string archivePath, string targetPath, CancellationToken cancellationToken) + { + var targetParent = Path.GetDirectoryName (targetPath); + if (string.IsNullOrEmpty (targetParent)) + targetParent = Path.GetTempPath (); + else + Directory.CreateDirectory (targetParent); + var tempExtractPath = Path.Combine (targetParent, $".jdk-extract-{Guid.NewGuid ()}"); + + try { + Directory.CreateDirectory (tempExtractPath); + + if (OS.IsWindows) + DownloadUtils.ExtractZipSafe (archivePath, tempExtractPath, cancellationToken); + else + await DownloadUtils.ExtractTarGzAsync (archivePath, tempExtractPath, logger, cancellationToken).ConfigureAwait (false); + + // Find the actual JDK root (archives contain a single top-level directory) + var extractedDirs = Directory.GetDirectories (tempExtractPath); + var jdkRoot = extractedDirs.Length == 1 ? extractedDirs [0] : tempExtractPath; + + // On macOS, the JDK is inside Contents/Home + if (OS.IsMac) { + var contentsHome = Path.Combine (jdkRoot, "Contents", "Home"); + if (Directory.Exists (contentsHome)) + jdkRoot = contentsHome; + } + + MoveWithRollback (jdkRoot, targetPath); + } + finally { + TryDeleteDirectory (tempExtractPath, "temp extract directory"); + } + } + + void MoveWithRollback (string sourcePath, string targetPath) + { + string? backupPath = null; + if (Directory.Exists (targetPath)) { + backupPath = targetPath + $".old-{Guid.NewGuid ():N}"; + Directory.Move (targetPath, backupPath); + } + + var parentDir = Path.GetDirectoryName (targetPath); + if (!string.IsNullOrEmpty (parentDir)) + Directory.CreateDirectory (parentDir); + + try { + Directory.Move (sourcePath, targetPath); + + // Only delete backup after successful move + if (backupPath is not null) + TryDeleteDirectory (backupPath, "old JDK backup"); + } + catch (Exception ex) { + logger (TraceLevel.Error, $"Failed to move to '{targetPath}': {ex.Message}"); + if (backupPath is not null && Directory.Exists (backupPath)) { + try { + if (Directory.Exists (targetPath)) + Directory.Delete (targetPath, recursive: true); + Directory.Move (backupPath, targetPath); + logger (TraceLevel.Warning, $"Restored previous JDK from backup '{backupPath}'."); + } + catch (Exception restoreEx) { + logger (TraceLevel.Error, $"Failed to restore from backup: {restoreEx.Message}"); + } + } + throw; + } + } + + async Task FetchChecksumAsync (string checksumUrl, string label, CancellationToken cancellationToken) + { + try { + using var response = await httpClient.GetAsync (checksumUrl, cancellationToken).ConfigureAwait (false); + response.EnsureSuccessStatusCode (); +#if NET5_0_OR_GREATER + var content = await response.Content.ReadAsStringAsync (cancellationToken).ConfigureAwait (false); +#else + var content = await response.Content.ReadAsStringAsync ().ConfigureAwait (false); +#endif + var checksum = DownloadUtils.ParseChecksumFile (content); + logger (TraceLevel.Verbose, $"{label}: checksum={checksum}"); + return checksum; + } + catch (Exception ex) { + logger (TraceLevel.Warning, $"Could not fetch checksum for {label}: {ex.Message}"); + return null; + } + } + + void TryDeleteFile (string path) + { + if (!File.Exists (path)) + return; + try { File.Delete (path); } + catch (Exception ex) { logger (TraceLevel.Warning, $"Could not delete '{path}': {ex.Message}"); } + } + + void TryDeleteDirectory (string path, string label) + { + if (!Directory.Exists (path)) + return; + try { Directory.Delete (path, recursive: true); } + catch (Exception ex) { logger (TraceLevel.Warning, $"Could not clean up {label} at '{path}': {ex.Message}"); } + } + + static string GetMicrosoftOpenJDKOSName () + { + if (OS.IsMac) return "macOS"; + if (OS.IsWindows) return "windows"; + if (OS.IsLinux) return "linux"; + throw new PlatformNotSupportedException ("Unsupported platform"); + } + + static string GetArchitectureName () + { + return RuntimeInformation.OSArchitecture switch { + Architecture.Arm64 => "aarch64", + _ => "x64", + }; + } + + static string GetArchiveExtension () + { + return OS.IsWindows ? ".zip" : ".tar.gz"; + } + + // Returns .msi (Windows), .pkg (macOS), or null (Linux) + static string? GetInstallerExtension () + { + if (OS.IsWindows) return ".msi"; + if (OS.IsMac) return ".pkg"; + return null; + } + + /// Checks if running as Administrator (Windows) or root (macOS/Linux). + public static bool IsElevated () + { + if (OS.IsWindows) { +#if NET5_0_OR_GREATER +#pragma warning disable CA1416 // Guarded by OS.IsWindows runtime check above + return IsElevatedWindows (); +#pragma warning restore CA1416 +#else + return false; +#endif + } + // Unix: geteuid() == 0 means root + return IsElevatedUnix (); + } + +#if NET5_0_OR_GREATER + [System.Runtime.Versioning.SupportedOSPlatform ("windows")] + static bool IsElevatedWindows () + { + using var identity = WindowsIdentity.GetCurrent (); + var principal = new WindowsPrincipal (identity); + return principal.IsInRole (WindowsBuiltInRole.Administrator); + } +#endif + + // Separate method so geteuid P/Invoke is never JIT-compiled on Windows + static bool IsElevatedUnix () + { + return geteuid () == 0; + } + + [DllImport ("libc")] + static extern uint geteuid (); + } +} diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Models/Jdk/JdkInstallPhase.cs b/src/Xamarin.Android.Tools.AndroidSdk/Models/Jdk/JdkInstallPhase.cs new file mode 100644 index 00000000..fff2af61 --- /dev/null +++ b/src/Xamarin.Android.Tools.AndroidSdk/Models/Jdk/JdkInstallPhase.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Xamarin.Android.Tools +{ + /// + /// Phases of JDK installation. + /// + public enum JdkInstallPhase + { + Downloading, + Verifying, + Extracting, + Validating, + Complete + } +} diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Models/Jdk/JdkInstallProgress.cs b/src/Xamarin.Android.Tools.AndroidSdk/Models/Jdk/JdkInstallProgress.cs new file mode 100644 index 00000000..16a3e016 --- /dev/null +++ b/src/Xamarin.Android.Tools.AndroidSdk/Models/Jdk/JdkInstallProgress.cs @@ -0,0 +1,10 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Xamarin.Android.Tools +{ + /// + /// Progress information for JDK installation. + /// + public record JdkInstallProgress (JdkInstallPhase Phase, double PercentComplete, string? Message = null); +} diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Models/Jdk/JdkVersionInfo.cs b/src/Xamarin.Android.Tools.AndroidSdk/Models/Jdk/JdkVersionInfo.cs new file mode 100644 index 00000000..0003cc7e --- /dev/null +++ b/src/Xamarin.Android.Tools.AndroidSdk/Models/Jdk/JdkVersionInfo.cs @@ -0,0 +1,44 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Xamarin.Android.Tools +{ + /// + /// Represents information about an available JDK version from Microsoft OpenJDK. + /// + public class JdkVersionInfo + { + /// Major version number (e.g. 17, 21). + public int MajorVersion { get; } + + /// Display name for the version (e.g. "Microsoft OpenJDK 17"). + public string DisplayName { get; } + + /// Download URL for the current platform. + public string DownloadUrl { get; } + + /// URL for the SHA-256 checksum file. + public string ChecksumUrl { get; } + + /// Expected file size in bytes, or 0 if unknown. + public long Size { get; internal set; } + + /// SHA-256 checksum for download verification, if fetched. + public string? Checksum { get; internal set; } + + /// The actual download URL after following redirects (reveals the specific version). + public string? ResolvedUrl { get; internal set; } + + public JdkVersionInfo (int majorVersion, string displayName, string downloadUrl, string checksumUrl, long size = 0, string? checksum = null) + { + MajorVersion = majorVersion; + DisplayName = displayName; + DownloadUrl = downloadUrl; + ChecksumUrl = checksumUrl; + Size = size; + Checksum = checksum; + } + + public override string ToString () => DisplayName; + } +} diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Xamarin.Android.Tools.AndroidSdk.csproj b/src/Xamarin.Android.Tools.AndroidSdk/Xamarin.Android.Tools.AndroidSdk.csproj index 4fb13f81..5944520a 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/Xamarin.Android.Tools.AndroidSdk.csproj +++ b/src/Xamarin.Android.Tools.AndroidSdk/Xamarin.Android.Tools.AndroidSdk.csproj @@ -27,8 +27,9 @@ true - - + + + diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInstallerTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInstallerTests.cs new file mode 100644 index 00000000..b312074e --- /dev/null +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInstallerTests.cs @@ -0,0 +1,263 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; + +namespace Xamarin.Android.Tools.Tests +{ + [TestFixture] + public class JdkInstallerTests + { + JdkInstaller installer; + + [SetUp] + public void SetUp () + { + installer = new JdkInstaller (logger: (level, message) => { + TestContext.WriteLine ($"[{level}] {message}"); + }); + } + + [TearDown] + public void TearDown () + { + installer?.Dispose (); + installer = null!; + } + + [Test] + public void IsValid_NullPath_ReturnsFalse () + { + Assert.IsFalse (installer.IsValid (null!)); + } + + [Test] + public void IsValid_EmptyPath_ReturnsFalse () + { + Assert.IsFalse (installer.IsValid ("")); + } + + [Test] + public void IsValid_NonExistentPath_ReturnsFalse () + { + Assert.IsFalse (installer.IsValid (Path.Combine (Path.GetTempPath (), Guid.NewGuid ().ToString ()))); + } + + [Test] + public void IsValid_EmptyDirectory_ReturnsFalse () + { + var dir = Path.Combine (Path.GetTempPath (), Guid.NewGuid ().ToString ()); + Directory.CreateDirectory (dir); + try { + Assert.IsFalse (installer.IsValid (dir)); + } + finally { + Directory.Delete (dir, recursive: true); + } + } + + [Test] + public void IsValid_FauxJdk_ReturnsTrue () + { + var dir = Path.Combine (Path.GetTempPath (), $"jdk-test-{Guid.NewGuid ()}"); + try { + JdkInfoTests.CreateFauxJdk (dir, releaseVersion: "17.0.1", releaseBuildNumber: "1", javaVersion: "17.0.1-1"); + Assert.IsTrue (installer.IsValid (dir)); + } + finally { + if (Directory.Exists (dir)) + Directory.Delete (dir, recursive: true); + } + } + + [Test] + public void IsValid_SystemJdk_ReturnsTrue () + { + // Find first known JDK on the system + var jdk = JdkInfo.GetKnownSystemJdkInfos ().FirstOrDefault (); + if (jdk is null) { + Assert.Ignore ("No system JDK found to validate."); + return; + } + Assert.IsTrue (installer.IsValid (jdk.HomePath)); + } + + [Test] + public async Task DiscoverAsync_ReturnsVersions () + { + IReadOnlyList versions; + try { + versions = await installer.DiscoverAsync (); + } + catch (Exception ex) when (ex is System.Net.Http.HttpRequestException || ex is TaskCanceledException) { + Assert.Ignore ($"Network unavailable: {ex.Message}"); + return; + } + + // We should get at least one version (if network is available) + Assert.IsNotNull (versions); + if (versions.Count == 0) { + Assert.Ignore ("No versions returned (network may be restricted)."); + return; + } + + // Verify structure of returned info + foreach (var v in versions) { + Assert.Greater (v.MajorVersion, 0, "MajorVersion should be positive"); + Assert.IsNotEmpty (v.DisplayName, "DisplayName should not be empty"); + Assert.IsNotEmpty (v.DownloadUrl, "DownloadUrl should not be empty"); + Assert.That (v.DownloadUrl, Does.Contain ("aka.ms/download-jdk"), "DownloadUrl should use Microsoft OpenJDK"); + } + } + + [Test] + public async Task DiscoverAsync_ContainsExpectedMajorVersions () + { + IReadOnlyList versions; + try { + versions = await installer.DiscoverAsync (); + } + catch (Exception ex) when (ex is System.Net.Http.HttpRequestException || ex is TaskCanceledException) { + Assert.Ignore ($"Network unavailable: {ex.Message}"); + return; + } + + if (versions.Count == 0) { + Assert.Ignore ("No versions returned."); + return; + } + + var majorVersions = versions.Select (v => v.MajorVersion).Distinct ().ToList (); + Assert.That (majorVersions, Does.Contain (21), "Should contain JDK 21"); + } + + [Test] + public async Task DiscoverAsync_CancellationToken_Cancels () + { + using var cts = new CancellationTokenSource (); + cts.Cancel (); + + Assert.ThrowsAsync ( + async () => await installer.DiscoverAsync (cts.Token)); + } + + [Test] + public void InstallAsync_InvalidVersion_Throws () + { + Assert.ThrowsAsync ( + async () => await installer.InstallAsync (8, Path.GetTempPath ())); + } + + [Test] + public void InstallAsync_NullPath_Throws () + { + Assert.ThrowsAsync ( + async () => await installer.InstallAsync (21, null!)); + } + + [Test] + public async Task InstallAsync_ReportsProgress () + { + // This test actually downloads and installs a JDK, so it may be slow. + // Skip if running in CI or if network is unavailable. + if (Environment.GetEnvironmentVariable ("CI") is not null || + Environment.GetEnvironmentVariable ("TF_BUILD") is not null) { + Assert.Ignore ("Skipping download test in CI environment."); + return; + } + + var targetPath = Path.Combine (Path.GetTempPath (), $"jdk-install-test-{Guid.NewGuid ()}"); + var reportedPhases = new List (); + var progress = new Progress (p => { + reportedPhases.Add (p.Phase); + }); + + try { + using var cts = new CancellationTokenSource (TimeSpan.FromMinutes (10)); + await installer.InstallAsync (21, targetPath, progress, cts.Token); + + // Verify installation + Assert.IsTrue (installer.IsValid (targetPath), "Installed JDK should be valid"); + Assert.IsTrue (reportedPhases.Contains (JdkInstallPhase.Downloading), "Should report Downloading phase"); + Assert.IsTrue (reportedPhases.Contains (JdkInstallPhase.Extracting), "Should report Extracting phase"); + Assert.IsTrue (reportedPhases.Contains (JdkInstallPhase.Complete), "Should report Complete phase"); + + // Verify we can create a JdkInfo from it + var jdkInfo = new JdkInfo (targetPath); + Assert.IsNotNull (jdkInfo.Version); + Assert.AreEqual (21, jdkInfo.Version!.Major); + } + catch (Exception ex) when (ex is System.Net.Http.HttpRequestException || ex is TaskCanceledException) { + Assert.Ignore ($"Network unavailable: {ex.Message}"); + } + finally { + if (Directory.Exists (targetPath)) + Directory.Delete (targetPath, recursive: true); + } + } + + [Test] + public void Constructor_DefaultLogger_DoesNotThrow () + { + using var defaultInstaller = new JdkInstaller (); + Assert.IsNotNull (defaultInstaller); + } + + [Test] + public void RecommendedMajorVersion_Is21 () + { + Assert.AreEqual (21, JdkInstaller.RecommendedMajorVersion); + } + + [Test] + public void SupportedVersions_ContainsExpected () + { + Assert.That (JdkInstaller.SupportedVersions, Does.Contain (21)); + } + + [Test] + public void IsTargetPathWritable_TempDir_ReturnsTrue () + { + var dir = Path.Combine (Path.GetTempPath (), $"jdk-write-test-{Guid.NewGuid ()}"); + try { + Assert.IsTrue (installer.IsTargetPathWritable (dir)); + } + finally { + if (Directory.Exists (dir)) + Directory.Delete (dir, recursive: true); + } + } + + [Test] + public void IsTargetPathWritable_NullOrEmpty_ReturnsFalse () + { + Assert.IsFalse (installer.IsTargetPathWritable (null!)); + Assert.IsFalse (installer.IsTargetPathWritable ("")); + } + + [Test] + public void Remove_NonExistentPath_ReturnsFalse () + { + Assert.IsFalse (installer.Remove (Path.Combine (Path.GetTempPath (), Guid.NewGuid ().ToString ()))); + } + + [Test] + public void Remove_ExistingDirectory_RemovesIt () + { + var dir = Path.Combine (Path.GetTempPath (), $"jdk-remove-test-{Guid.NewGuid ()}"); + Directory.CreateDirectory (dir); + File.WriteAllText (Path.Combine (dir, "test.txt"), "test"); + + Assert.IsTrue (installer.Remove (dir)); + Assert.IsFalse (Directory.Exists (dir)); + } + } +} From 78d04e3d87332a19d2c6b53d12f3e65b2325dfa9 Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Wed, 25 Feb 2026 19:20:07 +0000 Subject: [PATCH 02/24] Fix race condition, checksum fetch, and geteuid guard - IsTargetPathWritable: test nearest existing ancestor instead of creating target dir - InstallWithPlatformInstallerAsync: fetch checksum before download - IsElevatedUnix: guard geteuid P/Invoke with OperatingSystem.IsWindows() check - Add SetLastError=true to geteuid DllImport Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../JdkInstaller.cs | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index d29cdca1..ffaa63d1 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -177,6 +177,10 @@ async Task InstallWithPlatformInstallerAsync (int majorVersion, IProgress (p => progress.Report (new JdkInstallProgress (JdkInstallPhase.Downloading, p.pct, p.msg))), cancellationToken).ConfigureAwait (false); - // Verify checksum if available + // Verify checksum if (!string.IsNullOrEmpty (info.Checksum)) { progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 0, "Verifying SHA-256 checksum...")); DownloadUtils.VerifyChecksum (tempInstallerPath, info.Checksum!); @@ -276,9 +280,16 @@ public bool IsTargetPathWritable (string targetPath) } } + // Test writability on the nearest existing ancestor directory without creating the target try { - Directory.CreateDirectory (targetPath); - var testFile = Path.Combine (targetPath, $".write-test-{Guid.NewGuid ()}"); + var testDir = normalizedPath; + while (!string.IsNullOrEmpty (testDir) && !Directory.Exists (testDir)) + testDir = Path.GetDirectoryName (testDir); + + if (string.IsNullOrEmpty (testDir)) + return false; + + var testFile = Path.Combine (testDir, $".write-test-{Guid.NewGuid ()}"); using (File.Create (testFile, 1, FileOptions.DeleteOnClose)) { } return true; } @@ -492,10 +503,16 @@ static bool IsElevatedWindows () // Separate method so geteuid P/Invoke is never JIT-compiled on Windows static bool IsElevatedUnix () { +#if NET5_0_OR_GREATER + if (!OperatingSystem.IsWindows ()) + return geteuid () == 0; + return false; +#else return geteuid () == 0; +#endif } - [DllImport ("libc")] + [DllImport ("libc", SetLastError = true)] static extern uint geteuid (); } } From 815ffa9399d29e514f95967dfc50b663638c5234 Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Wed, 25 Feb 2026 19:44:03 +0000 Subject: [PATCH 03/24] Fix architecture validation, path boundary check, and catch pattern - GetArchitectureName: explicitly handle X64, throw for unsupported archs - IsTargetPathWritable: add directory boundary check (prevents false match on 'Program FilesX') - Modernize catch filter to use 'is not' pattern - Extract IsUnderDirectory helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../JdkInstaller.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index ffaa63d1..7d76474d 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -145,7 +145,7 @@ await DownloadUtils.DownloadFileAsync (httpClient, versionInfo.DownloadUrl, temp progress?.Report (new JdkInstallProgress (JdkInstallPhase.Complete, 100, $"Microsoft OpenJDK {majorVersion} installed successfully.")); } - catch (Exception ex) when (!(ex is ArgumentException) && !(ex is ArgumentNullException)) { + catch (Exception ex) when (ex is not ArgumentException and not ArgumentNullException) { logger (TraceLevel.Error, $"JDK installation failed: {ex.Message}"); logger (TraceLevel.Verbose, ex.ToString ()); throw; @@ -273,8 +273,7 @@ public bool IsTargetPathWritable (string targetPath) if (OS.IsWindows) { var programFiles = Environment.GetFolderPath (Environment.SpecialFolder.ProgramFiles); var programFilesX86 = Environment.GetFolderPath (Environment.SpecialFolder.ProgramFilesX86); - if ((!string.IsNullOrEmpty (programFiles) && normalizedPath.StartsWith (programFiles, StringComparison.OrdinalIgnoreCase)) || - (!string.IsNullOrEmpty (programFilesX86) && normalizedPath.StartsWith (programFilesX86, StringComparison.OrdinalIgnoreCase))) { + if (IsUnderDirectory (normalizedPath, programFiles) || IsUnderDirectory (normalizedPath, programFilesX86)) { logger (TraceLevel.Warning, $"Target path '{targetPath}' is in Program Files which typically requires elevation."); return false; } @@ -456,8 +455,9 @@ static string GetMicrosoftOpenJDKOSName () static string GetArchitectureName () { return RuntimeInformation.OSArchitecture switch { + Architecture.X64 => "x64", Architecture.Arm64 => "aarch64", - _ => "x64", + _ => throw new PlatformNotSupportedException ($"Unsupported architecture: {RuntimeInformation.OSArchitecture}"), }; } @@ -512,6 +512,15 @@ static bool IsElevatedUnix () #endif } + static bool IsUnderDirectory (string path, string directory) + { + if (string.IsNullOrEmpty (directory) || string.IsNullOrEmpty (path)) + return false; + if (path.Equals (directory, StringComparison.OrdinalIgnoreCase)) + return true; + return path.StartsWith (directory + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase); + } + [DllImport ("libc", SetLastError = true)] static extern uint geteuid (); } From 2e4088e9f3c95c6e9852e8336a1caea3ad5897cc Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Wed, 25 Feb 2026 22:54:21 +0000 Subject: [PATCH 04/24] Fix cancellation propagation, mandatory checksum in elevated install, and tar path sanitization - Let OperationCanceledException propagate in DiscoverAsync and FetchChecksumAsync - Make checksum verification mandatory in elevated install path (supply-chain integrity) - Validate tar paths for unsafe characters in netstandard2.0 fallback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DownloadUtils.cs | 4 ++++ .../JdkInstaller.cs | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs index 64c4469d..f523c3d1 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs @@ -103,6 +103,10 @@ public static async Task ExtractTarGzAsync (string archivePath, string destinati psi.ArgumentList.Add ("-C"); psi.ArgumentList.Add (destinationPath); #else + if (archivePath.IndexOfAny (new[] { '"', '\'', '`', '$' }) >= 0) + throw new ArgumentException ($"Archive path contains unsafe characters: '{archivePath}'", nameof (archivePath)); + if (destinationPath.IndexOfAny (new[] { '"', '\'', '`', '$' }) >= 0) + throw new ArgumentException ($"Destination path contains unsafe characters: '{destinationPath}'", nameof (destinationPath)); psi.Arguments = $"-xzf \"{archivePath}\" -C \"{destinationPath}\""; #endif diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index 7d76474d..ef38e892 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -71,6 +71,9 @@ public async Task> DiscoverAsync (CancellationToke results.Add (info); logger (TraceLevel.Info, $"Discovered {info.DisplayName} (size={info.Size})"); } + catch (OperationCanceledException) { + throw; + } catch (Exception ex) { logger (TraceLevel.Warning, $"Failed to discover JDK {version}: {ex.Message}"); logger (TraceLevel.Verbose, ex.ToString ()); @@ -191,12 +194,13 @@ await DownloadUtils.DownloadFileAsync (httpClient, info.DownloadUrl, tempInstall progress is null ? null : new Progress<(double pct, string msg)> (p => progress.Report (new JdkInstallProgress (JdkInstallPhase.Downloading, p.pct, p.msg))), cancellationToken).ConfigureAwait (false); - // Verify checksum - if (!string.IsNullOrEmpty (info.Checksum)) { - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 0, "Verifying SHA-256 checksum...")); - DownloadUtils.VerifyChecksum (tempInstallerPath, info.Checksum!); - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 100, "Checksum verified.")); - } + // Verify checksum — mandatory for supply-chain integrity + if (string.IsNullOrEmpty (info.Checksum)) + throw new InvalidOperationException ($"Checksum could not be retrieved for installer. Aborting to preserve supply-chain integrity."); + + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 0, "Verifying SHA-256 checksum...")); + DownloadUtils.VerifyChecksum (tempInstallerPath, info.Checksum!); + progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 100, "Checksum verified.")); // Run the installer silently progress?.Report (new JdkInstallProgress (JdkInstallPhase.Extracting, 0, "Running platform installer...")); @@ -422,6 +426,9 @@ void MoveWithRollback (string sourcePath, string targetPath) logger (TraceLevel.Verbose, $"{label}: checksum={checksum}"); return checksum; } + catch (OperationCanceledException) { + throw; + } catch (Exception ex) { logger (TraceLevel.Warning, $"Could not fetch checksum for {label}: {ex.Message}"); return null; From 9e9fc223497d9866b52c7c4e0798ae6728957d16 Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Wed, 25 Feb 2026 22:59:44 +0000 Subject: [PATCH 05/24] Add comment explaining netstandard2.0 tar path validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs index f523c3d1..8e37aa99 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs @@ -103,6 +103,8 @@ public static async Task ExtractTarGzAsync (string archivePath, string destinati psi.ArgumentList.Add ("-C"); psi.ArgumentList.Add (destinationPath); #else + // ArgumentList is not available on netstandard2.0, so we build Arguments manually. + // Reject characters that could allow argument injection through the shell. if (archivePath.IndexOfAny (new[] { '"', '\'', '`', '$' }) >= 0) throw new ArgumentException ($"Archive path contains unsafe characters: '{archivePath}'", nameof (archivePath)); if (destinationPath.IndexOfAny (new[] { '"', '\'', '`', '$' }) >= 0) From 8e589f22618aec9aca4a21803494cd1208212a3c Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Wed, 25 Feb 2026 23:01:10 +0000 Subject: [PATCH 06/24] Handle null checksum from FetchChecksumAsync at all call sites - DiscoverAsync: log warning when checksum unavailable - InstallElevatedAsync: throw if checksum is null (matches InstallAsync behavior) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index ef38e892..09f29775 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -67,6 +67,8 @@ public async Task> DiscoverAsync (CancellationToke info.ResolvedUrl = response.RequestMessage.RequestUri.ToString (); info.Checksum = await FetchChecksumAsync (info.ChecksumUrl, $"JDK {version}", cancellationToken).ConfigureAwait (false); + if (string.IsNullOrEmpty (info.Checksum)) + logger (TraceLevel.Warning, $"Could not fetch checksum for JDK {version}, integrity verification will be skipped."); results.Add (info); logger (TraceLevel.Info, $"Discovered {info.DisplayName} (size={info.Size})"); @@ -182,6 +184,8 @@ async Task InstallWithPlatformInstallerAsync (int majorVersion, IProgress Date: Wed, 25 Feb 2026 23:07:10 +0000 Subject: [PATCH 07/24] Extract shared utilities from JdkInstaller into FileUtil and DownloadUtils Move reusable methods to shared utility classes: - FileUtil: TryDeleteFile, TryDeleteDirectory, MoveWithRollback - DownloadUtils: FetchChecksumAsync JdkInstaller reduced from 546 to 464 lines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DownloadUtils.cs | 24 +++++ .../FileUtil.cs | 56 ++++++++++++ .../JdkInstaller.cs | 90 ++----------------- 3 files changed, 88 insertions(+), 82 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs index 8e37aa99..c4de3735 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs @@ -123,6 +123,30 @@ public static async Task ExtractTarGzAsync (string archivePath, string destinati } } + /// Fetches a SHA-256 checksum from a remote URL, returning null on failure. + public static async Task FetchChecksumAsync (HttpClient httpClient, string checksumUrl, string label, Action logger, CancellationToken cancellationToken) + { + try { + using var response = await httpClient.GetAsync (checksumUrl, cancellationToken).ConfigureAwait (false); + response.EnsureSuccessStatusCode (); +#if NET5_0_OR_GREATER + var content = await response.Content.ReadAsStringAsync (cancellationToken).ConfigureAwait (false); +#else + var content = await response.Content.ReadAsStringAsync ().ConfigureAwait (false); +#endif + var checksum = ParseChecksumFile (content); + logger (TraceLevel.Verbose, $"{label}: checksum={checksum}"); + return checksum; + } + catch (OperationCanceledException) { + throw; + } + catch (Exception ex) { + logger (TraceLevel.Warning, $"Could not fetch checksum for {label}: {ex.Message}"); + return null; + } + } + /// Parses "hash filename" or just "hash" from .sha256sum.txt content. public static string? ParseChecksumFile (string content) { diff --git a/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs b/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs index 1b845754..da09283b 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.IO; using System.Runtime.InteropServices; @@ -51,6 +52,61 @@ public static void SystemRename (string sourceFile, string destFile) } } + /// Deletes a file if it exists, logging any failure instead of throwing. + public static void TryDeleteFile (string path, Action logger) + { + if (!File.Exists (path)) + return; + try { File.Delete (path); } + catch (Exception ex) { logger (TraceLevel.Warning, $"Could not delete '{path}': {ex.Message}"); } + } + + /// Recursively deletes a directory if it exists, logging any failure instead of throwing. + public static void TryDeleteDirectory (string path, string label, Action logger) + { + if (!Directory.Exists (path)) + return; + try { Directory.Delete (path, recursive: true); } + catch (Exception ex) { logger (TraceLevel.Warning, $"Could not clean up {label} at '{path}': {ex.Message}"); } + } + + /// Moves a directory to the target path, backing up any existing directory and restoring on failure. + public static void MoveWithRollback (string sourcePath, string targetPath, Action logger) + { + string? backupPath = null; + if (Directory.Exists (targetPath)) { + backupPath = targetPath + $".old-{Guid.NewGuid ():N}"; + Directory.Move (targetPath, backupPath); + } + + var parentDir = Path.GetDirectoryName (targetPath); + if (!string.IsNullOrEmpty (parentDir)) + Directory.CreateDirectory (parentDir); + + try { + Directory.Move (sourcePath, targetPath); + + // Only delete backup after successful move + if (backupPath is not null) + TryDeleteDirectory (backupPath, "old backup", logger); + } + catch (Exception ex) { + logger (TraceLevel.Error, $"Failed to move to '{targetPath}': {ex.Message}"); + if (backupPath is not null && Directory.Exists (backupPath)) { + try { + if (Directory.Exists (targetPath)) + Directory.Delete (targetPath, recursive: true); + Directory.Move (backupPath, targetPath); + logger (TraceLevel.Warning, $"Restored previous directory from backup '{backupPath}'."); + } + catch (Exception restoreEx) { + logger (TraceLevel.Error, $"Failed to restore from backup: {restoreEx.Message}"); + } + } + throw; + } + } + [DllImport ("libc", SetLastError=true)] static extern int rename (string old, string @new); } diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index 09f29775..25f8c885 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -66,7 +66,7 @@ public async Task> DiscoverAsync (CancellationToke if (response.RequestMessage?.RequestUri is not null) info.ResolvedUrl = response.RequestMessage.RequestUri.ToString (); - info.Checksum = await FetchChecksumAsync (info.ChecksumUrl, $"JDK {version}", cancellationToken).ConfigureAwait (false); + info.Checksum = await DownloadUtils.FetchChecksumAsync (httpClient, info.ChecksumUrl, $"JDK {version}", logger, cancellationToken).ConfigureAwait (false); if (string.IsNullOrEmpty (info.Checksum)) logger (TraceLevel.Warning, $"Could not fetch checksum for JDK {version}, integrity verification will be skipped."); @@ -109,7 +109,7 @@ public async Task InstallAsync (int majorVersion, string targetPath, IProgress FetchChecksumAsync (string checksumUrl, string label, CancellationToken cancellationToken) - { - try { - using var response = await httpClient.GetAsync (checksumUrl, cancellationToken).ConfigureAwait (false); - response.EnsureSuccessStatusCode (); -#if NET5_0_OR_GREATER - var content = await response.Content.ReadAsStringAsync (cancellationToken).ConfigureAwait (false); -#else - var content = await response.Content.ReadAsStringAsync ().ConfigureAwait (false); -#endif - var checksum = DownloadUtils.ParseChecksumFile (content); - logger (TraceLevel.Verbose, $"{label}: checksum={checksum}"); - return checksum; - } - catch (OperationCanceledException) { - throw; - } - catch (Exception ex) { - logger (TraceLevel.Warning, $"Could not fetch checksum for {label}: {ex.Message}"); - return null; - } - } - - void TryDeleteFile (string path) - { - if (!File.Exists (path)) - return; - try { File.Delete (path); } - catch (Exception ex) { logger (TraceLevel.Warning, $"Could not delete '{path}': {ex.Message}"); } - } - - void TryDeleteDirectory (string path, string label) - { - if (!Directory.Exists (path)) - return; - try { Directory.Delete (path, recursive: true); } - catch (Exception ex) { logger (TraceLevel.Warning, $"Could not clean up {label} at '{path}': {ex.Message}"); } - } static string GetMicrosoftOpenJDKOSName () { From e3f4188ae9f7ac359cf0d68a81625754b12193c2 Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Wed, 25 Feb 2026 23:10:41 +0000 Subject: [PATCH 08/24] Move file/path utilities from JdkInstaller to FileUtil Extracted IsTargetPathWritable, IsUnderDirectory, GetInstallerExtension, and GetArchiveExtension to FileUtil for reuse across the codebase. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../FileUtil.cs | 64 ++++++++++++++++ .../JdkInstaller.cs | 75 ++----------------- 2 files changed, 69 insertions(+), 70 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs b/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs index da09283b..f712a507 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs @@ -107,6 +107,70 @@ public static void MoveWithRollback (string sourcePath, string targetPath, Actio } } + /// Checks if the target path is writable by testing write access on the nearest existing ancestor. + public static bool IsTargetPathWritable (string targetPath, Action logger) + { + if (string.IsNullOrEmpty (targetPath)) + return false; + + string normalizedPath; + try { + normalizedPath = Path.GetFullPath (targetPath); + } + catch { + normalizedPath = targetPath; + } + + if (OS.IsWindows) { + var programFiles = Environment.GetFolderPath (Environment.SpecialFolder.ProgramFiles); + var programFilesX86 = Environment.GetFolderPath (Environment.SpecialFolder.ProgramFilesX86); + if (IsUnderDirectory (normalizedPath, programFiles) || IsUnderDirectory (normalizedPath, programFilesX86)) { + logger (TraceLevel.Warning, $"Target path '{targetPath}' is in Program Files which typically requires elevation."); + return false; + } + } + + try { + var testDir = normalizedPath; + while (!string.IsNullOrEmpty (testDir) && !Directory.Exists (testDir)) + testDir = Path.GetDirectoryName (testDir); + + if (string.IsNullOrEmpty (testDir)) + return false; + + var testFile = Path.Combine (testDir, $".write-test-{Guid.NewGuid ()}"); + using (File.Create (testFile, 1, FileOptions.DeleteOnClose)) { } + return true; + } + catch (Exception ex) { + logger (TraceLevel.Warning, $"Target path '{targetPath}' is not writable: {ex.Message}"); + return false; + } + } + + /// Checks if a path is under a given directory. + public static bool IsUnderDirectory (string path, string directory) + { + if (string.IsNullOrEmpty (directory) || string.IsNullOrEmpty (path)) + return false; + if (path.Equals (directory, StringComparison.OrdinalIgnoreCase)) + return true; + return path.StartsWith (directory + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase); + } + + // Returns .msi (Windows), .pkg (macOS), or null (Linux) + public static string? GetInstallerExtension () + { + if (OS.IsWindows) return ".msi"; + if (OS.IsMac) return ".pkg"; + return null; + } + + public static string GetArchiveExtension () + { + return OS.IsWindows ? ".zip" : ".tar.gz"; + } + [DllImport ("libc", SetLastError=true)] static extern int rename (string old, string @new); } diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index 25f8c885..0275f228 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -95,13 +95,13 @@ public async Task InstallAsync (int majorVersion, string targetPath, IProgress? progress, CancellationToken cancellationToken) { - var installerExt = GetInstallerExtension ()!; + var installerExt = FileUtil.GetInstallerExtension ()!; var info = BuildVersionInfo (majorVersion, installerExt); // Fetch checksum before download for supply-chain integrity @@ -263,49 +263,6 @@ async Task RunPlatformInstallerAsync (string installerPath, CancellationToken ca } } - /// Checks if the target path is writable and not in a restricted location. - public bool IsTargetPathWritable (string targetPath) - { - if (string.IsNullOrEmpty (targetPath)) - return false; - - // Normalize the path to prevent path traversal bypasses (e.g., "C:\Program Files\..\Users") - string normalizedPath; - try { - normalizedPath = Path.GetFullPath (targetPath); - } - catch { - normalizedPath = targetPath; - } - - if (OS.IsWindows) { - var programFiles = Environment.GetFolderPath (Environment.SpecialFolder.ProgramFiles); - var programFilesX86 = Environment.GetFolderPath (Environment.SpecialFolder.ProgramFilesX86); - if (IsUnderDirectory (normalizedPath, programFiles) || IsUnderDirectory (normalizedPath, programFilesX86)) { - logger (TraceLevel.Warning, $"Target path '{targetPath}' is in Program Files which typically requires elevation."); - return false; - } - } - - // Test writability on the nearest existing ancestor directory without creating the target - try { - var testDir = normalizedPath; - while (!string.IsNullOrEmpty (testDir) && !Directory.Exists (testDir)) - testDir = Path.GetDirectoryName (testDir); - - if (string.IsNullOrEmpty (testDir)) - return false; - - var testFile = Path.Combine (testDir, $".write-test-{Guid.NewGuid ()}"); - using (File.Create (testFile, 1, FileOptions.DeleteOnClose)) { } - return true; - } - catch (Exception ex) { - logger (TraceLevel.Warning, $"Target path '{targetPath}' is not writable: {ex.Message}"); - return false; - } - } - /// Removes a JDK installation at the specified path. public bool Remove (string jdkPath) { @@ -328,7 +285,7 @@ static JdkVersionInfo BuildVersionInfo (int majorVersion, string? extensionOverr { var os = GetMicrosoftOpenJDKOSName (); var arch = GetArchitectureName (); - var ext = extensionOverride ?? GetArchiveExtension (); + var ext = extensionOverride ?? FileUtil.GetArchiveExtension (); var filename = $"microsoft-jdk-{majorVersion}-{os}-{arch}{ext}"; var downloadUrl = $"{DownloadUrlBase}/{filename}"; @@ -398,19 +355,6 @@ static string GetArchitectureName () }; } - static string GetArchiveExtension () - { - return OS.IsWindows ? ".zip" : ".tar.gz"; - } - - // Returns .msi (Windows), .pkg (macOS), or null (Linux) - static string? GetInstallerExtension () - { - if (OS.IsWindows) return ".msi"; - if (OS.IsMac) return ".pkg"; - return null; - } - /// Checks if running as Administrator (Windows) or root (macOS/Linux). public static bool IsElevated () { @@ -449,15 +393,6 @@ static bool IsElevatedUnix () #endif } - static bool IsUnderDirectory (string path, string directory) - { - if (string.IsNullOrEmpty (directory) || string.IsNullOrEmpty (path)) - return false; - if (path.Equals (directory, StringComparison.OrdinalIgnoreCase)) - return true; - return path.StartsWith (directory + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase); - } - [DllImport ("libc", SetLastError = true)] static extern uint geteuid (); } From 4efe463631c204618e2e8aa5a0cfe4d24a7d1f79 Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Wed, 25 Feb 2026 23:16:45 +0000 Subject: [PATCH 09/24] Fix test compilation and remove dead code - Fix IsTargetPathWritable tests to use FileUtil static method - Remove redundant checksum null check (already enforced above) - Remove double blank line Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs | 5 ----- .../JdkInstallerTests.cs | 7 ++++--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index 0275f228..df4beb81 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -198,10 +198,6 @@ await DownloadUtils.DownloadFileAsync (httpClient, info.DownloadUrl, tempInstall progress is null ? null : new Progress<(double pct, string msg)> (p => progress.Report (new JdkInstallProgress (JdkInstallPhase.Downloading, p.pct, p.msg))), cancellationToken).ConfigureAwait (false); - // Verify checksum — mandatory for supply-chain integrity - if (string.IsNullOrEmpty (info.Checksum)) - throw new InvalidOperationException ($"Checksum could not be retrieved for installer. Aborting to preserve supply-chain integrity."); - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 0, "Verifying SHA-256 checksum...")); DownloadUtils.VerifyChecksum (tempInstallerPath, info.Checksum!); progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 100, "Checksum verified.")); @@ -337,7 +333,6 @@ async Task ExtractArchiveAsync (string archivePath, string targetPath, Cancellat } } - static string GetMicrosoftOpenJDKOSName () { if (OS.IsMac) return "macOS"; diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInstallerTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInstallerTests.cs index b312074e..a8db0635 100644 --- a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInstallerTests.cs +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInstallerTests.cs @@ -228,7 +228,7 @@ public void IsTargetPathWritable_TempDir_ReturnsTrue () { var dir = Path.Combine (Path.GetTempPath (), $"jdk-write-test-{Guid.NewGuid ()}"); try { - Assert.IsTrue (installer.IsTargetPathWritable (dir)); + Assert.IsTrue (FileUtil.IsTargetPathWritable (dir, (level, msg) => TestContext.WriteLine ($"[{level}] {msg}"))); } finally { if (Directory.Exists (dir)) @@ -239,8 +239,9 @@ public void IsTargetPathWritable_TempDir_ReturnsTrue () [Test] public void IsTargetPathWritable_NullOrEmpty_ReturnsFalse () { - Assert.IsFalse (installer.IsTargetPathWritable (null!)); - Assert.IsFalse (installer.IsTargetPathWritable ("")); + var logger = new Action ((level, msg) => TestContext.WriteLine ($"[{level}] {msg}")); + Assert.IsFalse (FileUtil.IsTargetPathWritable (null!, logger)); + Assert.IsFalse (FileUtil.IsTargetPathWritable ("", logger)); } [Test] From 43c96066340449231ea8bf76dcb3779664f1f633 Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Thu, 26 Feb 2026 13:19:43 +0000 Subject: [PATCH 10/24] Change IsElevated and IsTargetPathWritable to internal Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs | 2 +- src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs b/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs index f712a507..22e8648d 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs @@ -108,7 +108,7 @@ public static void MoveWithRollback (string sourcePath, string targetPath, Actio } /// Checks if the target path is writable by testing write access on the nearest existing ancestor. - public static bool IsTargetPathWritable (string targetPath, Action logger) + internal static bool IsTargetPathWritable (string targetPath, Action logger) { if (string.IsNullOrEmpty (targetPath)) return false; diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index df4beb81..8ca1d4ce 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -351,7 +351,7 @@ static string GetArchitectureName () } /// Checks if running as Administrator (Windows) or root (macOS/Linux). - public static bool IsElevated () + internal static bool IsElevated () { if (OS.IsWindows) { #if NET5_0_OR_GREATER From 1c1e8a139826f3269de015f4dbeae8b8fd10021b Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Thu, 26 Feb 2026 14:10:20 +0000 Subject: [PATCH 11/24] Address PR review: rollback safety, cancellation handling, test timeouts - MoveWithRollback: defer backup deletion until after validation succeeds - Add CommitMove() to explicitly clean up backups post-validation - InstallAsync: clarify docs that elevated path ignores targetPath - Add dedicated OperationCanceledException catch (not logged as error) - DiscoverAsync test: add 15s CancellationToken timeout for CI reliability Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../FileUtil.cs | 34 +++++++++++++------ .../JdkInstaller.cs | 12 ++++++- .../JdkInstallerTests.cs | 6 ++-- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs b/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs index 22e8648d..2a5e4990 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs @@ -53,7 +53,7 @@ public static void SystemRename (string sourceFile, string destFile) } /// Deletes a file if it exists, logging any failure instead of throwing. - public static void TryDeleteFile (string path, Action logger) + internal static void TryDeleteFile (string path, Action logger) { if (!File.Exists (path)) return; @@ -62,7 +62,7 @@ public static void TryDeleteFile (string path, Action logger } /// Recursively deletes a directory if it exists, logging any failure instead of throwing. - public static void TryDeleteDirectory (string path, string label, Action logger) + internal static void TryDeleteDirectory (string path, string label, Action logger) { if (!Directory.Exists (path)) return; @@ -71,7 +71,7 @@ public static void TryDeleteDirectory (string path, string label, ActionMoves a directory to the target path, backing up any existing directory and restoring on failure. - public static void MoveWithRollback (string sourcePath, string targetPath, Action logger) + internal static void MoveWithRollback (string sourcePath, string targetPath, Action logger) { string? backupPath = null; if (Directory.Exists (targetPath)) { @@ -85,10 +85,6 @@ public static void MoveWithRollback (string sourcePath, string targetPath, Actio try { Directory.Move (sourcePath, targetPath); - - // Only delete backup after successful move - if (backupPath is not null) - TryDeleteDirectory (backupPath, "old backup", logger); } catch (Exception ex) { logger (TraceLevel.Error, $"Failed to move to '{targetPath}': {ex.Message}"); @@ -105,6 +101,24 @@ public static void MoveWithRollback (string sourcePath, string targetPath, Actio } throw; } + + // Delete backup only after move and caller validation succeed + if (backupPath is not null) + TryDeleteDirectory (backupPath, "old backup", logger); + } + + /// Deletes a backup created by MoveWithRollback. Call after validation succeeds. + internal static void CommitMove (string targetPath, Action logger) + { + // Find and clean up any leftover backup directories + var parentDir = Path.GetDirectoryName (targetPath); + if (string.IsNullOrEmpty (parentDir) || !Directory.Exists (parentDir)) + return; + + var dirName = Path.GetFileName (targetPath); + foreach (var dir in Directory.GetDirectories (parentDir, $"{dirName}.old-*")) { + TryDeleteDirectory (dir, "old backup", logger); + } } /// Checks if the target path is writable by testing write access on the nearest existing ancestor. @@ -149,7 +163,7 @@ internal static bool IsTargetPathWritable (string targetPath, ActionChecks if a path is under a given directory. - public static bool IsUnderDirectory (string path, string directory) + internal static bool IsUnderDirectory (string path, string directory) { if (string.IsNullOrEmpty (directory) || string.IsNullOrEmpty (path)) return false; @@ -159,14 +173,14 @@ public static bool IsUnderDirectory (string path, string directory) } // Returns .msi (Windows), .pkg (macOS), or null (Linux) - public static string? GetInstallerExtension () + internal static string? GetInstallerExtension () { if (OS.IsWindows) return ".msi"; if (OS.IsMac) return ".pkg"; return null; } - public static string GetArchiveExtension () + internal static string GetArchiveExtension () { return OS.IsWindows ? ".zip" : ".tar.gz"; } diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index 8ca1d4ce..a2201d28 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -85,7 +85,11 @@ public async Task> DiscoverAsync (CancellationToke return results.AsReadOnly (); } - /// Downloads and installs a Microsoft OpenJDK to the target path. + /// + /// Downloads and installs a Microsoft OpenJDK. + /// When running elevated, uses the platform installer (.msi/.pkg) which chooses its own install location; + /// is ignored in that case. When non-elevated, extracts to . + /// public async Task InstallAsync (int majorVersion, string targetPath, IProgress? progress = null, CancellationToken cancellationToken = default) { if (!SupportedVersions.Contains (majorVersion)) @@ -145,11 +149,17 @@ await DownloadUtils.DownloadFileAsync (httpClient, versionInfo.DownloadUrl, temp FileUtil.TryDeleteDirectory (targetPath, "invalid installation", logger); throw new InvalidOperationException ($"JDK installation at '{targetPath}' failed validation. The extracted files may be corrupted."); } + + // Validation passed — commit the move by cleaning up any backup + FileUtil.CommitMove (targetPath, logger); logger (TraceLevel.Info, $"Microsoft OpenJDK {majorVersion} installed successfully at {targetPath}"); progress?.Report (new JdkInstallProgress (JdkInstallPhase.Validating, 100, "Validation complete.")); progress?.Report (new JdkInstallProgress (JdkInstallPhase.Complete, 100, $"Microsoft OpenJDK {majorVersion} installed successfully.")); } + catch (OperationCanceledException) { + throw; + } catch (Exception ex) when (ex is not ArgumentException and not ArgumentNullException) { logger (TraceLevel.Error, $"JDK installation failed: {ex.Message}"); logger (TraceLevel.Verbose, ex.ToString ()); diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInstallerTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInstallerTests.cs index a8db0635..2e153056 100644 --- a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInstallerTests.cs +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInstallerTests.cs @@ -95,9 +95,11 @@ public async Task DiscoverAsync_ReturnsVersions () { IReadOnlyList versions; try { - versions = await installer.DiscoverAsync (); + using (var cts = new CancellationTokenSource (TimeSpan.FromSeconds (15))) { + versions = await installer.DiscoverAsync (cts.Token); + } } - catch (Exception ex) when (ex is System.Net.Http.HttpRequestException || ex is TaskCanceledException) { + catch (Exception ex) when (ex is System.Net.Http.HttpRequestException || ex is TaskCanceledException || ex is OperationCanceledException) { Assert.Ignore ($"Network unavailable: {ex.Message}"); return; } From d0f3bea2e971b2cf1eefa167105d52ea522adc22 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 08:28:57 -0600 Subject: [PATCH 12/24] Create `ProcessUtils.CreateProcessStartInfo()` --- .../JdkInstaller.cs | 34 ++---------------- .../ProcessUtils.cs | 35 +++++++++++++++++++ 2 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index a2201d28..6a2b96cb 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -226,37 +226,9 @@ await DownloadUtils.DownloadFileAsync (httpClient, info.DownloadUrl, tempInstall async Task RunPlatformInstallerAsync (string installerPath, CancellationToken cancellationToken) { - ProcessStartInfo psi; - if (OS.IsWindows) { - psi = new ProcessStartInfo { - FileName = "msiexec", - UseShellExecute = false, - CreateNoWindow = true, - }; -#if NET5_0_OR_GREATER - psi.ArgumentList.Add ("/i"); - psi.ArgumentList.Add (installerPath); - psi.ArgumentList.Add ("/quiet"); - psi.ArgumentList.Add ("/norestart"); -#else - psi.Arguments = $"/i \"{installerPath}\" /quiet /norestart"; -#endif - } - else { - psi = new ProcessStartInfo { - FileName = "/usr/sbin/installer", - UseShellExecute = false, - CreateNoWindow = true, - }; -#if NET5_0_OR_GREATER - psi.ArgumentList.Add ("-pkg"); - psi.ArgumentList.Add (installerPath); - psi.ArgumentList.Add ("-target"); - psi.ArgumentList.Add ("/"); -#else - psi.Arguments = $"-pkg \"{installerPath}\" -target /"; -#endif - } + var psi = OS.IsWindows + ? ProcessUtils.CreateProcessStartInfo ("msiexec", "/i", installerPath, "/quiet", "/norestart") + : ProcessUtils.CreateProcessStartInfo ("/usr/sbin/installer", "-pkg", installerPath, "-target", "/"); var stdout = new StringWriter (); var stderr = new StringWriter (); diff --git a/src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs index 00074a72..fd8fdaf7 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Text; using System.Threading; using System.Threading.Tasks; @@ -159,6 +160,40 @@ internal static void Exec (ProcessStartInfo processStartInfo, DataReceivedEventH } } + /// + /// Creates a with the given filename and arguments. + /// On .NET 5+ uses to avoid shell-escaping issues; + /// on older frameworks falls back to a single string. + /// + public static ProcessStartInfo CreateProcessStartInfo (string fileName, params string[] args) + { + var psi = new ProcessStartInfo { + FileName = fileName, + UseShellExecute = false, + CreateNoWindow = true, + }; +#if NET5_0_OR_GREATER + foreach (var arg in args) + psi.ArgumentList.Add (arg); +#else + psi.Arguments = JoinArguments (args); +#endif + return psi; + } + +#if !NET5_0_OR_GREATER + static string JoinArguments (string[] args) + { + var sb = new StringBuilder (); + for (int i = 0; i < args.Length; i++) { + if (i > 0) + sb.Append (' '); + sb.Append ('"').Append (args [i]).Append ('"'); + } + return sb.ToString (); + } +#endif + internal static IEnumerable FindExecutablesInPath (string executable) { var path = Environment.GetEnvironmentVariable ("PATH") ?? ""; From 9ed5ddc85b3787ff7317e88f4cad289ca01a6e37 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 08:31:12 -0600 Subject: [PATCH 13/24] More ProcessUtils.CreateProcessStartInfo() --- .../DownloadUtils.cs | 20 +------------------ 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs index c4de3735..aae20c71 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs @@ -92,25 +92,7 @@ public static void ExtractZipSafe (string archivePath, string destinationPath, C /// Extracts a tar.gz archive using the system tar command. public static async Task ExtractTarGzAsync (string archivePath, string destinationPath, Action logger, CancellationToken cancellationToken) { - var psi = new ProcessStartInfo { - FileName = "/usr/bin/tar", - UseShellExecute = false, - CreateNoWindow = true, - }; -#if NET5_0_OR_GREATER - psi.ArgumentList.Add ("-xzf"); - psi.ArgumentList.Add (archivePath); - psi.ArgumentList.Add ("-C"); - psi.ArgumentList.Add (destinationPath); -#else - // ArgumentList is not available on netstandard2.0, so we build Arguments manually. - // Reject characters that could allow argument injection through the shell. - if (archivePath.IndexOfAny (new[] { '"', '\'', '`', '$' }) >= 0) - throw new ArgumentException ($"Archive path contains unsafe characters: '{archivePath}'", nameof (archivePath)); - if (destinationPath.IndexOfAny (new[] { '"', '\'', '`', '$' }) >= 0) - throw new ArgumentException ($"Destination path contains unsafe characters: '{destinationPath}'", nameof (destinationPath)); - psi.Arguments = $"-xzf \"{archivePath}\" -C \"{destinationPath}\""; -#endif + var psi = ProcessUtils.CreateProcessStartInfo ("/usr/bin/tar", "-xzf", archivePath, "-C", destinationPath); var stdout = new StringWriter (); var stderr = new StringWriter (); From 8b7446a577e113b32f614c123a33db6f87634646 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 08:38:42 -0600 Subject: [PATCH 14/24] Move elevation checks to ProcessUtils This also uses `Environment.IsPrivilegedProcess` --- .../JdkInstaller.cs | 44 +------------------ .../ProcessUtils.cs | 25 +++++++++++ 2 files changed, 26 insertions(+), 43 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index 6a2b96cb..b341d14e 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -8,9 +8,6 @@ using System.Linq; using System.Net.Http; using System.Runtime.InteropServices; -#if NET5_0_OR_GREATER -using System.Security.Principal; -#endif using System.Threading; using System.Threading.Tasks; @@ -332,45 +329,6 @@ static string GetArchitectureName () }; } - /// Checks if running as Administrator (Windows) or root (macOS/Linux). - internal static bool IsElevated () - { - if (OS.IsWindows) { -#if NET5_0_OR_GREATER -#pragma warning disable CA1416 // Guarded by OS.IsWindows runtime check above - return IsElevatedWindows (); -#pragma warning restore CA1416 -#else - return false; -#endif - } - // Unix: geteuid() == 0 means root - return IsElevatedUnix (); - } - -#if NET5_0_OR_GREATER - [System.Runtime.Versioning.SupportedOSPlatform ("windows")] - static bool IsElevatedWindows () - { - using var identity = WindowsIdentity.GetCurrent (); - var principal = new WindowsPrincipal (identity); - return principal.IsInRole (WindowsBuiltInRole.Administrator); - } -#endif - - // Separate method so geteuid P/Invoke is never JIT-compiled on Windows - static bool IsElevatedUnix () - { -#if NET5_0_OR_GREATER - if (!OperatingSystem.IsWindows ()) - return geteuid () == 0; - return false; -#else - return geteuid () == 0; -#endif - } - - [DllImport ("libc", SetLastError = true)] - static extern uint geteuid (); + static bool IsElevated () => ProcessUtils.IsElevated (); } } diff --git a/src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs index fd8fdaf7..86dce45b 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs @@ -6,6 +6,10 @@ using System.Threading; using System.Threading.Tasks; +#if !NET5_0_OR_GREATER +using System.Runtime.InteropServices; +#endif + namespace Xamarin.Android.Tools { public static class ProcessUtils @@ -233,6 +237,27 @@ internal static IEnumerable ExecutableFiles (string executable) yield return Path.ChangeExtension (executable, ext); yield return executable; } + + /// Checks if running as Administrator (Windows) or root (macOS/Linux). + public static bool IsElevated () + { +#if NET5_0_OR_GREATER + return Environment.IsPrivilegedProcess; +#else + if (OS.IsWindows) + return IsUserAnAdmin (); + return geteuid () == 0; +#endif + } + +#if !NET5_0_OR_GREATER + [DllImport ("shell32.dll")] + [return: MarshalAs (UnmanagedType.Bool)] + static extern bool IsUserAnAdmin (); + + [DllImport ("libc", SetLastError = true)] + static extern uint geteuid (); +#endif } } From 3f2177bc2cc98275bd330b59b8cb6be62a70320c Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 08:40:01 -0600 Subject: [PATCH 15/24] Missing `using` --- src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs | 4 ++-- src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs index aae20c71..241eed86 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs @@ -94,8 +94,8 @@ public static async Task ExtractTarGzAsync (string archivePath, string destinati { var psi = ProcessUtils.CreateProcessStartInfo ("/usr/bin/tar", "-xzf", archivePath, "-C", destinationPath); - var stdout = new StringWriter (); - var stderr = new StringWriter (); + using var stdout = new StringWriter (); + using var stderr = new StringWriter (); var exitCode = await ProcessUtils.StartProcess (psi, stdout: stdout, stderr: stderr, cancellationToken).ConfigureAwait (false); if (exitCode != 0) { diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index b341d14e..a98b50e4 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -227,8 +227,8 @@ async Task RunPlatformInstallerAsync (string installerPath, CancellationToken ca ? ProcessUtils.CreateProcessStartInfo ("msiexec", "/i", installerPath, "/quiet", "/norestart") : ProcessUtils.CreateProcessStartInfo ("/usr/sbin/installer", "-pkg", installerPath, "-target", "/"); - var stdout = new StringWriter (); - var stderr = new StringWriter (); + using var stdout = new StringWriter (); + using var stderr = new StringWriter (); var exitCode = await ProcessUtils.StartProcess (psi, stdout: stdout, stderr: stderr, cancellationToken).ConfigureAwait (false); if (exitCode != 0) { From 48ede26d29d9a677f14538fadeb5508f6f7e85da Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 08:44:05 -0600 Subject: [PATCH 16/24] Simplify nullable `progress?` --- .../JdkInstaller.cs | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index a98b50e4..2ca4facf 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -28,6 +28,8 @@ public class JdkInstaller : IDisposable /// Gets the supported JDK major versions available for installation. public static IReadOnlyList SupportedVersions { get; } = [ RecommendedMajorVersion ]; + static readonly IProgress NullProgress = new Progress (); + readonly HttpClient httpClient = new(); readonly Action logger; @@ -89,6 +91,8 @@ public async Task> DiscoverAsync (CancellationToke /// public async Task InstallAsync (int majorVersion, string targetPath, IProgress? progress = null, CancellationToken cancellationToken = default) { + progress ??= NullProgress; + if (!SupportedVersions.Contains (majorVersion)) throw new ArgumentException ($"JDK version {majorVersion} is not supported. Supported versions: {string.Join (", ", SupportedVersions)}", nameof (majorVersion)); @@ -120,27 +124,27 @@ public async Task InstallAsync (int majorVersion, string targetPath, IProgress (p => progress.Report (new JdkInstallProgress (JdkInstallPhase.Downloading, p.pct, p.msg))), + new Progress<(double pct, string msg)> (p => progress.Report (new JdkInstallProgress (JdkInstallPhase.Downloading, p.pct, p.msg))), cancellationToken).ConfigureAwait (false); logger (TraceLevel.Info, $"Download complete: {tempArchivePath}"); // Verify checksum - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 0, "Verifying SHA-256 checksum...")); + progress.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 0, "Verifying SHA-256 checksum...")); DownloadUtils.VerifyChecksum (tempArchivePath, versionInfo.Checksum!); logger (TraceLevel.Info, "Checksum verified."); - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 100, "Checksum verified.")); + progress.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 100, "Checksum verified.")); // Extract logger (TraceLevel.Info, $"Extracting JDK to {targetPath}"); - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Extracting, 0, "Extracting JDK...")); + progress.Report (new JdkInstallProgress (JdkInstallPhase.Extracting, 0, "Extracting JDK...")); await ExtractArchiveAsync (tempArchivePath, targetPath, cancellationToken).ConfigureAwait (false); logger (TraceLevel.Info, "Extraction complete."); - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Extracting, 100, "Extraction complete.")); + progress.Report (new JdkInstallProgress (JdkInstallPhase.Extracting, 100, "Extraction complete.")); // Validate - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Validating, 0, "Validating installation...")); + progress.Report (new JdkInstallProgress (JdkInstallPhase.Validating, 0, "Validating installation...")); if (!IsValid (targetPath)) { logger (TraceLevel.Error, $"JDK installation at '{targetPath}' failed validation."); FileUtil.TryDeleteDirectory (targetPath, "invalid installation", logger); @@ -150,9 +154,9 @@ await DownloadUtils.DownloadFileAsync (httpClient, versionInfo.DownloadUrl, temp // Validation passed — commit the move by cleaning up any backup FileUtil.CommitMove (targetPath, logger); logger (TraceLevel.Info, $"Microsoft OpenJDK {majorVersion} installed successfully at {targetPath}"); - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Validating, 100, "Validation complete.")); + progress.Report (new JdkInstallProgress (JdkInstallPhase.Validating, 100, "Validation complete.")); - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Complete, 100, $"Microsoft OpenJDK {majorVersion} installed successfully.")); + progress.Report (new JdkInstallProgress (JdkInstallPhase.Complete, 100, $"Microsoft OpenJDK {majorVersion} installed successfully.")); } catch (OperationCanceledException) { throw; @@ -184,7 +188,7 @@ public bool IsValid (string jdkPath) } } - async Task InstallWithPlatformInstallerAsync (int majorVersion, IProgress? progress, CancellationToken cancellationToken) + async Task InstallWithPlatformInstallerAsync (int majorVersion, IProgress progress, CancellationToken cancellationToken) { var installerExt = FileUtil.GetInstallerExtension ()!; var info = BuildVersionInfo (majorVersion, installerExt); @@ -200,21 +204,21 @@ async Task InstallWithPlatformInstallerAsync (int majorVersion, IProgress (p => progress.Report (new JdkInstallProgress (JdkInstallPhase.Downloading, p.pct, p.msg))), + new Progress<(double pct, string msg)> (p => progress.Report (new JdkInstallProgress (JdkInstallPhase.Downloading, p.pct, p.msg))), cancellationToken).ConfigureAwait (false); - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 0, "Verifying SHA-256 checksum...")); + progress.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 0, "Verifying SHA-256 checksum...")); DownloadUtils.VerifyChecksum (tempInstallerPath, info.Checksum!); - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 100, "Checksum verified.")); + progress.Report (new JdkInstallProgress (JdkInstallPhase.Verifying, 100, "Checksum verified.")); // Run the installer silently - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Extracting, 0, "Running platform installer...")); + progress.Report (new JdkInstallProgress (JdkInstallPhase.Extracting, 0, "Running platform installer...")); await RunPlatformInstallerAsync (tempInstallerPath, cancellationToken).ConfigureAwait (false); logger (TraceLevel.Info, $"Microsoft OpenJDK {majorVersion} installed successfully via platform installer."); - progress?.Report (new JdkInstallProgress (JdkInstallPhase.Complete, 100, $"Microsoft OpenJDK {majorVersion} installed successfully.")); + progress.Report (new JdkInstallProgress (JdkInstallPhase.Complete, 100, $"Microsoft OpenJDK {majorVersion} installed successfully.")); } finally { FileUtil.TryDeleteFile (tempInstallerPath, logger); From 7c85bd7c61ba9ad446446f8eb74d235b3a113aa7 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 08:48:57 -0600 Subject: [PATCH 17/24] Simplify `DownloadUtils.ParseChecksumFile()` write unit tests --- .../DownloadUtils.cs | 6 +- .../DownloadUtilsTests.cs | 76 +++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 tests/Xamarin.Android.Tools.AndroidSdk-Tests/DownloadUtilsTests.cs diff --git a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs index 241eed86..415b2540 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs @@ -135,9 +135,9 @@ public static async Task ExtractTarGzAsync (string archivePath, string destinati if (string.IsNullOrWhiteSpace (content)) return null; - var line = content.Trim ().Split ('\n') [0].Trim (); - var parts = line.Split (new[] { ' ', '\t' }, StringSplitOptions.RemoveEmptyEntries); - return parts.Length > 0 ? parts [0] : null; + var trimmed = content.Trim (); + var end = trimmed.IndexOfAny ([' ', '\t', '\n', '\r']); + return end >= 0 ? trimmed[..end] : trimmed; } } } diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/DownloadUtilsTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/DownloadUtilsTests.cs new file mode 100644 index 00000000..d966e68c --- /dev/null +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/DownloadUtilsTests.cs @@ -0,0 +1,76 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using NUnit.Framework; + +namespace Xamarin.Android.Tools.Tests +{ + [TestFixture] + public class DownloadUtilsTests + { + [Test] + public void ParseChecksumFile_Null_ReturnsNull () + { + Assert.IsNull (DownloadUtils.ParseChecksumFile (null!)); + } + + [Test] + public void ParseChecksumFile_Empty_ReturnsNull () + { + Assert.IsNull (DownloadUtils.ParseChecksumFile ("")); + } + + [Test] + public void ParseChecksumFile_WhitespaceOnly_ReturnsNull () + { + Assert.IsNull (DownloadUtils.ParseChecksumFile (" \n\t ")); + } + + [Test] + public void ParseChecksumFile_HashOnly () + { + Assert.AreEqual ("abc123def456", DownloadUtils.ParseChecksumFile ("abc123def456")); + } + + [Test] + public void ParseChecksumFile_HashOnly_WithTrailingNewline () + { + Assert.AreEqual ("abc123def456", DownloadUtils.ParseChecksumFile ("abc123def456\n")); + } + + [Test] + public void ParseChecksumFile_HashAndFilename () + { + // Standard sha256sum format: " " + Assert.AreEqual ("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + DownloadUtils.ParseChecksumFile ("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 microsoft-jdk-21-linux-x64.tar.gz")); + } + + [Test] + public void ParseChecksumFile_HashAndFilename_WithTab () + { + Assert.AreEqual ("abc123", DownloadUtils.ParseChecksumFile ("abc123\tfilename.zip")); + } + + [Test] + public void ParseChecksumFile_MultipleLines_ReturnsFirstHash () + { + var content = "abc123 file1.zip\ndef456 file2.zip\n"; + Assert.AreEqual ("abc123", DownloadUtils.ParseChecksumFile (content)); + } + + [Test] + public void ParseChecksumFile_LeadingAndTrailingWhitespace () + { + Assert.AreEqual ("abc123", DownloadUtils.ParseChecksumFile (" abc123 filename.zip \n")); + } + + [TestCase ("abc123\r\n")] + [TestCase ("abc123\r")] + [TestCase ("abc123\n")] + public void ParseChecksumFile_VariousLineEndings (string content) + { + Assert.AreEqual ("abc123", DownloadUtils.ParseChecksumFile (content)); + } + } +} From e8e9db78c350e63aecb495867265a6414b87c37a Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 08:59:15 -0600 Subject: [PATCH 18/24] Remove `IsElevated()` --- src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs index 2ca4facf..c9825425 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInstaller.cs @@ -100,7 +100,7 @@ public async Task InstallAsync (int majorVersion, string targetPath, IProgress throw new PlatformNotSupportedException ($"Unsupported architecture: {RuntimeInformation.OSArchitecture}"), }; } - - static bool IsElevated () => ProcessUtils.IsElevated (); } } From 6e4d11749cd855e1305c9335a5292e552fd6c033 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 09:08:18 -0600 Subject: [PATCH 19/24] MOAR Tests! --- .../DownloadUtilsTests.cs | 145 +++++++++++++++++ .../FileUtilTests.cs | 151 ++++++++++++++++++ .../JdkVersionInfoTests.cs | 71 ++++++++ .../ProcessUtilsTests.cs | 70 ++++++++ 4 files changed, 437 insertions(+) create mode 100644 tests/Xamarin.Android.Tools.AndroidSdk-Tests/FileUtilTests.cs create mode 100644 tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkVersionInfoTests.cs create mode 100644 tests/Xamarin.Android.Tools.AndroidSdk-Tests/ProcessUtilsTests.cs diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/DownloadUtilsTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/DownloadUtilsTests.cs index d966e68c..995870df 100644 --- a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/DownloadUtilsTests.cs +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/DownloadUtilsTests.cs @@ -1,6 +1,12 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; +using System.IO; +using System.IO.Compression; +using System.Security.Cryptography; +using System.Threading; + using NUnit.Framework; namespace Xamarin.Android.Tools.Tests @@ -8,6 +14,21 @@ namespace Xamarin.Android.Tools.Tests [TestFixture] public class DownloadUtilsTests { + string tempDir = null!; + + [SetUp] + public void SetUp () + { + tempDir = Path.Combine (Path.GetTempPath (), $"DownloadUtilsTests-{Guid.NewGuid ():N}"); + Directory.CreateDirectory (tempDir); + } + + [TearDown] + public void TearDown () + { + if (Directory.Exists (tempDir)) + Directory.Delete (tempDir, recursive: true); + } [Test] public void ParseChecksumFile_Null_ReturnsNull () { @@ -72,5 +93,129 @@ public void ParseChecksumFile_VariousLineEndings (string content) { Assert.AreEqual ("abc123", DownloadUtils.ParseChecksumFile (content)); } + + // --- VerifyChecksum tests --- + + [Test] + public void VerifyChecksum_MatchingHash_DoesNotThrow () + { + var filePath = Path.Combine (tempDir, "test.bin"); + var content = new byte [] { 0x48, 0x65, 0x6c, 0x6c, 0x6f }; // "Hello" + File.WriteAllBytes (filePath, content); + + using var sha = SHA256.Create (); + var expected = BitConverter.ToString (sha.ComputeHash (content)).Replace ("-", "").ToLowerInvariant (); + + Assert.DoesNotThrow (() => DownloadUtils.VerifyChecksum (filePath, expected)); + } + + [Test] + public void VerifyChecksum_MismatchedHash_Throws () + { + var filePath = Path.Combine (tempDir, "test.bin"); + File.WriteAllBytes (filePath, new byte [] { 1, 2, 3 }); + + var ex = Assert.Throws (() => + DownloadUtils.VerifyChecksum (filePath, "0000000000000000000000000000000000000000000000000000000000000000")); + Assert.That (ex!.Message, Does.Contain ("Checksum verification failed")); + } + + [Test] + public void VerifyChecksum_CaseInsensitive () + { + var filePath = Path.Combine (tempDir, "test.bin"); + var content = new byte [] { 0xFF }; + File.WriteAllBytes (filePath, content); + + using var sha = SHA256.Create (); + var upperHash = BitConverter.ToString (sha.ComputeHash (content)).Replace ("-", "").ToUpperInvariant (); + + Assert.DoesNotThrow (() => DownloadUtils.VerifyChecksum (filePath, upperHash)); + } + + [Test] + public void VerifyChecksum_NonExistentFile_Throws () + { + var filePath = Path.Combine (tempDir, "nonexistent.bin"); + Assert.Throws (() => + DownloadUtils.VerifyChecksum (filePath, "abc123")); + } + + // --- ExtractZipSafe tests --- + + [Test] + public void ExtractZipSafe_ValidZip_ExtractsFiles () + { + var zipPath = Path.Combine (tempDir, "test.zip"); + var extractPath = Path.Combine (tempDir, "extracted"); + Directory.CreateDirectory (extractPath); + + using (var archive = ZipFile.Open (zipPath, ZipArchiveMode.Create)) { + var entry = archive.CreateEntry ("subdir/hello.txt"); + using var writer = new StreamWriter (entry.Open ()); + writer.Write ("hello world"); + } + + DownloadUtils.ExtractZipSafe (zipPath, extractPath, CancellationToken.None); + + var extractedFile = Path.Combine (extractPath, "subdir", "hello.txt"); + Assert.IsTrue (File.Exists (extractedFile), "Extracted file should exist"); + Assert.AreEqual ("hello world", File.ReadAllText (extractedFile)); + } + + [Test] + public void ExtractZipSafe_ZipSlip_Throws () + { + var zipPath = Path.Combine (tempDir, "evil.zip"); + var extractPath = Path.Combine (tempDir, "extracted"); + Directory.CreateDirectory (extractPath); + + using (var archive = ZipFile.Open (zipPath, ZipArchiveMode.Create)) { + // Create an entry with a path traversal + var entry = archive.CreateEntry ("../evil.txt"); + using var writer = new StreamWriter (entry.Open ()); + writer.Write ("malicious"); + } + + var ex = Assert.Throws (() => + DownloadUtils.ExtractZipSafe (zipPath, extractPath, CancellationToken.None)); + Assert.That (ex!.Message, Does.Contain ("outside target directory")); + } + + [Test] + public void ExtractZipSafe_EmptyZip_NoFilesExtracted () + { + var zipPath = Path.Combine (tempDir, "empty.zip"); + var extractPath = Path.Combine (tempDir, "extracted"); + Directory.CreateDirectory (extractPath); + + using (ZipFile.Open (zipPath, ZipArchiveMode.Create)) { } + + DownloadUtils.ExtractZipSafe (zipPath, extractPath, CancellationToken.None); + + Assert.AreEqual (0, Directory.GetFiles (extractPath, "*", SearchOption.AllDirectories).Length); + } + + [Test] + public void ExtractZipSafe_CancellationToken_Throws () + { + var zipPath = Path.Combine (tempDir, "test.zip"); + var extractPath = Path.Combine (tempDir, "extracted"); + Directory.CreateDirectory (extractPath); + + using (var archive = ZipFile.Open (zipPath, ZipArchiveMode.Create)) { + for (int i = 0; i < 10; i++) { + var entry = archive.CreateEntry ($"file{i}.txt"); + using var writer = new StreamWriter (entry.Open ()); + writer.Write ($"content {i}"); + } + } + + using var cts = new CancellationTokenSource (); + cts.Cancel (); + + Assert.Throws (() => + DownloadUtils.ExtractZipSafe (zipPath, extractPath, cts.Token)); + } } } diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/FileUtilTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/FileUtilTests.cs new file mode 100644 index 00000000..c94d2cc2 --- /dev/null +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/FileUtilTests.cs @@ -0,0 +1,151 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.IO; + +using NUnit.Framework; + +namespace Xamarin.Android.Tools.Tests +{ + [TestFixture] + public class FileUtilTests + { + string tempDir = null!; + Action logger = null!; + + [SetUp] + public void SetUp () + { + tempDir = Path.Combine (Path.GetTempPath (), $"FileUtilTests-{Guid.NewGuid ():N}"); + Directory.CreateDirectory (tempDir); + logger = (level, msg) => TestContext.WriteLine ($"[{level}] {msg}"); + } + + [TearDown] + public void TearDown () + { + if (Directory.Exists (tempDir)) + Directory.Delete (tempDir, recursive: true); + } + + [Test] + public void MoveWithRollback_NewTarget_Succeeds () + { + var source = Path.Combine (tempDir, "source"); + var target = Path.Combine (tempDir, "target"); + Directory.CreateDirectory (source); + File.WriteAllText (Path.Combine (source, "file.txt"), "hello"); + + FileUtil.MoveWithRollback (source, target, logger); + + Assert.IsFalse (Directory.Exists (source), "Source should no longer exist"); + Assert.IsTrue (Directory.Exists (target), "Target should exist"); + Assert.AreEqual ("hello", File.ReadAllText (Path.Combine (target, "file.txt"))); + } + + [Test] + public void MoveWithRollback_ExistingTarget_BacksUpAndReplaces () + { + var source = Path.Combine (tempDir, "source"); + var target = Path.Combine (tempDir, "target"); + Directory.CreateDirectory (source); + File.WriteAllText (Path.Combine (source, "new.txt"), "new"); + + Directory.CreateDirectory (target); + File.WriteAllText (Path.Combine (target, "old.txt"), "old"); + + FileUtil.MoveWithRollback (source, target, logger); + + Assert.IsFalse (Directory.Exists (source), "Source should no longer exist"); + Assert.IsTrue (File.Exists (Path.Combine (target, "new.txt")), "New file should exist"); + Assert.IsFalse (File.Exists (Path.Combine (target, "old.txt")), "Old file should be gone"); + } + + [Test] + public void MoveWithRollback_SourceDoesNotExist_RestoresBackup () + { + var source = Path.Combine (tempDir, "nonexistent"); + var target = Path.Combine (tempDir, "target"); + + // Create an existing target that should be backed up and restored + Directory.CreateDirectory (target); + File.WriteAllText (Path.Combine (target, "original.txt"), "preserve me"); + + Assert.Throws (() => FileUtil.MoveWithRollback (source, target, logger)); + + // The original target should be restored from backup + Assert.IsTrue (Directory.Exists (target), "Target should be restored"); + Assert.AreEqual ("preserve me", File.ReadAllText (Path.Combine (target, "original.txt"))); + } + + [Test] + public void MoveWithRollback_SourceDoesNotExist_NoExistingTarget_Throws () + { + var source = Path.Combine (tempDir, "nonexistent"); + var target = Path.Combine (tempDir, "also-nonexistent"); + + Assert.Throws (() => FileUtil.MoveWithRollback (source, target, logger)); + Assert.IsFalse (Directory.Exists (target)); + } + + [Test] + public void IsUnderDirectory_ChildPath_ReturnsTrue () + { + var parent = Path.Combine ($"{Path.DirectorySeparatorChar}opt", "programs"); + var child = Path.Combine (parent, "java", "jdk-21"); + Assert.IsTrue (FileUtil.IsUnderDirectory (child, parent)); + } + + [Test] + public void IsUnderDirectory_ExactMatch_ReturnsTrue () + { + var dir = Path.Combine ($"{Path.DirectorySeparatorChar}opt", "programs"); + Assert.IsTrue (FileUtil.IsUnderDirectory (dir, dir)); + } + + [Test] + public void IsUnderDirectory_SiblingPath_ReturnsFalse () + { + Assert.IsFalse (FileUtil.IsUnderDirectory ( + Path.Combine ($"{Path.DirectorySeparatorChar}opt", "data", "java"), + Path.Combine ($"{Path.DirectorySeparatorChar}opt", "programs"))); + } + + [Test] + public void IsUnderDirectory_DifferentRoot_ReturnsFalse () + { + Assert.IsFalse (FileUtil.IsUnderDirectory ( + Path.Combine ($"{Path.DirectorySeparatorChar}other", "java"), + Path.Combine ($"{Path.DirectorySeparatorChar}opt", "programs"))); + } + + [TestCase (null, "/dir")] + [TestCase ("/dir", null)] + [TestCase ("", "/dir")] + [TestCase ("/dir", "")] + [TestCase (null, null)] + public void IsUnderDirectory_NullOrEmpty_ReturnsFalse (string path, string directory) + { + Assert.IsFalse (FileUtil.IsUnderDirectory (path!, directory!)); + } + + [Test] + public void IsUnderDirectory_CaseInsensitive () + { + var parent = Path.Combine ($"{Path.DirectorySeparatorChar}opt", "Programs"); + var child = Path.Combine ($"{Path.DirectorySeparatorChar}opt", "PROGRAMS", "java"); + Assert.IsTrue (FileUtil.IsUnderDirectory (child, parent)); + } + + [Test] + public void IsUnderDirectory_PartialDirNameMatch_ReturnsFalse () + { + var parent = Path.Combine ($"{Path.DirectorySeparatorChar}opt", "programs"); + Assert.IsFalse (FileUtil.IsUnderDirectory ( + Path.Combine ($"{Path.DirectorySeparatorChar}opt", "programs-extra", "java"), + parent)); + } + } +} diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkVersionInfoTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkVersionInfoTests.cs new file mode 100644 index 00000000..b37f486e --- /dev/null +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkVersionInfoTests.cs @@ -0,0 +1,71 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using NUnit.Framework; + +namespace Xamarin.Android.Tools.Tests +{ + [TestFixture] + public class JdkVersionInfoTests + { + [Test] + public void Constructor_SetsAllProperties () + { + var info = new JdkVersionInfo ( + majorVersion: 21, + displayName: "Microsoft OpenJDK 21", + downloadUrl: "https://example.com/jdk-21.zip", + checksumUrl: "https://example.com/jdk-21.zip.sha256sum.txt", + size: 123456789, + checksum: "abc123"); + + Assert.AreEqual (21, info.MajorVersion); + Assert.AreEqual ("Microsoft OpenJDK 21", info.DisplayName); + Assert.AreEqual ("https://example.com/jdk-21.zip", info.DownloadUrl); + Assert.AreEqual ("https://example.com/jdk-21.zip.sha256sum.txt", info.ChecksumUrl); + Assert.AreEqual (123456789, info.Size); + Assert.AreEqual ("abc123", info.Checksum); + } + + [Test] + public void Constructor_DefaultSizeAndChecksum () + { + var info = new JdkVersionInfo ( + majorVersion: 17, + displayName: "Microsoft OpenJDK 17", + downloadUrl: "https://example.com/jdk-17.zip", + checksumUrl: "https://example.com/jdk-17.zip.sha256sum.txt"); + + Assert.AreEqual (0, info.Size); + Assert.IsNull (info.Checksum); + } + + [Test] + public void ToString_ReturnsDisplayName () + { + var info = new JdkVersionInfo (21, "Microsoft OpenJDK 21", "https://example.com/dl", "https://example.com/cs"); + Assert.AreEqual ("Microsoft OpenJDK 21", info.ToString ()); + } + + [Test] + public void MutableProperties_CanBeSet () + { + var info = new JdkVersionInfo (21, "Test", "https://example.com/dl", "https://example.com/cs"); + + info.Size = 999; + info.Checksum = "deadbeef"; + info.ResolvedUrl = "https://resolved.example.com/jdk-21.0.5.zip"; + + Assert.AreEqual (999, info.Size); + Assert.AreEqual ("deadbeef", info.Checksum); + Assert.AreEqual ("https://resolved.example.com/jdk-21.0.5.zip", info.ResolvedUrl); + } + + [Test] + public void ResolvedUrl_DefaultsToNull () + { + var info = new JdkVersionInfo (21, "Test", "https://example.com/dl", "https://example.com/cs"); + Assert.IsNull (info.ResolvedUrl); + } + } +} diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/ProcessUtilsTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/ProcessUtilsTests.cs new file mode 100644 index 00000000..a9e7be31 --- /dev/null +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/ProcessUtilsTests.cs @@ -0,0 +1,70 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; + +using NUnit.Framework; + +namespace Xamarin.Android.Tools.Tests +{ + [TestFixture] + public class ProcessUtilsTests + { + [Test] + public void CreateProcessStartInfo_SetsFileName () + { + var psi = ProcessUtils.CreateProcessStartInfo ("myapp"); + Assert.AreEqual ("myapp", psi.FileName); + } + + [Test] + public void CreateProcessStartInfo_SetsShellAndWindow () + { + var psi = ProcessUtils.CreateProcessStartInfo ("myapp"); + Assert.IsFalse (psi.UseShellExecute, "UseShellExecute should be false"); + Assert.IsTrue (psi.CreateNoWindow, "CreateNoWindow should be true"); + } + + [Test] + public void CreateProcessStartInfo_NoArgs () + { + var psi = ProcessUtils.CreateProcessStartInfo ("myapp"); + Assert.AreEqual (0, psi.ArgumentList.Count); + } + + [Test] + public void CreateProcessStartInfo_SingleArg () + { + var psi = ProcessUtils.CreateProcessStartInfo ("myapp", "--version"); + Assert.AreEqual (1, psi.ArgumentList.Count); + Assert.AreEqual ("--version", psi.ArgumentList [0]); + } + + [Test] + public void CreateProcessStartInfo_MultipleArgs () + { + var psi = ProcessUtils.CreateProcessStartInfo ("tar", "-xzf", "archive.tar.gz", "-C", "/tmp/output"); + Assert.AreEqual (4, psi.ArgumentList.Count); + Assert.AreEqual ("-xzf", psi.ArgumentList [0]); + Assert.AreEqual ("archive.tar.gz", psi.ArgumentList [1]); + Assert.AreEqual ("-C", psi.ArgumentList [2]); + Assert.AreEqual ("/tmp/output", psi.ArgumentList [3]); + } + + [Test] + public void CreateProcessStartInfo_ArgWithSpaces () + { + var psi = ProcessUtils.CreateProcessStartInfo ("cmd", "/c", "path with spaces"); + Assert.AreEqual (2, psi.ArgumentList.Count); + Assert.AreEqual ("path with spaces", psi.ArgumentList [1]); + } + + [Test] + public void IsElevated_DoesNotThrow () + { + // Smoke test: just verify it returns without crashing + bool result = ProcessUtils.IsElevated (); + Assert.That (result, Is.TypeOf ()); + } + } +} From b2a8a7be83f457f47b1082249af860394f601a4a Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 09:12:03 -0600 Subject: [PATCH 20/24] We can't use these new lang features --- src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs index 415b2540..a5e10310 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs @@ -136,8 +136,8 @@ public static async Task ExtractTarGzAsync (string archivePath, string destinati return null; var trimmed = content.Trim (); - var end = trimmed.IndexOfAny ([' ', '\t', '\n', '\r']); - return end >= 0 ? trimmed[..end] : trimmed; + var end = trimmed.IndexOfAny (new [] { ' ', '\t', '\n', '\r' }); + return end >= 0 ? trimmed.Substring (0, end) : trimmed; } } } From 6e167e0097341a0b36cc726d6f2f2a7111ae8067 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 09:15:00 -0600 Subject: [PATCH 21/24] Use ArrayPool.Rent/Return Fix magic numbers --- .../DownloadUtils.cs | 29 ++++++++++++------- .../Xamarin.Android.Tools.AndroidSdk.csproj | 1 + 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs index a5e10310..a2a020e2 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Buffers; using System.Diagnostics; using System.IO; using System.IO.Compression; @@ -17,6 +18,9 @@ namespace Xamarin.Android.Tools /// static class DownloadUtils { + const int BufferSize = 81920; + const long BytesPerMB = 1024 * 1024; + /// Downloads a file from the given URL with optional progress reporting. public static async Task DownloadFileAsync (HttpClient client, string url, string destinationPath, long expectedSize, IProgress<(double percent, string message)>? progress, CancellationToken cancellationToken) { @@ -31,21 +35,26 @@ public static async Task DownloadFileAsync (HttpClient client, string url, strin if (!string.IsNullOrEmpty (dirPath)) Directory.CreateDirectory (dirPath); - using var fileStream = new FileStream (destinationPath, FileMode.Create, FileAccess.Write, FileShare.None, 81920, useAsync: true); + using var fileStream = new FileStream (destinationPath, FileMode.Create, FileAccess.Write, FileShare.None, BufferSize, useAsync: true); - var buffer = new byte [81920]; - long totalRead = 0; - int bytesRead; + var buffer = ArrayPool.Shared.Rent (BufferSize); + try { + long totalRead = 0; + int bytesRead; - while ((bytesRead = await contentStream.ReadAsync (buffer, 0, buffer.Length, cancellationToken).ConfigureAwait (false)) > 0) { - await fileStream.WriteAsync (buffer, 0, bytesRead, cancellationToken).ConfigureAwait (false); - totalRead += bytesRead; + while ((bytesRead = await contentStream.ReadAsync (buffer, 0, buffer.Length, cancellationToken).ConfigureAwait (false)) > 0) { + await fileStream.WriteAsync (buffer, 0, bytesRead, cancellationToken).ConfigureAwait (false); + totalRead += bytesRead; - if (progress is not null && totalBytes > 0) { - var pct = (double) totalRead / totalBytes * 100; - progress.Report ((pct, $"Downloaded {totalRead / (1024 * 1024)} MB / {totalBytes / (1024 * 1024)} MB")); + if (progress is not null && totalBytes > 0) { + var pct = (double) totalRead / totalBytes * 100; + progress.Report ((pct, $"Downloaded {totalRead / BytesPerMB} MB / {totalBytes / BytesPerMB} MB")); + } } } + finally { + ArrayPool.Shared.Return (buffer); + } } /// Verifies a file's SHA-256 hash against an expected value. diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Xamarin.Android.Tools.AndroidSdk.csproj b/src/Xamarin.Android.Tools.AndroidSdk/Xamarin.Android.Tools.AndroidSdk.csproj index 5944520a..13032f2d 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/Xamarin.Android.Tools.AndroidSdk.csproj +++ b/src/Xamarin.Android.Tools.AndroidSdk/Xamarin.Android.Tools.AndroidSdk.csproj @@ -38,6 +38,7 @@ runtime; build; native; contentfiles; analyzers + From 94b3ac4a1eda67dcfaaf536c02e08e21bc0af9bc Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 09:16:27 -0600 Subject: [PATCH 22/24] Cache WhitespaceChars --- src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs index a2a020e2..e4c5573d 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs @@ -20,6 +20,7 @@ static class DownloadUtils { const int BufferSize = 81920; const long BytesPerMB = 1024 * 1024; + static readonly char[] WhitespaceChars = [' ', '\t', '\n', '\r']; /// Downloads a file from the given URL with optional progress reporting. public static async Task DownloadFileAsync (HttpClient client, string url, string destinationPath, long expectedSize, IProgress<(double percent, string message)>? progress, CancellationToken cancellationToken) @@ -145,7 +146,7 @@ public static async Task ExtractTarGzAsync (string archivePath, string destinati return null; var trimmed = content.Trim (); - var end = trimmed.IndexOfAny (new [] { ' ', '\t', '\n', '\r' }); + var end = trimmed.IndexOfAny (WhitespaceChars); return end >= 0 ? trimmed.Substring (0, end) : trimmed; } } From 212ac4367ef15a1a97b84fb2fad170931a7ecb17 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 09:18:58 -0600 Subject: [PATCH 23/24] Pass `CancellationToken` --- .../DownloadUtils.cs | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs index e4c5573d..3ef4f53f 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs @@ -22,6 +22,24 @@ static class DownloadUtils const long BytesPerMB = 1024 * 1024; static readonly char[] WhitespaceChars = [' ', '\t', '\n', '\r']; + static Task ReadAsStreamAsync (HttpContent content, CancellationToken cancellationToken) + { +#if NET5_0_OR_GREATER + return content.ReadAsStreamAsync (cancellationToken); +#else + return content.ReadAsStreamAsync (); +#endif + } + + static Task ReadAsStringAsync (HttpContent content, CancellationToken cancellationToken) + { +#if NET5_0_OR_GREATER + return content.ReadAsStringAsync (cancellationToken); +#else + return content.ReadAsStringAsync (); +#endif + } + /// Downloads a file from the given URL with optional progress reporting. public static async Task DownloadFileAsync (HttpClient client, string url, string destinationPath, long expectedSize, IProgress<(double percent, string message)>? progress, CancellationToken cancellationToken) { @@ -30,7 +48,7 @@ public static async Task DownloadFileAsync (HttpClient client, string url, strin var totalBytes = response.Content.Headers.ContentLength ?? expectedSize; - using var contentStream = await response.Content.ReadAsStreamAsync ().ConfigureAwait (false); + using var contentStream = await ReadAsStreamAsync (response.Content, cancellationToken).ConfigureAwait (false); var dirPath = Path.GetDirectoryName (destinationPath); if (!string.IsNullOrEmpty (dirPath)) @@ -121,11 +139,7 @@ public static async Task ExtractTarGzAsync (string archivePath, string destinati try { using var response = await httpClient.GetAsync (checksumUrl, cancellationToken).ConfigureAwait (false); response.EnsureSuccessStatusCode (); -#if NET5_0_OR_GREATER - var content = await response.Content.ReadAsStringAsync (cancellationToken).ConfigureAwait (false); -#else - var content = await response.Content.ReadAsStringAsync ().ConfigureAwait (false); -#endif + var content = await ReadAsStringAsync (response.Content, cancellationToken).ConfigureAwait (false); var checksum = ParseChecksumFile (content); logger (TraceLevel.Verbose, $"{label}: checksum={checksum}"); return checksum; From 70966f79f4875209fb069062f54892fa74b542e6 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 26 Feb 2026 09:29:56 -0600 Subject: [PATCH 24/24] Better `FileUtil.IsTargetPathWritable()` --- .../DownloadUtils.cs | 3 +- .../FileUtil.cs | 30 ++++++++----------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs index 3ef4f53f..1a686c21 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs @@ -104,8 +104,7 @@ public static void ExtractZipSafe (string archivePath, string destinationPath, C var destinationFile = Path.GetFullPath (Path.Combine (fullExtractRoot, entry.FullName)); // Zip Slip protection - if (!destinationFile.StartsWith (fullExtractRoot + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase) && - destinationFile != fullExtractRoot) { + if (!FileUtil.IsUnderDirectory (destinationFile, fullExtractRoot)) { throw new InvalidOperationException ($"Archive entry '{entry.FullName}' would extract outside target directory."); } diff --git a/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs b/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs index 2a5e4990..69012b88 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs @@ -121,43 +121,39 @@ internal static void CommitMove (string targetPath, Action l } } - /// Checks if the target path is writable by testing write access on the nearest existing ancestor. + /// Checks if the target path is writable by probing write access on the nearest existing ancestor. + /// + /// Follows the same pattern as dotnet/sdk WorkloadInstallerFactory.CanWriteToDotnetRoot: + /// probe with File.Create + DeleteOnClose, only catch UnauthorizedAccessException. + /// See https://github.com/dotnet/sdk/blob/db01067a9c4b67dc1806956393ec63b032032166/src/Cli/dotnet/Commands/Workload/Install/WorkloadInstallerFactory.cs + /// internal static bool IsTargetPathWritable (string targetPath, Action logger) { if (string.IsNullOrEmpty (targetPath)) return false; - string normalizedPath; try { - normalizedPath = Path.GetFullPath (targetPath); + targetPath = Path.GetFullPath (targetPath); } catch { - normalizedPath = targetPath; - } - - if (OS.IsWindows) { - var programFiles = Environment.GetFolderPath (Environment.SpecialFolder.ProgramFiles); - var programFilesX86 = Environment.GetFolderPath (Environment.SpecialFolder.ProgramFilesX86); - if (IsUnderDirectory (normalizedPath, programFiles) || IsUnderDirectory (normalizedPath, programFilesX86)) { - logger (TraceLevel.Warning, $"Target path '{targetPath}' is in Program Files which typically requires elevation."); - return false; - } + return false; } try { - var testDir = normalizedPath; + // Walk up to the nearest existing ancestor directory + var testDir = targetPath; while (!string.IsNullOrEmpty (testDir) && !Directory.Exists (testDir)) testDir = Path.GetDirectoryName (testDir); if (string.IsNullOrEmpty (testDir)) return false; - var testFile = Path.Combine (testDir, $".write-test-{Guid.NewGuid ()}"); + var testFile = Path.Combine (testDir, Path.GetRandomFileName ()); using (File.Create (testFile, 1, FileOptions.DeleteOnClose)) { } return true; } - catch (Exception ex) { - logger (TraceLevel.Warning, $"Target path '{targetPath}' is not writable: {ex.Message}"); + catch (UnauthorizedAccessException) { + logger (TraceLevel.Warning, $"Target path '{targetPath}' is not writable."); return false; } }