diff --git a/.pr_agent_auto_best_practices.md b/.pr_agent_auto_best_practices.md index 41eb947..8cb9bb9 100644 --- a/.pr_agent_auto_best_practices.md +++ b/.pr_agent_auto_best_practices.md @@ -1,22 +1,23 @@ -Pattern 1: Add explicit null/empty/invalid-input guards before dereferencing objects, indexing, or performing operations that can throw (e.g., Substring, math on empty arrays, dereferencing refs, schema access). Prefer returning a safe sentinel (null/false) or throwing a clear Argument/InvalidOperation exception. +Pattern 1: Add explicit input/type/null validation before dereferencing, parsing, or doing arithmetic/substring operations, and return a safe value or throw a clear exception when arguments or intermediate results are invalid. Example code before: ``` -// C# -var pk = schema.PrimaryKey; // schema might be null -var tail = path.Substring(prefix.Length); // path may be too short -return obj.Ref.Value.ToString(); // Ref may be null +function buildUrl(base, path) { + return base + "/" + path.substring(3); // can throw if path is short +} + +if (obj.name.length > 0) { ... } // obj or name may be null ``` Example code after: ``` -// C# -if (schema == null) throw new ArgumentNullException(nameof(schema)); -if (!path.StartsWith(prefix, StringComparison.OrdinalIgnoreCase) || path.Length <= prefix.Length) - return null; -if (obj?.Ref == null) return null; -var pk = schema.PrimaryKey; +function buildUrl(base, path) { + if (typeof path !== "string" || path.length < 3) return null; + return base.replace(/\/$/, "") + "/" + path.substring(3); +} + +if (obj && typeof obj.name === "string" && obj.name.length > 0) { ... } ```
Relevant past accepted suggestions: @@ -26,6 +27,51 @@ ___ Suggestion 1: +Fix incorrect type check + + + + + +**In repositionObject, fix the incorrect type check for the position parameter by using typeof position !== "number" instead of position !== "number".** + +[lib/pipe-ipc.js [162-163]](https://github.com/gnh1201/welsonjs/pull/386/files#diff-be768ebd00b0134435282f146e85e6d89798fe679849026f071aad594df5ab92R162-R163) + +```diff + this.repositionObject = function(stream, position) { +- position = (position !== "number" ? 0 : position); ++ position = (typeof position !== "number" ? 0 : position); +``` + + + +___ + + +Suggestion 2: + +Null-check logger before use + +**Guard uses of Logger with a null check to avoid potential NullReferenceException when logging optional events. Add a simple Logger != null check before calling Info.** + +[WelsonJS.Toolkit/WelsonJS.Launcher/AssemblyLoader.cs [343-347]](https://github.com/gnh1201/welsonjs/pull/360/files#diff-70109da2862343aaf098ff9f7bce9d06f5f51331146ec54aa573ccbaa776fdaaR343-R347) + +```diff + if (isDll && TryDownloadCompressedFile(gzUrl, dest)) + { +- Logger.Info("Downloaded and decompressed file to: {0}", dest); ++ if (Logger != null) Logger.Info("Downloaded and decompressed file to: {0}", dest); + downloaded = true; + } +``` + + + +___ + + +Suggestion 3: + Add nullability guards **Add null checks for inputs and guard against null return from asm.GetName() before dereferencing.** @@ -48,167 +94,11 @@ Add nullability guards -___ - - -Suggestion 2: - -Add null guard before calls - -**Guard against a null _resourceServer after initialization before calling methods on it. Log an error and return early to avoid NullReferenceException if the server failed to initialize.** - -[WelsonJS.Toolkit/WelsonJS.Launcher/MainForm.cs [178-186]](https://github.com/gnh1201/welsonjs/pull/323/files#diff-e1f9629965d64fcb012ff0a87d9838aadc49ce7630c09a8907b9452f819b4effR178-R186) - -```diff - Program.InitializeResourceServer(); -+ -+if (Program._resourceServer == null) -+{ -+ _logger.Error("Resource server failed to initialize."); -+ return false; -+} - - if (!Program._resourceServer.IsRunning()) - { - Program._resourceServer.Start(false); - startCodeEditorToolStripMenuItem.Text = "Open the code editor..."; - } - - return Program._resourceServer.IsRunning(); -``` - - - -___ - - -Suggestion 3: - -Guard deserialization with null checks - -**Add a null check on the deserialization result before using it to avoid possible NullReferenceException. Also validate that the stream is not null before constructing the reader.** - -[WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [448-453]](https://github.com/gnh1201/welsonjs/pull/318/files#diff-818c326b0529d19f21de13c9a00464e3eaacb0de06acbbc0bcea444e1cd3bef8R448-R453) - -```diff - using (var response = await _httpClient.GetStreamAsync(url)) --using (var reader = new StreamReader(response)) - { -- var serializer = new XmlSerializer(typeof(BlobConfig)); -- _blobConfig = (BlobConfig)serializer.Deserialize(reader); -+ if (response == null) -+ { -+ _logger?.Warn("Blob config response stream was null."); -+ return; -+ } -+ using (var reader = new StreamReader(response)) -+ { -+ var serializer = new XmlSerializer(typeof(BlobConfig)); -+ var deserialized = serializer.Deserialize(reader) as BlobConfig; -+ if (deserialized == null) -+ { -+ _logger?.Warn("Failed to deserialize BlobConfig (null result)."); -+ return; -+ } -+ _blobConfig = deserialized; -+ } - } -``` - - - ___ Suggestion 4: -Fix null reference exception risk - -**The null check for schema should occur before accessing schema.PrimaryKey to prevent potential null reference exceptions. Move the null validation to the beginning of the constructor.** - -[WelsonJS.Toolkit/WelsonJS.Esent/EsentDatabase.cs [35-41]](https://github.com/gnh1201/welsonjs/pull/279/files#diff-21f279a29d8eeede6f37b552960fc5b0aef7c187c40481256398bd8a318f5590R35-R41) - -```diff - public EsentDatabase(Schema schema, string workingDirectory, ICompatibleLogger logger = null) - { -+ if (schema == null) -+ throw new ArgumentNullException(nameof(schema)); -+ - _logger = logger ?? new TraceLogger(); -- - _primaryKey = schema.PrimaryKey; - -- if (schema == null) -- -``` - - - -___ - - -Suggestion 5: - -Prevent substring exception - -**The JqueryCdnPrefix transform is removing the "jquery/" prefix from the path, but this could cause issues if the path doesn't have sufficient length. Add a check to prevent potential ArgumentOutOfRangeException when the path is too short.** - -[WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [196-210]](https://github.com/gnh1201/welsonjs/pull/245/files#diff-818c326b0529d19f21de13c9a00464e3eaacb0de06acbbc0bcea444e1cd3bef8R196-R210) - -```diff - private async Task TryServeFromCdn(HttpListenerContext context, string path) - { - bool isNodePackageExpression = Regex.IsMatch(path, @"^[^/@]+@[^/]+/"); - - var sources = new (bool isMatch, string configKey, Func transform)[] - { - (path.StartsWith("ajax/libs/"), "CdnJsPrefix", p => p), - (isNodePackageExpression, "UnpkgPrefix", p => p), - (isNodePackageExpression, "SkypackPrefix", p => p), - (isNodePackageExpression, "EsmShPrefix", p => p), - (isNodePackageExpression, "EsmRunPrefix", p => p), - (path.StartsWith("npm/") || path.StartsWith("gh/") || path.StartsWith("wp/"), "JsDeliverPrefix", p => p), -- (path.StartsWith("jquery/"), "JqueryCdnPrefix", p => p.Substring("jquery/".Length)), -+ (path.StartsWith("jquery/") && path.Length > "jquery/".Length, "JqueryCdnPrefix", p => p.Substring("jquery/".Length)), - (true, "BlobStoragePrefix", p => p) // fallback - }; -``` - - - -___ - - -Suggestion 6: - -Add null check - -**The function doesn't check if promptEditorRef.current exists before calling methods on it, which could lead to runtime errors if the ref isn't initialized.** - -[WelsonJS.Toolkit/WelsonJS.Launcher/editor.html [195-202]](https://github.com/gnh1201/welsonjs/pull/242/files#diff-ccffd2e8a5e0cef355ada30018830cd5516644b2e800c61b2298ac8260d778d5R195-R202) - -```diff - const invoke = () => { - try { -- const updated = promptEditorRef.current.get(); -- promptMessagesRef.current = updated; -+ if (promptEditorRef.current) { -+ const updated = promptEditorRef.current.get(); -+ promptMessagesRef.current = updated; -+ } - } catch (e) { - console.error("Invalid JSON structure", e); - } - }; -``` - - - -___ - - -Suggestion 7: - Handle empty/invalid inputs safely **Guard against empty coords after validation; current code will use Math.min/max on empty arrays, yielding Infinity/-Infinity and NaN density. Return false if no valid cells remain. Also coerce minDensity to a number and clamp between 0 and 1 to avoid unexpected truthy strings.** @@ -273,7 +163,7 @@ Handle empty/invalid inputs safely ___ -Suggestion 8: +Suggestion 5: Validate tile position inputs @@ -315,7 +205,72 @@ Validate tile position inputs ___ -Suggestion 9: +Suggestion 6: + +Guard deserialization with null checks + +**Add a null check on the deserialization result before using it to avoid possible NullReferenceException. Also validate that the stream is not null before constructing the reader.** + +[WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [448-453]](https://github.com/gnh1201/welsonjs/pull/318/files#diff-818c326b0529d19f21de13c9a00464e3eaacb0de06acbbc0bcea444e1cd3bef8R448-R453) + +```diff + using (var response = await _httpClient.GetStreamAsync(url)) +-using (var reader = new StreamReader(response)) + { +- var serializer = new XmlSerializer(typeof(BlobConfig)); +- _blobConfig = (BlobConfig)serializer.Deserialize(reader); ++ if (response == null) ++ { ++ _logger?.Warn("Blob config response stream was null."); ++ return; ++ } ++ using (var reader = new StreamReader(response)) ++ { ++ var serializer = new XmlSerializer(typeof(BlobConfig)); ++ var deserialized = serializer.Deserialize(reader) as BlobConfig; ++ if (deserialized == null) ++ { ++ _logger?.Warn("Failed to deserialize BlobConfig (null result)."); ++ return; ++ } ++ _blobConfig = deserialized; ++ } + } +``` + + + +___ + + +Suggestion 7: + +Fix null reference exception risk + +**The null check for schema should occur before accessing schema.PrimaryKey to prevent potential null reference exceptions. Move the null validation to the beginning of the constructor.** + +[WelsonJS.Toolkit/WelsonJS.Esent/EsentDatabase.cs [35-41]](https://github.com/gnh1201/welsonjs/pull/279/files#diff-21f279a29d8eeede6f37b552960fc5b0aef7c187c40481256398bd8a318f5590R35-R41) + +```diff + public EsentDatabase(Schema schema, string workingDirectory, ICompatibleLogger logger = null) + { ++ if (schema == null) ++ throw new ArgumentNullException(nameof(schema)); ++ + _logger = logger ?? new TraceLogger(); +- + _primaryKey = schema.PrimaryKey; + +- if (schema == null) +- +``` + + + +___ + + +Suggestion 8: Fix grammar and add validation @@ -337,7 +292,7 @@ Fix grammar and add validation ___ -Suggestion 10: +Suggestion 9: Add type check before parsing @@ -355,21 +310,59 @@ Add type check before parsing ___ -Suggestion 11: +Suggestion 10: -Null-check logger before use +Prevent substring exception -**Guard uses of Logger with a null check to avoid potential NullReferenceException when logging optional events. Add a simple Logger != null check before calling Info.** +**The JqueryCdnPrefix transform is removing the "jquery/" prefix from the path, but this could cause issues if the path doesn't have sufficient length. Add a check to prevent potential ArgumentOutOfRangeException when the path is too short.** -[WelsonJS.Toolkit/WelsonJS.Launcher/AssemblyLoader.cs [343-347]](https://github.com/gnh1201/welsonjs/pull/360/files#diff-70109da2862343aaf098ff9f7bce9d06f5f51331146ec54aa573ccbaa776fdaaR343-R347) +[WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [196-210]](https://github.com/gnh1201/welsonjs/pull/245/files#diff-818c326b0529d19f21de13c9a00464e3eaacb0de06acbbc0bcea444e1cd3bef8R196-R210) ```diff - if (isDll && TryDownloadCompressedFile(gzUrl, dest)) + private async Task TryServeFromCdn(HttpListenerContext context, string path) { -- Logger.Info("Downloaded and decompressed file to: {0}", dest); -+ if (Logger != null) Logger.Info("Downloaded and decompressed file to: {0}", dest); - downloaded = true; - } + bool isNodePackageExpression = Regex.IsMatch(path, @"^[^/@]+@[^/]+/"); + + var sources = new (bool isMatch, string configKey, Func transform)[] + { + (path.StartsWith("ajax/libs/"), "CdnJsPrefix", p => p), + (isNodePackageExpression, "UnpkgPrefix", p => p), + (isNodePackageExpression, "SkypackPrefix", p => p), + (isNodePackageExpression, "EsmShPrefix", p => p), + (isNodePackageExpression, "EsmRunPrefix", p => p), + (path.StartsWith("npm/") || path.StartsWith("gh/") || path.StartsWith("wp/"), "JsDeliverPrefix", p => p), +- (path.StartsWith("jquery/"), "JqueryCdnPrefix", p => p.Substring("jquery/".Length)), ++ (path.StartsWith("jquery/") && path.Length > "jquery/".Length, "JqueryCdnPrefix", p => p.Substring("jquery/".Length)), + (true, "BlobStoragePrefix", p => p) // fallback + }; +``` + + + +___ + + +Suggestion 11: + +Add null check + +**The function doesn't check if promptEditorRef.current exists before calling methods on it, which could lead to runtime errors if the ref isn't initialized.** + +[WelsonJS.Toolkit/WelsonJS.Launcher/editor.html [195-202]](https://github.com/gnh1201/welsonjs/pull/242/files#diff-ccffd2e8a5e0cef355ada30018830cd5516644b2e800c61b2298ac8260d778d5R195-R202) + +```diff + const invoke = () => { + try { +- const updated = promptEditorRef.current.get(); +- promptMessagesRef.current = updated; ++ if (promptEditorRef.current) { ++ const updated = promptEditorRef.current.get(); ++ promptMessagesRef.current = updated; ++ } + } catch (e) { + console.error("Invalid JSON structure", e); + } + }; ``` @@ -381,27 +374,33 @@ Null-check logger before use ___ -Pattern 2: Do not silently swallow exceptions; log meaningful context (operation + key identifiers) and either rethrow for critical failures or return an explicit failure result that preserves error details. +Pattern 2: Do not use unobserved fire-and-forget async calls; instead return Task where possible, or capture the Task and attach a continuation to log/handle faults to avoid swallowed exceptions and startup races. Example code before: ``` -try { - DoNetworkCall(); -} catch { - return false; // no context, hard to diagnose +void Init() { + LoadConfigAsync(); // fire-and-forget, exceptions may be lost +} + +async void LoadConfigAsync() { + await http.GetStringAsync(url); } ``` Example code after: ``` -try { - DoNetworkCall(); - return true; -} catch (Exception ex) { - logger?.Warn($"DoNetworkCall failed for url={url}: {ex.Message}"); - return false; // or: throw; +Task InitAsync() { + return LoadConfigAsync(); // caller can await } + +async Task LoadConfigAsync() { + await http.GetStringAsync(url); +} + +// If you truly can't await: +_ = LoadConfigAsync().ContinueWith(t => logger?.Error(t.Exception), + TaskContinuationOptions.OnlyOnFaulted); ```
Relevant past accepted suggestions: @@ -450,28 +449,6 @@ ___ Suggestion 2: -Log exceptions instead of swallowing them - -**In TryDownloadGzipToFile, modify the catch block to log the exception details before returning false, instead of silently swallowing the error.** - -[WelsonJS.Toolkit/WelsonJS.Launcher/AssemblyLoader.cs [389-392]](https://github.com/gnh1201/welsonjs/pull/359/files#diff-70109da2862343aaf098ff9f7bce9d06f5f51331146ec54aa573ccbaa776fdaaR389-R392) - -```diff --catch -+catch (Exception ex) - { -+ Logger?.Warn("Failed to download or decompress gzipped file from {0}: {1}", gzUrl, ex.Message); - return false; - } -``` - - - -___ - - -Suggestion 3: - Log exceptions in telemetry provider **In PosthogTelemetryProvider, log exceptions that occur when sending telemetry data instead of silently swallowing them. This will require passing a logger into the provider.** @@ -503,25 +480,57 @@ Log exceptions in telemetry provider ___ -Suggestion 4: +Suggestion 3: -Improve error handling for exceptions +Guard fire-and-forget telemetry task -**Modify the catch block to handle both exception objects and simple string errors by checking the type of $_ before attempting to access $_.Exception.Message.** +**Do not fire-and-forget the async call; capture the Task and log any faults to avoid silent failures.** -[afterInstall.ps1 [370-374]](https://github.com/gnh1201/welsonjs/pull/347/files#diff-f949d7ffdfe1d45af92618873869a6a2af4eccbe25e49b4de9992b51205f1ea7R370-R374) +[WelsonJS.Toolkit/WelsonJS.Launcher/Program.cs [86-87]](https://github.com/gnh1201/welsonjs/pull/357/files#diff-b39850b00a6addf57c4f05806bb4caaf4dafbcd28211377f08e705cd021ebf11R86-R87) ```diff - catch { - Write-Host "[FATAL] Extraction/installation phase failed." -- Write-Host $_.Exception.Message -+ if ($_ -is [System.Exception]) { -+ Write-Host $_.Exception.Message -+ } else { -+ Write-Host $_ -+ } - exit 1 - } + // send event to the telemetry server +-_telemetryClient.TrackAppStartedAsync("WelsonJS.Launcher", "0.2.7.57"); ++_ = _telemetryClient.TrackAppStartedAsync("WelsonJS.Launcher", "0.2.7.57") ++ .ContinueWith(t => { if (t.IsFaulted) _logger?.Error($"Telemetry failed: {t.Exception}"); }, ++ TaskScheduler.Default); +``` + + + +___ + + +Suggestion 4: + +Safely handle async init task + +**The async call to fetch configuration is fire-and-forget, which can swallow exceptions and race with code that depends on _blobConfig. Capture the task and log exceptions, or await during startup. If startup cannot be async, store the task and handle its exceptions on a background continuation.** + +[WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [45-57]](https://github.com/gnh1201/welsonjs/pull/318/files#diff-818c326b0529d19f21de13c9a00464e3eaacb0de06acbbc0bcea444e1cd3bef8R45-R57) + +```diff + public ResourceServer(string prefix, string resourceName, ICompatibleLogger logger = null) + { + _logger = logger ?? new TraceLogger(); + _prefix = prefix; + _listener = new HttpListener(); + _resourceName = resourceName; + +- // Fetch a blob config from Internet +- FetchBlobConfig().ConfigureAwait(false); ++ // Fetch a blob config from Internet (safe fire-and-forget with logging) ++ _ = FetchBlobConfig().ContinueWith(t => ++ { ++ if (t.IsFaulted) ++ { ++ _logger?.Error($"FetchBlobConfig failed: {t.Exception}"); ++ } ++ }, TaskScheduler.Default); + + // Add resource tools + _tools.Add(new ResourceTools.Completion(this, _httpClient)); + _tools.Add(new ResourceTools.Settings(this, _httpClient)); ``` @@ -531,17 +540,112 @@ ___ Suggestion 5: -Return structured error on failure +Fix async method signature -**Preserve error context in the response to allow callers to detect failures instead of returning an indistinguishable empty object. Include an error flag and message so downstream logic can branch appropriately.** +**The FetchBlobConfig method is asynchronous but doesn't return a Task, making it fire-and-forget. This could lead to unhandled exceptions and the application might continue without a properly loaded blob configuration. Consider making it return a Task and awaiting it during initialization.** -[lib/chrome.js [452-455]](https://github.com/gnh1201/welsonjs/pull/309/files#diff-868ee66369a322a75a321ede18241bfb18f199a9e5bb5371d4d049f0af14f598R452-R455) +[WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [425-444]](https://github.com/gnh1201/welsonjs/pull/249/files#diff-818c326b0529d19f21de13c9a00464e3eaacb0de06acbbc0bcea444e1cd3bef8R425-R444) ```diff - } catch (e) { - console.warn(e.message); -- response = {}; -+ response = { error: true, message: e && e.message ? e.message : "Request failed" }; +-private static async void FetchBlobConfig() ++private static async Task FetchBlobConfig() + { + try + { + string url = Program.GetAppConfig("BlobConfigUrl"); + var response = await _httpClient.GetStreamAsync(url); + + var serializer = new XmlSerializer(typeof(BlobConfig)); + using (var reader = new StreamReader(response)) + { + _blobConfig = (BlobConfig)serializer.Deserialize(reader); + } + + _blobConfig?.Compile(); + } + catch (Exception ex) + { + Trace.TraceError($"Failed to fetch a blob config. Exception: {ex.Message}"); + } + } +``` + + + +
+ + + +___ + + +Pattern 3: Manage I/O and native/COM resources defensively: ensure deterministic disposal, avoid handle leaks, prefer atomic write patterns (temp file + move), and surface failures with logged context instead of swallowing exceptions. + + +Example code before: +``` +var fs = new FileStream(dest, FileMode.Create); +stream.CopyTo(fs); // fs not disposed if CopyTo throws + +try { DoWork(); } catch { return false; } // loses error context +``` + +Example code after: +``` +var tmp = dest + ".tmp"; +try { + using (var fs = new FileStream(tmp, FileMode.Create, FileAccess.Write, FileShare.None)) { + stream.CopyTo(fs); + } + File.Move(tmp, dest, overwrite: true); +} finally { + if (File.Exists(tmp)) File.Delete(tmp); +} + +try { DoWork(); } +catch (Exception ex) { logger?.Warn($"DoWork failed: {ex.Message}"); return false; } +``` +
Relevant past accepted suggestions: + + +___ + + +Suggestion 1: + +Validate callback before object creation + + + + + +**In the UseObject function, validate the callback parameter before calling CreateObject to prevent unnecessary object creation and disposal.** + +[app.js [191-208]](https://github.com/gnh1201/welsonjs/pull/386/files#diff-e07d531ac040ce3f40e0ce632ac2a059d7cd60f20e61f78268ac3be015b3b28fR191-R208) + +```diff + var UseObject = function(progId, callback, dispose) { ++ if (typeof callback !== "function") { ++ return null; ++ } ++ + if (typeof dispose !== "function") { + dispose = function(obj) { + try { + obj.Close(); + } catch (e) { /* ignore */ } + }; + } + + var obj = CreateObject(progId); + try { +- return (typeof callback === "function" ? +- callback(obj) : null); ++ return callback(obj); + } finally { + dispose(obj); + obj = null; + } } ``` @@ -550,7 +654,68 @@ Return structured error on failure ___ -Suggestion 6: +Suggestion 2: + +Use a temporary file for downloads + +**To prevent partial or corrupt files on download failure, write the decompressed stream to a temporary file first, and then atomically move it to the final destination upon success.** + +[WelsonJS.Toolkit/WelsonJS.Launcher/AssemblyLoader.cs [379-384]](https://github.com/gnh1201/welsonjs/pull/359/files#diff-70109da2862343aaf098ff9f7bce9d06f5f51331146ec54aa573ccbaa776fdaaR379-R384) + +```diff +-using (Stream s = res.Content.ReadAsStreamAsync().GetAwaiter().GetResult()) +-using (var gz = new GZipStream(s, CompressionMode.Decompress)) +-using (var fs = new FileStream(dest, FileMode.Create, FileAccess.Write)) ++string tempFile = dest + ".tmp"; ++try + { +- gz.CopyTo(fs); ++ using (Stream s = res.Content.ReadAsStreamAsync().GetAwaiter().GetResult()) ++ using (var gz = new GZipStream(s, CompressionMode.Decompress)) ++ using (var fs = new FileStream(tempFile, FileMode.Create, FileAccess.Write)) ++ { ++ gz.CopyTo(fs); ++ } ++ ++ File.Move(tempFile, dest); ++} ++finally ++{ ++ if (File.Exists(tempFile)) ++ { ++ File.Delete(tempFile); ++ } + } +``` + + + +___ + + +Suggestion 3: + +Log exceptions instead of swallowing them + +**In TryDownloadGzipToFile, modify the catch block to log the exception details before returning false, instead of silently swallowing the error.** + +[WelsonJS.Toolkit/WelsonJS.Launcher/AssemblyLoader.cs [389-392]](https://github.com/gnh1201/welsonjs/pull/359/files#diff-70109da2862343aaf098ff9f7bce9d06f5f51331146ec54aa573ccbaa776fdaaR389-R392) + +```diff +-catch ++catch (Exception ex) + { ++ Logger?.Warn("Failed to download or decompress gzipped file from {0}: {1}", gzUrl, ex.Message); + return false; + } +``` + + + +___ + + +Suggestion 4: Improve native library loading security @@ -610,242 +775,10 @@ Improve native library loading security -
- - - -___ - - -Pattern 3: When startup or event handlers kick off async work, avoid unsafe fire-and-forget; capture the Task and ensure faults are observed (log them), or change APIs to return Task and await them during initialization. - - -Example code before: -``` -// C# -FetchConfigAsync(); // fire-and-forget, exceptions can be lost -UseConfig(_config); // may run before config is loaded -``` - -Example code after: -``` -// C# -_ = FetchConfigAsync().ContinueWith(t => -{ - if (t.IsFaulted) logger?.Error($"FetchConfigAsync failed: {t.Exception}"); -}, TaskScheduler.Default); - -// or (preferred when possible): -// await FetchConfigAsync(); -``` -
Relevant past accepted suggestions: - - ___ -Suggestion 1: - -Guard fire-and-forget telemetry task - -**Do not fire-and-forget the async call; capture the Task and log any faults to avoid silent failures.** - -[WelsonJS.Toolkit/WelsonJS.Launcher/Program.cs [86-87]](https://github.com/gnh1201/welsonjs/pull/357/files#diff-b39850b00a6addf57c4f05806bb4caaf4dafbcd28211377f08e705cd021ebf11R86-R87) - -```diff - // send event to the telemetry server --_telemetryClient.TrackAppStartedAsync("WelsonJS.Launcher", "0.2.7.57"); -+_ = _telemetryClient.TrackAppStartedAsync("WelsonJS.Launcher", "0.2.7.57") -+ .ContinueWith(t => { if (t.IsFaulted) _logger?.Error($"Telemetry failed: {t.Exception}"); }, -+ TaskScheduler.Default); -``` - - - -___ - - -Suggestion 2: - -Safely handle async init task - -**The async call to fetch configuration is fire-and-forget, which can swallow exceptions and race with code that depends on _blobConfig. Capture the task and log exceptions, or await during startup. If startup cannot be async, store the task and handle its exceptions on a background continuation.** - -[WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [45-57]](https://github.com/gnh1201/welsonjs/pull/318/files#diff-818c326b0529d19f21de13c9a00464e3eaacb0de06acbbc0bcea444e1cd3bef8R45-R57) - -```diff - public ResourceServer(string prefix, string resourceName, ICompatibleLogger logger = null) - { - _logger = logger ?? new TraceLogger(); - _prefix = prefix; - _listener = new HttpListener(); - _resourceName = resourceName; - -- // Fetch a blob config from Internet -- FetchBlobConfig().ConfigureAwait(false); -+ // Fetch a blob config from Internet (safe fire-and-forget with logging) -+ _ = FetchBlobConfig().ContinueWith(t => -+ { -+ if (t.IsFaulted) -+ { -+ _logger?.Error($"FetchBlobConfig failed: {t.Exception}"); -+ } -+ }, TaskScheduler.Default); - - // Add resource tools - _tools.Add(new ResourceTools.Completion(this, _httpClient)); - _tools.Add(new ResourceTools.Settings(this, _httpClient)); -``` - - - -___ - - -Suggestion 3: - -Fix async method signature - -**The FetchBlobConfig method is asynchronous but doesn't return a Task, making it fire-and-forget. This could lead to unhandled exceptions and the application might continue without a properly loaded blob configuration. Consider making it return a Task and awaiting it during initialization.** - -[WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [425-444]](https://github.com/gnh1201/welsonjs/pull/249/files#diff-818c326b0529d19f21de13c9a00464e3eaacb0de06acbbc0bcea444e1cd3bef8R425-R444) - -```diff --private static async void FetchBlobConfig() -+private static async Task FetchBlobConfig() - { - try - { - string url = Program.GetAppConfig("BlobConfigUrl"); - var response = await _httpClient.GetStreamAsync(url); - - var serializer = new XmlSerializer(typeof(BlobConfig)); - using (var reader = new StreamReader(response)) - { - _blobConfig = (BlobConfig)serializer.Deserialize(reader); - } - - _blobConfig?.Compile(); - } - catch (Exception ex) - { - Trace.TraceError($"Failed to fetch a blob config. Exception: {ex.Message}"); - } - } -``` - - - -___ - - -Suggestion 4: - -Log exceptions in telemetry provider - -**In PosthogTelemetryProvider, log exceptions that occur when sending telemetry data instead of silently swallowing them. This will require passing a logger into the provider.** - -[WelsonJS.Toolkit/WelsonJS.Launcher/Telemetry/PosthogTelemetryProvider.cs [64-75]](https://github.com/gnh1201/welsonjs/pull/357/files#diff-55c9e372e67bc0c3c8727ddfcd66c20f8f0bf5512420be0919341df364218738R64-R75) - -```diff - try - { -- await _httpClient.PostAsync( -+ var response = await _httpClient.PostAsync( - url, - new StringContent(json, Encoding.UTF8, "application/json"), - cancellationToken - ); -+ response.EnsureSuccessStatusCode(); - } --catch -+catch (Exception ex) - { -- // swallow for fire-and-forget telemetry -+ // Log and swallow for fire-and-forget telemetry -+ _logger?.Error($"Failed to send telemetry event '{eventName}': {ex.Message}"); - } -``` - - - -
- - - -___ - - -Pattern 4: Use safe resource/file handling patterns: ensure streams/handles/COM objects are disposed, avoid file handle leaks, and write downloads/extractions to a temporary path then atomically move into place on success. - - -Example code before: -``` -var fs = new FileStream(dest, FileMode.Create); -responseStream.CopyTo(fs); // if this fails, dest is left half-written -// fs might not be disposed on exception -``` - -Example code after: -``` -var tmp = dest + ".tmp"; -try -{ - using (var fs = new FileStream(tmp, FileMode.Create, FileAccess.Write, FileShare.None)) - responseStream.CopyTo(fs); - File.Move(tmp, dest, overwrite: true); -} -finally -{ - if (File.Exists(tmp)) File.Delete(tmp); -} -``` -
Relevant past accepted suggestions: - - -___ - - -Suggestion 1: - -Use a temporary file for downloads - -**To prevent partial or corrupt files on download failure, write the decompressed stream to a temporary file first, and then atomically move it to the final destination upon success.** - -[WelsonJS.Toolkit/WelsonJS.Launcher/AssemblyLoader.cs [379-384]](https://github.com/gnh1201/welsonjs/pull/359/files#diff-70109da2862343aaf098ff9f7bce9d06f5f51331146ec54aa573ccbaa776fdaaR379-R384) - -```diff --using (Stream s = res.Content.ReadAsStreamAsync().GetAwaiter().GetResult()) --using (var gz = new GZipStream(s, CompressionMode.Decompress)) --using (var fs = new FileStream(dest, FileMode.Create, FileAccess.Write)) -+string tempFile = dest + ".tmp"; -+try - { -- gz.CopyTo(fs); -+ using (Stream s = res.Content.ReadAsStreamAsync().GetAwaiter().GetResult()) -+ using (var gz = new GZipStream(s, CompressionMode.Decompress)) -+ using (var fs = new FileStream(tempFile, FileMode.Create, FileAccess.Write)) -+ { -+ gz.CopyTo(fs); -+ } -+ -+ File.Move(tempFile, dest); -+} -+finally -+{ -+ if (File.Exists(tempFile)) -+ { -+ File.Delete(tempFile); -+ } - } -``` - - - -___ - - -Suggestion 2: +Suggestion 5: Fix a file handle leak @@ -876,38 +809,7 @@ Fix a file handle leak ___ -Suggestion 3: - -Use UseObject pattern for resource management - -**The function should use the new UseObject pattern for proper resource management. This ensures the COM object is properly cleaned up even if an exception occurs during the operation.** - -[lib/file.js [63-70]](https://github.com/gnh1201/welsonjs/pull/307/files#diff-387904f6b07263914bc1b0cb291a27832cc6963e01b5d44b75460771fc25ee8fR63-R70) - -```diff - function writeBinaryFile(path, data) { -- var binaryStream = CreateObject("ADODB.Stream"); -- binaryStream.Type = PipeIPC.adTypeBinary; -- binaryStream.Open(); -- binaryStream.Write(data); -- binaryStream.SaveToFile(path, adSaveCreateOverWrite); -- binaryStream.Close(); -+ return UseObject("ADODB.Stream", function(binaryStream) { -+ binaryStream.Type = PipeIPC.adTypeBinary; -+ binaryStream.Open(); -+ binaryStream.Write(data); -+ binaryStream.SaveToFile(path, adSaveCreateOverWrite); -+ binaryStream.Close(); -+ }); - } -``` - - - -___ - - -Suggestion 4: +Suggestion 6: Make trace listener setup safer @@ -948,6 +850,37 @@ Make trace listener setup safer +___ + + +Suggestion 7: + +Use UseObject pattern for resource management + +**The function should use the new UseObject pattern for proper resource management. This ensures the COM object is properly cleaned up even if an exception occurs during the operation.** + +[lib/file.js [63-70]](https://github.com/gnh1201/welsonjs/pull/307/files#diff-387904f6b07263914bc1b0cb291a27832cc6963e01b5d44b75460771fc25ee8fR63-R70) + +```diff + function writeBinaryFile(path, data) { +- var binaryStream = CreateObject("ADODB.Stream"); +- binaryStream.Type = PipeIPC.adTypeBinary; +- binaryStream.Open(); +- binaryStream.Write(data); +- binaryStream.SaveToFile(path, adSaveCreateOverWrite); +- binaryStream.Close(); ++ return UseObject("ADODB.Stream", function(binaryStream) { ++ binaryStream.Type = PipeIPC.adTypeBinary; ++ binaryStream.Open(); ++ binaryStream.Write(data); ++ binaryStream.SaveToFile(path, adSaveCreateOverWrite); ++ binaryStream.Close(); ++ }); + } +``` + + +
@@ -955,24 +888,257 @@ Make trace listener setup safer ___ -Pattern 5: Prefer compatibility- and correctness-focused APIs over brittle assumptions: use Path.Combine for paths, TryParse (and case-insensitive comparisons) for enums/strings, and handle cross-arch/platform differences (e.g., registry value types, protocol availability). +Pattern 4: Avoid insecure or privacy-risky data propagation: do not reflect request headers into CORS responses, ensure correct caching headers for CORS, add required SRI attributes, and sanitize/limit sensitive content before sending it to external services. + + +Example code before: +``` +// CORS: reflect request headers (unsafe) +res.setHeader("Access-Control-Allow-Headers", req.headers["access-control-request-headers"]); + +// External AI: send full text payload +ai.send(emailBody); +``` + +Example code after: +``` +// CORS: fixed allow-list +res.setHeader("Access-Control-Allow-Headers", "Content-Type, Authorization"); +res.setHeader("Access-Control-Allow-Methods", "GET, POST, OPTIONS"); +if (originAllowed) res.setHeader("Vary", "Origin"); + +// External AI: redact + truncate, optionally require consent +const safe = redact(emailBody).slice(0, 1000); +ai.send(safe); +``` +
Relevant past accepted suggestions: + + +___ + + +Suggestion 1: + +Sending full email content externally poses a major privacy risk + + + + + +**The code sends the full body of multiple emails to an external AI, creating a significant privacy and security risk. This should be addressed with data sanitization, summarization, or user consent before being merged.** + + +### Examples: + + + + + +testloader.js [1395-1428] + + + + +```javascript + results.forEach(function (m, i) { + var body = String(m.getBody() || ""); + var preview = body.replace(/\r/g, "").replace(/\n+/g, " ").substr(0, previewLen); + + var text = "#" + String(i) + + " | From: " + String(m.getSenderEmailAddress()) + + " | To: " + String(m.mail.To || "") + + " | Subject: " + String(m.getSubject()) + + " | Received: " + String(m.getReceivedTime()); + + + ... (clipped 24 lines) +``` + + + + + +### Solution Walkthrough: + + + +#### Before: +```javascript +function outlook_open_outlook_with_chatgpt() { + var prompt_texts = []; + var results = outlook.searchBySenderOrRecipientContains(keyword); + + results.forEach(function (m, i) { + var body = String(m.getBody() || ""); // Get full email body + + // Add email metadata to prompt + prompt_texts.push(...); + + // Add FULL, raw email body to prompt + prompt_texts.push(" Body: " + body); + }, 10); + + var prompt_text_completed = prompt_texts.join("\r\n"); + + // Send the entire collected text, including full email bodies, to OpenAI + var response_text = LIE.create().setProvider("openai").inference(prompt_text_completed, 0).join(' '); +} + +``` + + + +#### After: +```javascript +function outlook_open_outlook_with_chatgpt() { + var prompt_texts = []; + var results = outlook.searchBySenderOrRecipientContains(keyword); + + results.forEach(function (m, i) { + var body = String(m.getBody() || ""); + + // Example: Sanitize or summarize the body before adding it to the prompt + var processed_body = sanitizeAndSummarize(body); + + // Example: Obtain user consent before processing sensitive data + if (!confirm("Allow sending content of email '" + m.getSubject() + "' to AI?")) { + return; // Skip if user denies + } + + prompt_texts.push(" Body: " + processed_body); + }, 10); + + var prompt_text_completed = prompt_texts.join("\r\n"); + var response_text = LIE.create().setProvider("openai").inference(prompt_text_completed, 0).join(' '); +} + +``` + + + +___ + + +Suggestion 2: + +truncate full body input + + + + + +**Truncate the full email body to a maximum length before adding it to prompt_texts to avoid exceeding the AI model's context window.** + +[testloader.js [1411]](https://github.com/gnh1201/welsonjs/pull/382/files#diff-a0a90600928bcca35f3491bcc014ca7e144f2a31ab9f38f86086e13a4305e546R1411-R1411) + +```diff +-prompt_texts.push(" Body: " + body); ++var maxBodyLen = 1000; ++prompt_texts.push(" Body: " + body.substr(0, maxBodyLen)); +``` + + + +___ + + +Suggestion 3: + +Avoid reflecting request headers in response + +**To improve security, use a fixed, server-approved list for Access-Control-Allow-Methods and Access-Control-Allow-Headers response headers instead of reflecting values from the request.** + +[WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [547-553]](https://github.com/gnh1201/welsonjs/pull/334/files#diff-818c326b0529d19f21de13c9a00464e3eaacb0de06acbbc0bcea444e1cd3bef8R547-R553) + +```diff + var h = context.Response.Headers; +-h["Access-Control-Allow-Methods"] = string.IsNullOrEmpty(requestMethod) +- ? "GET, POST, PUT, DELETE, OPTIONS" +- : requestMethod; +-h["Access-Control-Allow-Headers"] = string.IsNullOrEmpty(requestHeaders) +- ? "Content-Type, Authorization, X-Requested-With" +- : requestHeaders; ++h["Access-Control-Allow-Methods"] = "GET, POST, PUT, DELETE, OPTIONS"; ++h["Access-Control-Allow-Headers"] = "Content-Type, Authorization, X-Requested-With"; +``` + + + +___ + + +Suggestion 4: + +Only set Vary header on CORS responses + +**Move the addition of the Vary: Origin header to only occur when a request's origin is successfully validated and a CORS response is generated. This will prevent potential caching issues.** + +[WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [514-530]](https://github.com/gnh1201/welsonjs/pull/334/files#diff-818c326b0529d19f21de13c9a00464e3eaacb0de06acbbc0bcea444e1cd3bef8R514-R530) + +```diff + var respHeaders = context.Response.Headers; +-respHeaders["Vary"] = "Origin"; + + if (allowed.Any(a => a == "*")) + { + respHeaders["Access-Control-Allow-Origin"] = "*"; ++ respHeaders["Vary"] = "Origin"; + return true; + } + + if (allowed.Contains(origin, StringComparer.OrdinalIgnoreCase)) + { + respHeaders["Access-Control-Allow-Origin"] = origin; + respHeaders["Access-Control-Allow-Credentials"] = "true"; ++ respHeaders["Vary"] = "Origin"; + return true; + } + + return false; +``` + + + +___ + + +Suggestion 5: + +Add missing crossorigin attribute + +**The integrity attribute for the lodash script is missing the 'crossorigin' attribute which is required for Subresource Integrity (SRI) checks to work properly.** + +[WelsonJS.Toolkit/WelsonJS.Launcher/editor.html [53]](https://github.com/gnh1201/welsonjs/pull/242/files#diff-ccffd2e8a5e0cef355ada30018830cd5516644b2e800c61b2298ac8260d778d5R53-R53) + +```diff +- ++ +``` + + + +
+ + + +___ + + +Pattern 5: Prefer robust platform/cross-environment compatibility patterns: use standard library helpers (e.g., Path.Combine), case-insensitive comparisons where specs require it, avoid hardcoded version/config values, and add feature-detection fallbacks for older runtimes. Example code before: ``` var path = baseDir + "\\" + fileName; -var v = (MyEnum)Enum.Parse(typeof(MyEnum), input); -if (set.Contains(uri.Scheme)) { ... } // case-sensitive +if (allowedSchemes.includes(uri.scheme)) { ... } +var version = "1.2.3"; ``` Example code after: ``` var path = Path.Combine(baseDir, fileName); - -if (!Enum.TryParse(input, ignoreCase: true, out var v)) - return false; - -if (set.Contains(uri.Scheme, StringComparer.OrdinalIgnoreCase)) { ... } +if (allowedSchemes.Contains(uri.Scheme, StringComparer.OrdinalIgnoreCase)) { ... } +var version = Assembly.GetExecutingAssembly().GetName().Version?.ToString(); ```
Relevant past accepted suggestions: @@ -1000,6 +1166,112 @@ ___ Suggestion 2: +Improve compatibility with older PowerShell + +**To improve backward compatibility, check if Tls12 and Tls11 protocols exist before setting them, preventing errors on older PowerShell versions that do not support them.** + +[postInstall.ps1 [33-37]](https://github.com/gnh1201/welsonjs/pull/375/files#diff-23ea1b89de4aef4a17475eec8f60b9e0076fdccbf6054b3a02b05c417c627f35R33-R37) + +```diff + # Fix TLS 1.2 connectivity issue (Tested in Windows 8.1) +-[Net.ServicePointManager]::SecurityProtocol = ` +- [Net.SecurityProtocolType]::Tls12 -bor ` +- [Net.SecurityProtocolType]::Tls11 -bor ` +- [Net.SecurityProtocolType]::Tls ++$supportedProtocols = [Net.SecurityProtocolType]::Tls ++if ([Enum]::GetNames([Net.SecurityProtocolType]) -contains 'Tls11') { ++ $supportedProtocols = $supportedProtocols -bor [Net.SecurityProtocolType]::Tls11 ++} ++if ([Enum]::GetNames([Net.SecurityProtocolType]) -contains 'Tls12') { ++ $supportedProtocols = $supportedProtocols -bor [Net.SecurityProtocolType]::Tls12 ++} ++[Net.ServicePointManager]::SecurityProtocol = $supportedProtocols +``` + + + +___ + + +Suggestion 3: + +Use case-insensitive URI scheme comparison + +**In the IsValidUriScheme method, use a case-insensitive comparer when checking if _allowSchemes contains the uri.Scheme to align with RFC 3986.** + +[WelsonJS.Augmented/Catswords.Phantomizer/AssemblyLoader.cs [677-683]](https://github.com/gnh1201/welsonjs/pull/370/files#diff-83a483e9d7ff70dc70d062328284abd1ad4301208ceae0ab91e801eb17497b5bR677-R683) + +```diff + private static bool IsValidUriScheme(Uri uri) + { + if (uri == null) + return false; + +- return _allowSchemes.Contains(uri.Scheme); ++ return _allowSchemes.Contains(uri.Scheme, StringComparer.OrdinalIgnoreCase); + } +``` + + + +___ + + +Suggestion 4: + +Avoid hardcoding the application version + +**Avoid hardcoding the application version "0.2.7.57" in Program.cs by dynamically retrieving it from the assembly information for telemetry tracking.** + +[WelsonJS.Toolkit/WelsonJS.Launcher/Program.cs [86-87]](https://github.com/gnh1201/welsonjs/pull/357/files#diff-b39850b00a6addf57c4f05806bb4caaf4dafbcd28211377f08e705cd021ebf11R86-R87) + +```diff + // send event to the telemetry server +-_telemetryClient.TrackAppStartedAsync("WelsonJS.Launcher", "0.2.7.57"); ++var version = System.Reflection.Assembly.GetExecutingAssembly().GetName().Version.ToString(); ++_telemetryClient.TrackAppStartedAsync("WelsonJS.Launcher", version); +``` + + + +___ + + +Suggestion 5: + +Handle long type for registry values + +**In ReadDwordHKLM, add a check for long type registry values and cast them to int to correctly handle reads by 32-bit processes on 64-bit operating systems.** + +[WelsonJS.Augmented/Catswords.TlsReport/Tls12OfflineInspector.cs [645-660]](https://github.com/gnh1201/welsonjs/pull/378/files#diff-e1c803ec24ac0f3fc9ca70c1b1ba5747e633a316075a71f88c0d6ee012ed5a61R645-R660) + +```diff + private static int? ReadDwordHKLM(string subKeyPath, string valueName) + { + try + { + using (var key = Registry.LocalMachine.OpenSubKey(subKeyPath, false)) + { + if (key == null) return null; + object v = key.GetValue(valueName, null); + if (v == null) return null; + if (v is int i) return i; ++ if (v is long l) return (int)l; // Handle 64-bit to 32-bit conversion + if (v is byte[] b && b.Length >= 4) return BitConverter.ToInt32(b, 0); + return null; + } + } + catch { return null; } + } +``` + + + +___ + + +Suggestion 6: + Use Enum.TryParse for better performance **Replace the Enum.Parse call and its surrounding try-catch block with Enum.TryParse to improve performance and simplify the code.** @@ -1034,120 +1306,11 @@ Use Enum.TryParse for better performance -___ - - -Suggestion 3: - -Use case-insensitive enum parsing - -**Make enum parsing case-insensitive by using the Enum.Parse overload that accepts a boolean ignoreCase argument.** - -[WelsonJS.Augmented/Catswords.Phantomizer/AssemblyLoader.cs [775-779]](https://github.com/gnh1201/welsonjs/pull/377/files#diff-83a483e9d7ff70dc70d062328284abd1ad4301208ceae0ab91e801eb17497b5bR775-R779) - -```diff - SecurityProtocolType p = - (SecurityProtocolType)Enum.Parse( - typeof(SecurityProtocolType), -- protocolName -+ protocolName, -+ true // Case-insensitive - ); -``` - - - -___ - - -Suggestion 4: - -Improve compatibility with older PowerShell - -**To improve backward compatibility, check if Tls12 and Tls11 protocols exist before setting them, preventing errors on older PowerShell versions that do not support them.** - -[postInstall.ps1 [33-37]](https://github.com/gnh1201/welsonjs/pull/375/files#diff-23ea1b89de4aef4a17475eec8f60b9e0076fdccbf6054b3a02b05c417c627f35R33-R37) - -```diff - # Fix TLS 1.2 connectivity issue (Tested in Windows 8.1) --[Net.ServicePointManager]::SecurityProtocol = ` -- [Net.SecurityProtocolType]::Tls12 -bor ` -- [Net.SecurityProtocolType]::Tls11 -bor ` -- [Net.SecurityProtocolType]::Tls -+$supportedProtocols = [Net.SecurityProtocolType]::Tls -+if ([Enum]::GetNames([Net.SecurityProtocolType]) -contains 'Tls11') { -+ $supportedProtocols = $supportedProtocols -bor [Net.SecurityProtocolType]::Tls11 -+} -+if ([Enum]::GetNames([Net.SecurityProtocolType]) -contains 'Tls12') { -+ $supportedProtocols = $supportedProtocols -bor [Net.SecurityProtocolType]::Tls12 -+} -+[Net.ServicePointManager]::SecurityProtocol = $supportedProtocols -``` - - - -___ - - -Suggestion 5: - -Use case-insensitive URI scheme comparison - -**In the IsValidUriScheme method, use a case-insensitive comparer when checking if _allowSchemes contains the uri.Scheme to align with RFC 3986.** - -[WelsonJS.Augmented/Catswords.Phantomizer/AssemblyLoader.cs [677-683]](https://github.com/gnh1201/welsonjs/pull/370/files#diff-83a483e9d7ff70dc70d062328284abd1ad4301208ceae0ab91e801eb17497b5bR677-R683) - -```diff - private static bool IsValidUriScheme(Uri uri) - { - if (uri == null) - return false; - -- return _allowSchemes.Contains(uri.Scheme); -+ return _allowSchemes.Contains(uri.Scheme, StringComparer.OrdinalIgnoreCase); - } -``` - - - -___ - - -Suggestion 6: - -Handle long type for registry values - -**In ReadDwordHKLM, add a check for long type registry values and cast them to int to correctly handle reads by 32-bit processes on 64-bit operating systems.** - -[WelsonJS.Augmented/Catswords.TlsReport/Tls12OfflineInspector.cs [645-660]](https://github.com/gnh1201/welsonjs/pull/378/files#diff-e1c803ec24ac0f3fc9ca70c1b1ba5747e633a316075a71f88c0d6ee012ed5a61R645-R660) - -```diff - private static int? ReadDwordHKLM(string subKeyPath, string valueName) - { - try - { - using (var key = Registry.LocalMachine.OpenSubKey(subKeyPath, false)) - { - if (key == null) return null; - object v = key.GetValue(valueName, null); - if (v == null) return null; - if (v is int i) return i; -+ if (v is long l) return (int)l; // Handle 64-bit to 32-bit conversion - if (v is byte[] b && b.Length >= 4) return BitConverter.ToInt32(b, 0); - return null; - } - } - catch { return null; } - } -``` - - -
___ -`[Auto-generated best practices - 2026-01-18]` +`[Auto-generated best practices - 2026-04-12]`