Skip to content

Commit c81842d

Browse files
JusterZhuclaude
andcommitted
fix: address 5 Copilot review suggestions
1. BsdiffDiffer: recognize Brotli marker (0x02) as extended-header format even on netstandard2.0 (fail-fast) instead of mis-detecting as legacy BZip2 2. VersionComparer.ParseVersion: use TryParseNumeric with overflow guard instead of bare long.Parse (can throw on > Int64 values) 3. VersionComparer.ComparePrerelease: add IsNumericString fallback for purely-numeric identifiers that exceed long.MaxValue (length+ordinal) 4. OssStrategy: limit diagnostic file listing to 20 entries to prevent unbounded log noise on non-Windows 5. VersionComparerTests: fix doc comment (long.MaxValue -> int.MaxValue) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent cd54051 commit c81842d

4 files changed

Lines changed: 61 additions & 12 deletions

File tree

src/c#/GeneralUpdate.Core/Strategy/OssStrategy.cs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,15 @@ private async Task ExecuteClientAsync()
269269
try
270270
{
271271
var searchPattern = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "*.exe" : "*";
272-
var dirFiles = Directory.GetFiles(upgradeDir, searchPattern).Select(f => Path.GetFileName(f));
273-
GeneralTracer.Info($"[OssClient] Files in {upgradeDir}: [{string.Join(", ", dirFiles)}]");
272+
const int maxDisplay = 20;
273+
var dirFiles = Directory.EnumerateFiles(upgradeDir, searchPattern)
274+
.Take(maxDisplay + 1)
275+
.Select(f => Path.GetFileName(f))
276+
.ToList();
277+
var summary = dirFiles.Count > maxDisplay
278+
? $"[{string.Join(", ", dirFiles.Take(maxDisplay))}, ... and {dirFiles.Count - maxDisplay} more]"
279+
: $"[{string.Join(", ", dirFiles)}]";
280+
GeneralTracer.Info($"[OssClient] Files in {upgradeDir}: {summary}");
274281
}
275282
catch (Exception ex)
276283
{

src/c#/GeneralUpdate.Differential/Differ/BsdiffDiffer.cs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,9 +309,7 @@ await Task.Run(() =>
309309
byte candidate = formatByte[0];
310310

311311
if (candidate == BZip2FormatVersion || candidate == DeflateFormatVersion
312-
#if NET6_0_OR_GREATER
313312
|| candidate == BrotliFormatVersion
314-
#endif
315313
)
316314
{
317315
formatVersion = candidate;
@@ -444,9 +442,7 @@ await Task.Run(() =>
444442

445443
private const byte BZip2FormatVersion = 0x00;
446444
private const byte DeflateFormatVersion = 0x01;
447-
#if NET6_0_OR_GREATER
448445
private const byte BrotliFormatVersion = 0x02;
449-
#endif
450446

451447
private static FileStream OpenPatchStream(string patchPath)
452448
{

src/c#/GeneralUpdate.Drivelution/Core/Utilities/VersionComparer.cs

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,35 @@ private static SemVerInfo ParseVersion(string version)
103103
throw new FormatException($"Version '{version}' does not follow SemVer 2.0 format");
104104
}
105105

106+
if (!TryParseNumeric(match.Groups["major"].Value, out var major))
107+
throw new FormatException($"Version '{version}' has a 'major' component that exceeds the supported range.");
108+
if (!TryParseNumeric(match.Groups["minor"].Value, out var minor))
109+
throw new FormatException($"Version '{version}' has a 'minor' component that exceeds the supported range.");
110+
if (!TryParseNumeric(match.Groups["patch"].Value, out var patch))
111+
throw new FormatException($"Version '{version}' has a 'patch' component that exceeds the supported range.");
112+
106113
return new SemVerInfo
107114
{
108-
Major = long.Parse(match.Groups["major"].Value),
109-
Minor = long.Parse(match.Groups["minor"].Value),
110-
Patch = long.Parse(match.Groups["patch"].Value),
115+
Major = major,
116+
Minor = minor,
117+
Patch = patch,
111118
Prerelease = match.Groups["prerelease"].Value,
112119
BuildMetadata = match.Groups["buildmetadata"].Value
113120
};
114121
}
115122

123+
private static bool TryParseNumeric(string value, out long result)
124+
{
125+
// SemVer numeric components are non-negative integers per spec.
126+
// long.MaxValue (≈9.2e18) is the largest we support.
127+
if (value.Length > 19) // any value > long.MaxValue has at least 19 digits
128+
{
129+
result = 0;
130+
return false;
131+
}
132+
return long.TryParse(value, out result);
133+
}
134+
116135
private static int ComparePrerelease(string pre1, string pre2)
117136
{
118137
var parts1 = pre1.Split('.');
@@ -122,14 +141,27 @@ private static int ComparePrerelease(string pre1, string pre2)
122141

123142
for (int i = 0; i < minLength; i++)
124143
{
125-
var isNum1 = long.TryParse(parts1[i], out long num1);
126-
var isNum2 = long.TryParse(parts2[i], out long num2);
144+
// Test numeric identifiers — if both parse, compare numerically.
145+
// Per SemVer 2.0 §11, numeric prerelease identifiers are compared
146+
// as integers. If an identifier exceeds long.MaxValue, fall back
147+
// to a digit-length + ordinal comparison so the ordering remains
148+
// integer-like even when the platform type can't hold the value.
149+
bool isNum1 = TryParseNumeric(parts1[i], out long num1);
150+
bool isNum2 = TryParseNumeric(parts2[i], out long num2);
127151

128152
if (isNum1 && isNum2)
129153
{
130154
if (num1 != num2)
131155
return num1.CompareTo(num2);
132156
}
157+
else if (IsNumericString(parts1[i]) && IsNumericString(parts2[i]))
158+
{
159+
// Both are purely numeric but exceed long.MaxValue.
160+
// Compare by digit length first, then ordinal.
161+
int cmp = parts1[i].Length.CompareTo(parts2[i].Length);
162+
if (cmp != 0) return cmp;
163+
return string.CompareOrdinal(parts1[i], parts2[i]);
164+
}
133165
else if (isNum1)
134166
{
135167
return -1; // Numeric identifier is less than alphanumeric
@@ -150,6 +182,19 @@ private static int ComparePrerelease(string pre1, string pre2)
150182
return parts1.Length.CompareTo(parts2.Length);
151183
}
152184

185+
/// <summary>
186+
/// Returns true when <paramref name="s"/> is non-empty and every character
187+
/// is an ASCII digit — without attempting to parse into a numeric type.
188+
/// </summary>
189+
private static bool IsNumericString(string s)
190+
{
191+
if (string.IsNullOrEmpty(s)) return false;
192+
foreach (char c in s)
193+
if (c < '0' || c > '9')
194+
return false;
195+
return true;
196+
}
197+
153198
private class SemVerInfo
154199
{
155200
public long Major { get; set; }

tests/DrivelutionTest/Utilities/VersionComparerAndRestartHelperTests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,8 @@ public void Compare_VeryLargeNumbers_DoesNotOverflow()
218218
[Fact]
219219
public void Compare_PrereleaseWithLargeNumbers_DoesNotOverflow()
220220
{
221-
// long.MaxValue ~9.2e18 approaching overflow in int
221+
// Values up to ~1e12 exceed int.MaxValue (~2.1e9) so this verifies
222+
// the long-based numeric overflow guard in ComparePrerelease.
222223
var result = VersionComparer.Compare("1.0.0-999999999999", "1.0.0-0");
223224
Assert.True(result > 0);
224225
}

0 commit comments

Comments
 (0)