| PR 245 (2025-05-05) |
[possible issue] Prevent substring exception
✅ 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]
private async Task<bool> TryServeFromCdn(HttpListenerContext context, string path)
{
bool isNodePackageExpression = Regex.IsMatch(path, @"^[^/@]+@[^/]+/");
var sources = new (bool isMatch, string configKey, Func<string, string> 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 importance[1-10]: 7
__
Why: The suggestion correctly identifies a potential ArgumentOutOfRangeException if the path variable is exactly "jquery/". Adding the length check (&& path.Length > "jquery/".Length) prevents this runtime error, improving the code's robustness.
[general] Improve package detection regex
✅ Improve package detection regex
The regex pattern for detecting node package expressions might not handle all valid package formats. Consider using a more comprehensive pattern that accounts for scoped packages (starting with @) and various version formats.
WelsonJS.Toolkit/WelsonJS.Launcher/ResourceServer.cs [198]
-bool isNodePackageExpression = Regex.IsMatch(path, @"^[^/@]+@[^/]+/");
+bool isNodePackageExpression = Regex.IsMatch(path, @"^(@[a-z0-9-]+\/[a-z0-9-]+|[a-z0-9-]+)@([0-9]+\.[0-9]+\.[0-9]+|[0-9]+\.[0-9]+|[0-9]+|latest|next|[^/]+)/");
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly points out that the original regex @"^[^/@]+@[^/]+/" is too simple and might not match all valid node package formats (e.g., scoped packages like @scope/package@version/). The improved regex aims to be more comprehensive, enhancing the accuracy of the CDN routing logic.
| PR 242 (2025-04-27) |
[security] Add missing crossorigin attribute
✅ 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]
-<script src="http://localhost:3000/ajax/libs/lodash/4.17.21/lodash.min.js" integrity="sha384-H6KKS1H1WwuERMSm+54dYLzjg0fKqRK5ZRyASdbrI/lwrCc6bXEmtGYr5SwvP1pZ"></script>
+<script src="http://localhost:3000/ajax/libs/lodash/4.17.21/lodash.min.js" integrity="sha384-H6KKS1H1WwuERMSm+54dYLzjg0fKqRK5ZRyASdbrI/lwrCc6bXEmtGYr5SwvP1pZ" crossorigin="anonymous"></script>
Suggestion importance[1-10]: 8
__
Why: This is a valid security enhancement. The integrity attribute is present but without the crossorigin attribute, Subresource Integrity (SRI) checks won't work properly. Adding this attribute improves security for the external script.
[possible issue] Add null check
✅ 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]
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 importance[1-10]: 7
__
Why: This is a good defensive programming practice that prevents potential runtime errors if promptEditorRef.current is null or undefined. The check adds robustness to the code and prevents potential crashes.