Fix encoding corruption from #420 and cross-project BlackListManager references#422
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to undo UTF-8/mojibake corruption introduced in #420 and to fix remaining cross-project references to the removed BlackListManager singleton by switching callers to BlackListDefaults / DefaultBlackListMatcher, plus updating affected tests.
Changes:
- Replace
BlackListManager.Instanceusages in Differential dirty/apply flows withBlackListDefaults. - Update
ParameterMatrixAndEventTeststo constructDefaultBlackListMatcherinstead of referencing the removed singleton. - Attempt to restore clean encoding in docs/comments and metadata (but several mojibake sequences remain).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/CoreTest/Bootstrap/ParameterMatrixAndEventTests.cs | Updates tests away from removed BlackListManager; assertions updated. |
| src/c#/GeneralUpdate.Differential/Pipeline/DiffPipeline.cs | Uses BlackListDefaults.DefaultSkipDirectories and extension filtering in copy phase. |
| src/c#/GeneralUpdate.Differential/Matchers/DefaultDirtyStrategy.cs | Uses BlackListDefaults for skip dirs and extension filtering. |
| src/c#/GeneralUpdate.Core/Strategy/OSSUpdateStrategy.cs | Adjusts download path and contains encoding-corrupted docs/comments. |
| src/c#/GeneralUpdate.Core/Strategy/ClientUpdateStrategy.cs | Comment text touched; still contains mojibake sequences. |
| src/c#/GeneralUpdate.Core/Silent/SilentPollOrchestrator.cs | Comment text touched; still contains mojibake sequences. |
| src/c#/GeneralUpdate.Core/Ipc/IProcessInfoProvider.cs | Comment/doc adjustment. |
| src/c#/GeneralUpdate.Core/GeneralUpdate.Core.csproj | Fixes description dash and removes a comment line. |
| src/c#/GeneralUpdate.Core/Configuration/Environments.cs | Introduces fixed AES key/IV derivation for env IPC encryption. |
Comments suppressed due to low confidence (1)
src/c#/GeneralUpdate.Core/Strategy/ClientUpdateStrategy.cs:96
- This comment contains the corrupted
�?sequence (mojibake). Please replace with the intended punctuation/text and ensure the file is committed with correct UTF-8 encoding; there are multiple occurrences in this file.
private async Task ExecuteWorkflowAsync()
{
// Standard mode �silent mode is handled by GeneralUpdateBootstrap.LaunchSilentAsync().
// Runtime options (Encoding, Format, DownloadTimeOut, etc.) are already
// populated on _configInfo by Bootstrap.ApplyRuntimeOptions().
await ExecuteStandardWorkflowAsync();
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
264
to
274
| [Fact] | ||
| public void BlackListManager_VariousConfigurations_AcceptsAllRules() | ||
| { | ||
| var manager = BlackListManager.Instance; | ||
| var manager = new DefaultBlackListMatcher(BlackListConfig.Empty); | ||
|
|
||
| Assert.NotNull(manager); | ||
| Assert.NotNull(manager.BlackFiles); | ||
| Assert.NotNull(manager.BlackFormats); | ||
| Assert.NotNull(manager.SkipDirectorys); | ||
| Assert.NotNull(BlackListDefaults.DefaultBlackFiles); | ||
| Assert.NotNull(BlackListDefaults.DefaultBlackFormats); | ||
| Assert.NotNull(BlackListDefaults.DefaultSkipDirectories); | ||
| Assert.False(manager.IsBlacklisted("test.dll")); | ||
| } |
| { | ||
| var extensionName = Path.GetExtension(file.FullName); | ||
| if (BlackListManager.Instance.IsBlacklisted(extensionName)) continue; | ||
| if (BlackListDefaults.DefaultBlackFormats.Contains(extensionName)) continue; |
Comment on lines
128
to
132
| foreach (var file in comparisonResult.DifferentNodes) | ||
| { | ||
| var extensionName = Path.GetExtension(file.FullName); | ||
| if (BlackListManager.Instance.IsBlacklisted(extensionName)) continue; | ||
| if (BlackListDefaults.DefaultBlackFormats.Contains(extensionName)) continue; | ||
|
|
Comment on lines
18
to
25
| /// <summary> | ||
| /// OSS (Object Storage Service) update strategy �?client/upgrade split via AppType. | ||
| /// OSS (Object Storage Service) update strategy �client/upgrade split via AppType. | ||
| /// <list type="bullet"> | ||
| /// <item><see cref="AppType.OSSClient"/> �?downloads version config, checks for updates, | ||
| /// <item><see cref="AppType.OSSClient"/> �downloads version config, checks for updates, | ||
| /// starts the upgrade process, and exits.</item> | ||
| /// <item><see cref="AppType.OSSUpgrade"/> �?reads version config, downloads packages from OSS, | ||
| /// <item><see cref="AppType.OSSUpgrade"/> �reads version config, downloads packages from OSS, | ||
| /// decompresses them, starts the main app, and exits.</item> | ||
| /// </list> |
Comment on lines
234
to
249
| private async Task DownloadAssetsAsync(List<DownloadAsset> assets) | ||
| { | ||
| var plan = new DownloadPlan(assets, false); | ||
| if (DownloadOrchestrator != null) | ||
| { | ||
| await DownloadOrchestrator.ExecuteAsync(plan, _appPath).ConfigureAwait(false); | ||
| } | ||
| else | ||
| { | ||
| using var httpClient = GeneralUpdate.Core.Network.HttpClientProvider.Shared; | ||
| using var cts = new CancellationTokenSource( | ||
| TimeSpan.FromSeconds(_configInfo?.DownloadTimeOut > 0 ? _configInfo!.DownloadTimeOut : DefaultTimeOut)); | ||
| using var httpClient = new HttpClient | ||
| { | ||
| Timeout = TimeSpan.FromSeconds(_configInfo?.DownloadTimeOut > 0 ? _configInfo!.DownloadTimeOut : DefaultTimeOut) | ||
| }; | ||
| var orchestrator = new DefaultDownloadOrchestrator(httpClient); | ||
| await orchestrator.ExecuteAsync(plan, _appPath, token: cts.Token).ConfigureAwait(false); | ||
| await orchestrator.ExecuteAsync(plan, _appPath).ConfigureAwait(false); | ||
| } |
|
|
||
| /// <summary> | ||
| /// Silent update poll orchestrator �?periodically checks for updates, | ||
| /// Silent update poll orchestrator �?periodically checks for updates, |
Comment on lines
+15
to
19
| // Fixed key/IV derived from a constant — not crypto-grade, but sufficient for | ||
| // ephemeral IPC where the file lives < 1 second and is in a per-user directory. | ||
| private static readonly byte[] _aesKey = SHA256.Create() | ||
| .ComputeHash(Encoding.UTF8.GetBytes("GeneralUpdate.IPC.EnvironmentProvider.v1")); | ||
| private static readonly byte[] _aesIV = new byte[16] { 0x47, 0x55, 0x50, 0x44, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }; |
…ListManager references - Restore all files from before #420 merge and re-apply changes with proper encoding - Replace BlackListManager.Instance references in Differential project and tests - Fix ParameterMatrixAndEventTests to work with DefaultBlackListMatcher API - Use BlackListDefaults instead of removed BlackListManager singleton All changes from #419 + CI build fixes. Build and tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #421
Problem
PR #420 had:
Changes
CI
All 3 jobs pass: aot-verify ✓ | build-and-test (ubuntu) ✓ | build-and-test (windows) ✓