From 7b6f2e71b9ac017cf02f74034022f62d611ab7ed Mon Sep 17 00:00:00 2001 From: Christian Mourier Date: Thu, 10 Sep 2026 14:53:58 +0200 Subject: [PATCH] Fix the one-click installer finding no assets in any release ExtractAssets ended the "assets" array at the first "]" in the payload. Every release built by the workflow is uploaded by "github-actions[bot]", and that bracket sits inside a string a few hundred characters into the first asset, so the array body was truncated mid-object, no asset survived the brace matcher, and the installer refused every release with "has no SqlPilot ZIP asset attached". Verified against the live v1.0.0 and v1.1.1 payloads. Scan the array with a depth counter that skips over string literals instead of regex-matching it. Confirmed end to end: the installer now downloads and installs into SSMS 18, 20 and 22. The self-update banner was unaffected because it only reads scalars. Adds tests/SqlPilot.Installer.Tests to cover the parsing directly -- a silent failure there means the installer downloads nothing, which is how this shipped. --- SqlPilot.sln | 15 +++ docs/ARCHITECTURE.md | 3 +- .../Services/GitHubReleaseClient.cs | 73 ++++++++--- .../SqlPilot.Installer.csproj | 8 ++ .../GitHubReleaseAssetTests.cs | 114 ++++++++++++++++++ .../SqlPilot.Installer.Tests.csproj | 24 ++++ 6 files changed, 222 insertions(+), 15 deletions(-) create mode 100644 tests/SqlPilot.Installer.Tests/GitHubReleaseAssetTests.cs create mode 100644 tests/SqlPilot.Installer.Tests/SqlPilot.Installer.Tests.csproj diff --git a/SqlPilot.sln b/SqlPilot.sln index 345734b..dbee949 100644 --- a/SqlPilot.sln +++ b/SqlPilot.sln @@ -25,6 +25,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SqlPilot.Package.Legacy", " EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SqlPilot.Installer", "src\SqlPilot.Installer\SqlPilot.Installer.csproj", "{5DE3DAB8-FC8F-462F-9369-A32A54D5E4F6}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SqlPilot.Installer.Tests", "tests\SqlPilot.Installer.Tests\SqlPilot.Installer.Tests.csproj", "{277D16A8-40F7-4E2D-A953-62C4734295C4}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -143,6 +145,18 @@ Global {5DE3DAB8-FC8F-462F-9369-A32A54D5E4F6}.Release|x64.Build.0 = Release|Any CPU {5DE3DAB8-FC8F-462F-9369-A32A54D5E4F6}.Release|x86.ActiveCfg = Release|Any CPU {5DE3DAB8-FC8F-462F-9369-A32A54D5E4F6}.Release|x86.Build.0 = Release|Any CPU + {277D16A8-40F7-4E2D-A953-62C4734295C4}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {277D16A8-40F7-4E2D-A953-62C4734295C4}.Debug|Any CPU.Build.0 = Debug|Any CPU + {277D16A8-40F7-4E2D-A953-62C4734295C4}.Debug|x64.ActiveCfg = Debug|Any CPU + {277D16A8-40F7-4E2D-A953-62C4734295C4}.Debug|x64.Build.0 = Debug|Any CPU + {277D16A8-40F7-4E2D-A953-62C4734295C4}.Debug|x86.ActiveCfg = Debug|Any CPU + {277D16A8-40F7-4E2D-A953-62C4734295C4}.Debug|x86.Build.0 = Debug|Any CPU + {277D16A8-40F7-4E2D-A953-62C4734295C4}.Release|Any CPU.ActiveCfg = Release|Any CPU + {277D16A8-40F7-4E2D-A953-62C4734295C4}.Release|Any CPU.Build.0 = Release|Any CPU + {277D16A8-40F7-4E2D-A953-62C4734295C4}.Release|x64.ActiveCfg = Release|Any CPU + {277D16A8-40F7-4E2D-A953-62C4734295C4}.Release|x64.Build.0 = Release|Any CPU + {277D16A8-40F7-4E2D-A953-62C4734295C4}.Release|x86.ActiveCfg = Release|Any CPU + {277D16A8-40F7-4E2D-A953-62C4734295C4}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -157,6 +171,7 @@ Global {B36CB175-6642-4E6A-A6A2-E8A7913404CF} = {E1F2A3B4-0001-0002-0003-000400050006} {DB162456-E5F3-4B9E-ABD8-E17B60C8C124} = {E1F2A3B4-0001-0002-0003-000400050006} {5DE3DAB8-FC8F-462F-9369-A32A54D5E4F6} = {E1F2A3B4-0001-0002-0003-000400050006} + {277D16A8-40F7-4E2D-A953-62C4734295C4} = {E1F2A3B4-0001-0002-0003-000400050007} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {F1E2D3C4-A5B6-C7D8-E9F0-112233445566} diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 96de992..901ed53 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -53,7 +53,8 @@ SqlPilot/ │ └── SqlPilot.Installer/ # Standalone WPF one-click installer (downloads from GitHub Releases) │ # Target: net472 + WPF ├── tests/ -│ └── SqlPilot.Core.Tests/ # xUnit tests for core logic +│ ├── SqlPilot.Core.Tests/ # xUnit tests for core logic +│ └── SqlPilot.Installer.Tests/# xUnit tests for the installer's release-JSON parsing ├── spike/ # Phase 0 spike (SSMS 22, VSSDK 17.x) ├── spike-legacy/ # Phase 0 spike (SSMS 18/20, VSSDK 15.x) ├── lib/Ssms18/ # Compile-time refs against SSMS 18 SMO/SqlWorkbench DLLs diff --git a/src/SqlPilot.Installer/Services/GitHubReleaseClient.cs b/src/SqlPilot.Installer/Services/GitHubReleaseClient.cs index 7ede69c..f1396f1 100644 --- a/src/SqlPilot.Installer/Services/GitHubReleaseClient.cs +++ b/src/SqlPilot.Installer/Services/GitHubReleaseClient.cs @@ -174,24 +174,16 @@ private static string UnescapeJsonString(string s) /// /// Walks the "assets" array in the release JSON and pulls out each entry's - /// name, size, and browser_download_url. Regex-based — fragile by design, - /// but the GitHub API shape is stable enough that this is acceptable for - /// the small set of fields we need. + /// name, size, and browser_download_url. /// - private static List ExtractAssets(string json) + internal static List ExtractAssets(string json) { var assets = new List(); - // Find each {...} object inside the "assets":[...] array - var assetsArrayMatch = Regex.Match(json, "\"assets\"\\s*:\\s*\\[(.*?)\\]", RegexOptions.Singleline); - if (!assetsArrayMatch.Success) return assets; - - var arrayBody = assetsArrayMatch.Groups[1].Value; - // Each asset object — match braces non-greedy - foreach (Match obj in Regex.Matches(arrayBody, "\\{(?:[^{}]|(?\\{)|(?<-o>\\}))*\\}", RegexOptions.Singleline)) + foreach (var obj in EnumerateArrayObjects(json, "assets")) { - var name = ExtractJsonValue(obj.Value, "name"); - var url = ExtractJsonValue(obj.Value, "browser_download_url"); - var size = ExtractJsonLong(obj.Value, "size") ?? 0; + var name = ExtractJsonValue(obj, "name"); + var url = ExtractJsonValue(obj, "browser_download_url"); + var size = ExtractJsonLong(obj, "size") ?? 0; if (!string.IsNullOrEmpty(name) && !string.IsNullOrEmpty(url)) { assets.Add(new ReleaseAsset { Name = name, BrowserDownloadUrl = url, Size = size }); @@ -199,6 +191,59 @@ private static List ExtractAssets(string json) } return assets; } + + /// + /// Yields each top-level object of the named JSON array, tracking depth and + /// skipping over string literals so that punctuation inside a string can't be + /// mistaken for structure. + /// + /// + /// This has to scan rather than match: a regex ending the array at the first + /// "]" is wrong, and not theoretically so. GitHub reports the uploader of an + /// Actions-built release as "github-actions[bot]", and that bracket truncated + /// the array mid-object, so every release looked like it had no assets at all + /// and the installer refused to install anything. + /// + private static IEnumerable EnumerateArrayObjects(string json, string key) + { + var arrayStart = Regex.Match(json, $"\"{Regex.Escape(key)}\"\\s*:\\s*\\["); + if (!arrayStart.Success) yield break; + + int depth = 0; + int objectStart = -1; + bool inString = false; + bool escaped = false; + + for (int i = arrayStart.Index + arrayStart.Length; i < json.Length; i++) + { + char c = json[i]; + + if (inString) + { + if (escaped) escaped = false; + else if (c == '\\') escaped = true; + else if (c == '"') inString = false; + continue; + } + + switch (c) + { + case '"': + inString = true; + break; + case '{': + if (depth++ == 0) objectStart = i; + break; + case '}': + if (--depth == 0) yield return json.Substring(objectStart, i - objectStart + 1); + break; + case ']': + // Only a "]" outside every object closes the array itself. + if (depth == 0) yield break; + break; + } + } + } } internal sealed class ReleaseInfo diff --git a/src/SqlPilot.Installer/SqlPilot.Installer.csproj b/src/SqlPilot.Installer/SqlPilot.Installer.csproj index bddf49a..60209b1 100644 --- a/src/SqlPilot.Installer/SqlPilot.Installer.csproj +++ b/src/SqlPilot.Installer/SqlPilot.Installer.csproj @@ -16,6 +16,14 @@ + + + + <_Parameter1>SqlPilot.Installer.Tests + + + diff --git a/tests/SqlPilot.Installer.Tests/GitHubReleaseAssetTests.cs b/tests/SqlPilot.Installer.Tests/GitHubReleaseAssetTests.cs new file mode 100644 index 0000000..235f3f4 --- /dev/null +++ b/tests/SqlPilot.Installer.Tests/GitHubReleaseAssetTests.cs @@ -0,0 +1,114 @@ +using System.Linq; +using FluentAssertions; +using SqlPilot.Installer.Services; +using Xunit; + +namespace SqlPilot.Installer.Tests +{ + /// + /// Guards the release-asset parsing the one-click installer depends on. If this + /// returns nothing the installer can't download anything, and it fails with + /// "Release vX has no SqlPilot ZIP asset attached" — which is what shipped. + /// + public class GitHubReleaseAssetTests + { + /// + /// Shaped like a real GitHub releases payload. The load-bearing detail is the + /// uploader login "github-actions[bot]": every release built by the workflow has + /// it, and the "]" inside that string used to end the assets array early. + /// + private const string ReleaseJson = @"{ + ""url"": ""https://api.github.com/repos/mourier/sql-pilot/releases/1"", + ""assets_url"": ""https://api.github.com/repos/mourier/sql-pilot/releases/1/assets"", + ""html_url"": ""https://github.com/mourier/sql-pilot/releases/tag/v1.0.0"", + ""tag_name"": ""v1.0.0"", + ""assets"": [ + { + ""url"": ""https://api.github.com/repos/mourier/sql-pilot/releases/assets/1"", + ""id"": 1, + ""name"": ""SqlPilot-v1.0.0.zip"", + ""label"": """", + ""uploader"": { ""login"": ""github-actions[bot]"", ""id"": 41898282, ""type"": ""Bot"" }, + ""content_type"": ""application/zip"", + ""state"": ""uploaded"", + ""size"": 492544, + ""browser_download_url"": ""https://github.com/mourier/sql-pilot/releases/download/v1.0.0/SqlPilot-v1.0.0.zip"" + }, + { + ""url"": ""https://api.github.com/repos/mourier/sql-pilot/releases/assets/2"", + ""id"": 2, + ""name"": ""SqlPilotInstaller-v1.0.0.zip"", + ""label"": """", + ""uploader"": { ""login"": ""github-actions[bot]"", ""id"": 41898282, ""type"": ""Bot"" }, + ""content_type"": ""application/zip"", + ""state"": ""uploaded"", + ""size"": 376832, + ""browser_download_url"": ""https://github.com/mourier/sql-pilot/releases/download/v1.0.0/SqlPilotInstaller-v1.0.0.zip"" + } + ], + ""body"": ""Release notes [with a bracket] and a } brace."" + }"; + + [Fact] + public void ExtractAssets_ReturnsEveryAsset_WhenTheUploaderLoginContainsABracket() + { + var assets = GitHubReleaseClient.ExtractAssets(ReleaseJson); + + assets.Select(a => a.Name).Should().Equal( + "SqlPilot-v1.0.0.zip", + "SqlPilotInstaller-v1.0.0.zip"); + } + + [Fact] + public void ExtractAssets_KeepsTheDownloadUrlAndSizeOfEachAsset() + { + var payload = GitHubReleaseClient.ExtractAssets(ReleaseJson) + .Single(a => a.Name == "SqlPilot-v1.0.0.zip"); + + payload.BrowserDownloadUrl.Should().Be( + "https://github.com/mourier/sql-pilot/releases/download/v1.0.0/SqlPilot-v1.0.0.zip"); + payload.Size.Should().Be(492544); + } + + [Fact] + public void ExtractAssets_FindsThePayloadZip_NotTheInstallerZipOrTheVsix() + { + // Mirrors InstallEngine's selection: the "SqlPilot-" prefix excludes the + // installer's own ZIP, and the ".zip" suffix excludes the gallery .vsix, + // which sorts first in a real v1.1.1 payload. + const string withVsix = @"{ ""tag_name"": ""v1.1.1"", ""assets"": [ + { ""name"": ""SqlPilot-v1.1.1.vsix"", ""size"": 1, ""uploader"": { ""login"": ""github-actions[bot]"" }, + ""browser_download_url"": ""https://example.invalid/SqlPilot-v1.1.1.vsix"" }, + { ""name"": ""SqlPilot-v1.1.1.zip"", ""size"": 2, ""uploader"": { ""login"": ""github-actions[bot]"" }, + ""browser_download_url"": ""https://example.invalid/SqlPilot-v1.1.1.zip"" }, + { ""name"": ""SqlPilotInstaller-v1.1.1.zip"", ""size"": 3, ""uploader"": { ""login"": ""github-actions[bot]"" }, + ""browser_download_url"": ""https://example.invalid/SqlPilotInstaller-v1.1.1.zip"" } ] }"; + + var picked = GitHubReleaseClient.ExtractAssets(withVsix) + .FirstOrDefault(a => a.Name.StartsWith("SqlPilot-") && a.Name.EndsWith(".zip")); + + picked.Should().NotBeNull(); + picked.Name.Should().Be("SqlPilot-v1.1.1.zip"); + } + + [Fact] + public void ExtractAssets_ReturnsEmpty_WhenTheReleaseHasNoAssets() + { + GitHubReleaseClient.ExtractAssets(@"{ ""tag_name"": ""v9.9.9"", ""assets"": [] }") + .Should().BeEmpty(); + } + + [Fact] + public void ExtractAssets_StopsAtTheEndOfTheAssetsArray() + { + // A later array of objects in the payload must not be read as more assets. + const string trailing = @"{ ""assets"": [ + { ""name"": ""SqlPilot-v1.0.0.zip"", ""size"": 1, ""uploader"": { ""login"": ""github-actions[bot]"" }, + ""browser_download_url"": ""https://example.invalid/a.zip"" } ], + ""reactions"": [ { ""name"": ""not-an-asset.zip"", ""browser_download_url"": ""https://example.invalid/b.zip"" } ] }"; + + GitHubReleaseClient.ExtractAssets(trailing) + .Select(a => a.Name).Should().Equal("SqlPilot-v1.0.0.zip"); + } + } +} diff --git a/tests/SqlPilot.Installer.Tests/SqlPilot.Installer.Tests.csproj b/tests/SqlPilot.Installer.Tests/SqlPilot.Installer.Tests.csproj new file mode 100644 index 0000000..2c9e96f --- /dev/null +++ b/tests/SqlPilot.Installer.Tests/SqlPilot.Installer.Tests.csproj @@ -0,0 +1,24 @@ + + + net472 + SqlPilot.Installer.Tests + SqlPilot.Installer.Tests + false + true + + + + + + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + + +