fix out of index error when install source is not standard github link #3054
Conversation
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
📝 WalkthroughWalkthroughThe changes in this pull request focus on the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PluginsManager
participant PluginSource
User->>PluginsManager: Request plugin update
PluginsManager->>PluginSource: Check local zip path
PluginSource-->>PluginsManager: Return plugin data
PluginsManager->>PluginsManager: Process plugin update
PluginsManager->>User: Confirm update completion
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs (1)
535-540: Good fix for the out-of-index error, consider additional URL validation
The changes properly handle the array bounds checking before accessing URL segments, fixing the original issue. However, consider adding more robust URL validation:
Consider these improvements:
private bool InstallSourceKnown(string url)
{
+ if (!Uri.TryCreate(url, UriKind.Absolute, out var uri))
+ return false;
+
var pieces = url.Split('/');
if (pieces.Length < 4)
return false;
var author = pieces[3];
+ // Handle encoded characters in the author segment
+ author = Uri.UnescapeDataString(author);
var acceptedSource = "https://github.com";
var constructedUrlPart = string.Format("{0}/{1}/", acceptedSource, author);
return url.StartsWith(acceptedSource) &&
Context.API.GetAllPlugins().Any(x => x.Metadata.Website.StartsWith(constructedUrlPart));
}📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs (5 hunks)
🔇 Additional comments (2)
Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs (2)
206-210: LGTM! Code readability improvement
The changes make the update source selection logic more concise and clearer to understand.
Line range hint 535-544: Verify similar URL validation patterns in the codebase
While the fix addresses the immediate issue, it would be good to verify other places where GitHub URLs are parsed.
✅ Verification successful
Based on the search results, I can see that GitHub URL handling is primarily used in the context of plugin management, updates, and documentation links. The specific URL validation pattern in the review (splitting and checking URL parts) is only used in the PluginsManager.cs file for validating plugin sources.
Let me verify if there are any other URL splitting patterns that might need similar validation:
No similar URL validation patterns require attention
The search results show that URL splitting and validation patterns are only used in two specific contexts:
PluginsManager.cs: For validating GitHub plugin sources (the code under review)Win32.cs: For resolving Windows shortcut paths with protocol handlers, which is a different use case
The current validation in PluginsManager.cs is sufficient as it's the only place where GitHub repository URLs need to be validated for plugin management.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for URL splitting patterns that might need similar validation
rg -l 'Split\(["/'']\)' | while read -r file; do
echo "=== $file ==="
rg -A 5 'Split\(["/'']\)' "$file"
done
# Search for GitHub URL handling
rg -l 'github\.com' | while read -r file; do
echo "=== $file ==="
rg -A 5 'github\.com' "$file"
done
Length of output: 76693
Script:
#!/bin/bash
# Search for URL splitting patterns in the codebase
ast-grep --pattern 'Split($_, $_)'
# Also search for URL validation patterns
rg -A 5 'url\.(StartsWith|Contains|EndsWith)'
Length of output: 1341
fix #3017