From 8e5549983facd748020023ac0cac696315411840 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 22 Jun 2026 14:07:23 -0400 Subject: [PATCH 1/8] Fix IndexOutOfRangeException in ChorusHubServerInfo.GetValue for bare query flags String.Split always returns at least one element, so the guard `parts.Length < 1` was unreachable. Any query segment without '=' would throw on `parts[1]`. Change the guard to `parts.Length < 2` so those segments are skipped safely. Co-Authored-By: Claude Sonnet 4.6 --- src/LibChorus/ChorusHub/ChorusHubServerInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs b/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs index 8bd6f174c..a06df52ed 100644 --- a/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs +++ b/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs @@ -57,7 +57,7 @@ private static string GetValue(string parameters, string name) foreach (var segment in querySegments) { var parts = segment.Split('='); - if (parts.Length < 1) + if (parts.Length < 2) continue; var key = parts[0].Trim(new[] { '?', ' ' }); From 3df2edb2c226de19a561c627750d4a2641f70894 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 22 Jun 2026 14:36:58 -0400 Subject: [PATCH 2/8] Preserve '=' characters in query values by limiting split to 2 parts Splitting on all '=' would silently drop everything after the second '=' in a value like key=val=ue. Use Split(..., 2) so the key is parts[0] and the full remainder (including any embedded '=') is parts[1]. Co-Authored-By: Claude Sonnet 4.6 --- src/LibChorus/ChorusHub/ChorusHubServerInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs b/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs index a06df52ed..37d9e493a 100644 --- a/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs +++ b/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs @@ -56,7 +56,7 @@ private static string GetValue(string parameters, string name) var querySegments = parameters.Split('&'); foreach (var segment in querySegments) { - var parts = segment.Split('='); + var parts = segment.Split(new[] { '=' }, 2); if (parts.Length < 2) continue; From ecbda5aabc8546088a4f92f41921b7dd04e81b45 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 22 Jun 2026 14:45:48 -0400 Subject: [PATCH 3/8] Replace manual query-string parsing with HttpUtility.ParseQueryString The hand-rolled split loop had two bugs (unreachable guard for missing '=', truncated values containing '='). Replace it with HttpUtility.ParseQueryString, consistent with how UrlHelper parses query strings elsewhere in the library. Co-Authored-By: Claude Sonnet 4.6 --- src/LibChorus/ChorusHub/ChorusHubServerInfo.cs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs b/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs index 37d9e493a..5f0d35408 100644 --- a/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs +++ b/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs @@ -6,6 +6,7 @@ using System.Net.Sockets; using System.Text; using System.Threading; +using System.Web; namespace Chorus.ChorusHub { @@ -52,21 +53,8 @@ public static ChorusHubServerInfo Parse(string parameters) private static string GetValue(string parameters, string name) { - var queryParameters = new NameValueCollection(); - var querySegments = parameters.Split('&'); - foreach (var segment in querySegments) - { - var parts = segment.Split(new[] { '=' }, 2); - if (parts.Length < 2) - continue; - - var key = parts[0].Trim(new[] { '?', ' ' }); - var val = parts[1].Trim(); - - queryParameters.Add(key, val); - } - - var r = queryParameters.GetValues(name); + var parsed = HttpUtility.ParseQueryString(parameters.TrimStart('?')); + var r = parsed.GetValues(name); return r == null ? "?" : r.First(); } From 5254c0b0275486e2293c72fef553556def0806cb Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 22 Jun 2026 14:55:02 -0400 Subject: [PATCH 4/8] Fix round-trip asymmetry and remove unused import HttpUtility.ParseQueryString URL-decodes values, so ToString() must URL-encode them to avoid silent corruption on a round-trip. Use Uri.EscapeDataString on all string fields. Also remove the now-unused System.Collections.Specialized import left over from the old manual parser. Co-Authored-By: Claude Sonnet 4.6 --- src/LibChorus/ChorusHub/ChorusHubServerInfo.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs b/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs index 5f0d35408..d917cf866 100644 --- a/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs +++ b/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Specialized; using System.Diagnostics; using System.Linq; using System.Net; @@ -69,7 +68,7 @@ public static bool IsChorusHubInfo(string parameters) /// public override string ToString() { - return $"ChorusHubInfo?version={VersionOfThisCode}&address={_ipAddress}&port={_port}&hostname={HostName}"; + return $"ChorusHubInfo?version={VersionOfThisCode}&address={Uri.EscapeDataString(_ipAddress)}&port={Uri.EscapeDataString(_port)}&hostname={Uri.EscapeDataString(HostName)}"; } public string ServiceUri => $"net.tcp://{_ipAddress}:{ChorusHubOptions.ServicePort}"; From d8b4bf86b5f6a8ab6dcb2652a4e639560605892d Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 22 Jun 2026 14:57:11 -0400 Subject: [PATCH 5/8] Revert to manual parser; keep only the two targeted bug fixes HttpUtility.ParseQueryString introduced a round-trip asymmetry (URL-decoding on parse but no encoding on serialization), and fixing that with Uri.EscapeDataString would encode IPv6 colons, breaking old clients. Since all values are machine-generated and will never contain '+' or '%', the manual parser is correct as-is with the two fixes: guard parts.Length < 2 to skip bare flags, and Split(..., 2) to preserve '=' in values. Co-Authored-By: Claude Sonnet 4.6 --- .../ChorusHub/ChorusHubServerInfo.cs | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs b/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs index d917cf866..29cb35e80 100644 --- a/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs +++ b/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs @@ -1,11 +1,11 @@ using System; +using System.Collections.Specialized; using System.Diagnostics; using System.Linq; using System.Net; using System.Net.Sockets; using System.Text; using System.Threading; -using System.Web; namespace Chorus.ChorusHub { @@ -52,8 +52,20 @@ public static ChorusHubServerInfo Parse(string parameters) private static string GetValue(string parameters, string name) { - var parsed = HttpUtility.ParseQueryString(parameters.TrimStart('?')); - var r = parsed.GetValues(name); + var queryParameters = new NameValueCollection(); + foreach (var segment in parameters.Split('&')) + { + var parts = segment.Split(new[] { '=' }, 2); + if (parts.Length < 2) + continue; + + var key = parts[0].Trim(new[] { '?', ' ' }); + var val = parts[1].Trim(); + + queryParameters.Add(key, val); + } + + var r = queryParameters.GetValues(name); return r == null ? "?" : r.First(); } @@ -68,7 +80,7 @@ public static bool IsChorusHubInfo(string parameters) /// public override string ToString() { - return $"ChorusHubInfo?version={VersionOfThisCode}&address={Uri.EscapeDataString(_ipAddress)}&port={Uri.EscapeDataString(_port)}&hostname={Uri.EscapeDataString(HostName)}"; + return $"ChorusHubInfo?version={VersionOfThisCode}&address={_ipAddress}&port={_port}&hostname={HostName}"; } public string ServiceUri => $"net.tcp://{_ipAddress}:{ChorusHubOptions.ServicePort}"; From aa91b0132eaa18693e8f18cc11a79b6ec8d3ac81 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Tue, 23 Jun 2026 06:31:45 -0400 Subject: [PATCH 6/8] Add CHANGELOG entry and unit tests for ChorusHubServerInfo query parse fix Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 1 + src/ChorusHubTests/ChorusHubTests.cs | 30 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a419750c..11bd3eaae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [SIL.Chorus] Fix collection-modified exception when UsbDrives property is read while background scan thread updates the list - [SIL.Chorus.LibChorus] Fix null-key insertion when XML record identifier attribute is absent in XmlMergeService - [SIL.Chorus.LibChorus] Fix crash in password encryption/decryption on non-Windows platforms and on Windows data-protection failure +- [SIL.Chorus.LibChorus] Fix IndexOutOfRangeException parsing ChorusHub query parameters without '=', and preserve values containing '=' ## [5.1.0] - 2023-03-07 diff --git a/src/ChorusHubTests/ChorusHubTests.cs b/src/ChorusHubTests/ChorusHubTests.cs index 8e4bee822..440f477d7 100644 --- a/src/ChorusHubTests/ChorusHubTests.cs +++ b/src/ChorusHubTests/ChorusHubTests.cs @@ -240,4 +240,34 @@ public void GetRepostoryNames_TwoItemsInHubFolder_GetNoneForProjectFilter() } } } + + [TestFixture] + public class ChorusHubServerInfoParseTests + { + [Test] + public void Parse_SegmentWithoutEquals_DoesNotThrow() + { + // A bare flag segment (no '=') previously caused IndexOutOfRangeException on parts[1]. + const string input = "ChorusHubInfo?version=3&address=192.168.1.1&port=5912&hostname=mypc&flag"; + Assert.DoesNotThrow(() => ChorusHubServerInfo.Parse(input)); + } + + [Test] + public void Parse_SegmentWithoutEquals_OtherValuesStillParsed() + { + const string input = "ChorusHubInfo?version=3&address=192.168.1.1&port=5912&hostname=mypc&flag"; + var info = ChorusHubServerInfo.Parse(input); + Assert.That(info.HostName, Is.EqualTo("mypc")); + } + + [Test] + public void Parse_ValueContainsEquals_FullValuePreserved() + { + // Previously Split('=') on "hostname=my=pc" produced ["hostname","my","pc"]; + // only parts[1] was used, silently dropping "=pc". + const string input = "ChorusHubInfo?version=3&address=192.168.1.1&port=5912&hostname=my=pc"; + var info = ChorusHubServerInfo.Parse(input); + Assert.That(info.HostName, Is.EqualTo("my=pc")); + } + } } From 8ec06d57f587ca1a7dd1d0e2bd06e28e7a3a649a Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Tue, 23 Jun 2026 13:40:10 -0400 Subject: [PATCH 7/8] Remove comments from ChorusHubServerInfoParseTests Co-Authored-By: Claude Sonnet 4.6 --- src/ChorusHubTests/ChorusHubTests.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ChorusHubTests/ChorusHubTests.cs b/src/ChorusHubTests/ChorusHubTests.cs index 440f477d7..7e9eacad9 100644 --- a/src/ChorusHubTests/ChorusHubTests.cs +++ b/src/ChorusHubTests/ChorusHubTests.cs @@ -247,7 +247,6 @@ public class ChorusHubServerInfoParseTests [Test] public void Parse_SegmentWithoutEquals_DoesNotThrow() { - // A bare flag segment (no '=') previously caused IndexOutOfRangeException on parts[1]. const string input = "ChorusHubInfo?version=3&address=192.168.1.1&port=5912&hostname=mypc&flag"; Assert.DoesNotThrow(() => ChorusHubServerInfo.Parse(input)); } @@ -263,8 +262,6 @@ public void Parse_SegmentWithoutEquals_OtherValuesStillParsed() [Test] public void Parse_ValueContainsEquals_FullValuePreserved() { - // Previously Split('=') on "hostname=my=pc" produced ["hostname","my","pc"]; - // only parts[1] was used, silently dropping "=pc". const string input = "ChorusHubInfo?version=3&address=192.168.1.1&port=5912&hostname=my=pc"; var info = ChorusHubServerInfo.Parse(input); Assert.That(info.HostName, Is.EqualTo("my=pc")); From 91ee5adad99f0d736b80db1d232f58ded289c9d9 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 2 Jul 2026 08:52:18 -0400 Subject: [PATCH 8/8] Parse valueless query parameters as empty string instead of discarding them Addresses review feedback on #379: a segment without '=' (e.g. 'flag') is now stored with an empty-string value, distinguishable from an absent parameter (which GetValue reports as "?"). Co-Authored-By: Claude Fable 5 --- src/ChorusHubTests/ChorusHubTests.cs | 8 ++++++++ src/LibChorus/ChorusHub/ChorusHubServerInfo.cs | 5 +---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/ChorusHubTests/ChorusHubTests.cs b/src/ChorusHubTests/ChorusHubTests.cs index 7e9eacad9..7f3812652 100644 --- a/src/ChorusHubTests/ChorusHubTests.cs +++ b/src/ChorusHubTests/ChorusHubTests.cs @@ -266,5 +266,13 @@ public void Parse_ValueContainsEquals_FullValuePreserved() var info = ChorusHubServerInfo.Parse(input); Assert.That(info.HostName, Is.EqualTo("my=pc")); } + + [Test] + public void Parse_SegmentWithoutEquals_ParsedWithEmptyValue() + { + const string input = "ChorusHubInfo?version=3&address=192.168.1.1&port=5912&hostname"; + var info = ChorusHubServerInfo.Parse(input); + Assert.That(info.HostName, Is.EqualTo("")); + } } } diff --git a/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs b/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs index 29cb35e80..5d8b7b748 100644 --- a/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs +++ b/src/LibChorus/ChorusHub/ChorusHubServerInfo.cs @@ -56,11 +56,8 @@ private static string GetValue(string parameters, string name) foreach (var segment in parameters.Split('&')) { var parts = segment.Split(new[] { '=' }, 2); - if (parts.Length < 2) - continue; - var key = parts[0].Trim(new[] { '?', ' ' }); - var val = parts[1].Trim(); + var val = parts.Length < 2 ? "" : parts[1].Trim(); queryParameters.Add(key, val); }