-
-
Notifications
You must be signed in to change notification settings - Fork 563
Fixing matching issues in URL plugin #4191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
be630f1
e9a68d2
d0a274a
77f81cf
a8e0d65
1872755
4942eab
73cc5a9
0c51c41
ffc9b81
f2f14b1
09d310e
f6ca3c8
74c18d8
9017ce6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,33 +1,84 @@ | ||||||||||||||
| using NUnit.Framework; | ||||||||||||||
| using NUnit.Framework.Legacy; | ||||||||||||||
| using Flow.Launcher.Plugin.Url; | ||||||||||||||
| using System.Reflection; | ||||||||||||||
|
|
||||||||||||||
| namespace Flow.Launcher.Test.Plugins | ||||||||||||||
| { | ||||||||||||||
| [TestFixture] | ||||||||||||||
| public class UrlPluginTest | ||||||||||||||
| { | ||||||||||||||
| [Test] | ||||||||||||||
| public void URLMatchTest() | ||||||||||||||
| private static Main plugin; | ||||||||||||||
|
|
||||||||||||||
| [OneTimeSetUp] | ||||||||||||||
| public void OneTimeSetup() | ||||||||||||||
| { | ||||||||||||||
| var plugin = new Main(); | ||||||||||||||
| ClassicAssert.IsTrue(plugin.IsURL("http://www.google.com")); | ||||||||||||||
| ClassicAssert.IsTrue(plugin.IsURL("https://www.google.com")); | ||||||||||||||
| ClassicAssert.IsTrue(plugin.IsURL("http://google.com")); | ||||||||||||||
| ClassicAssert.IsTrue(plugin.IsURL("www.google.com")); | ||||||||||||||
| ClassicAssert.IsTrue(plugin.IsURL("google.com")); | ||||||||||||||
| ClassicAssert.IsTrue(plugin.IsURL("http://localhost")); | ||||||||||||||
| ClassicAssert.IsTrue(plugin.IsURL("https://localhost")); | ||||||||||||||
| ClassicAssert.IsTrue(plugin.IsURL("http://localhost:80")); | ||||||||||||||
| ClassicAssert.IsTrue(plugin.IsURL("https://localhost:80")); | ||||||||||||||
| ClassicAssert.IsTrue(plugin.IsURL("http://110.10.10.10")); | ||||||||||||||
| ClassicAssert.IsTrue(plugin.IsURL("110.10.10.10")); | ||||||||||||||
| ClassicAssert.IsTrue(plugin.IsURL("ftp://110.10.10.10")); | ||||||||||||||
| var settingsProperty = typeof(Main).GetProperty("Settings", BindingFlags.NonPublic | BindingFlags.Static); | ||||||||||||||
| settingsProperty?.SetValue(null, new Settings()); | ||||||||||||||
|
|
||||||||||||||
| plugin = new Main(); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| [TestCase("http://www.google.com")] | ||||||||||||||
| [TestCase("https://www.google.com")] | ||||||||||||||
| [TestCase("http://google.com")] | ||||||||||||||
| [TestCase("ftp://google.com")] | ||||||||||||||
| [TestCase("www.google.com")] | ||||||||||||||
| [TestCase("google.com")] | ||||||||||||||
|
Check warning on line 26 in Flow.Launcher.Test/Plugins/UrlPluginTest.cs
|
||||||||||||||
| [TestCase("http://localhost")] | ||||||||||||||
| [TestCase("https://localhost")] | ||||||||||||||
| [TestCase("http://localhost:80")] | ||||||||||||||
| [TestCase("https://localhost:80")] | ||||||||||||||
| [TestCase("localhost")] | ||||||||||||||
| [TestCase("localhost:8080")] | ||||||||||||||
| [TestCase("http://110.10.10.10")] | ||||||||||||||
| [TestCase("110.10.10.10")] | ||||||||||||||
| [TestCase("110.10.10.10:8080")] | ||||||||||||||
| [TestCase("192.168.1.1")] | ||||||||||||||
| [TestCase("192.168.1.1:3000")] | ||||||||||||||
| [TestCase("ftp://110.10.10.10")] | ||||||||||||||
| [TestCase("[2001:db8::1]")] | ||||||||||||||
| [TestCase("[2001:db8::1]:8080")] | ||||||||||||||
| [TestCase("http://[2001:db8::1]")] | ||||||||||||||
| [TestCase("https://[2001:db8::1]:8080")] | ||||||||||||||
| [TestCase("[::1]")] | ||||||||||||||
| [TestCase("[::1]:8080")] | ||||||||||||||
| [TestCase("2001:db8::1")] | ||||||||||||||
| [TestCase("fe80:1:2::3:4")] | ||||||||||||||
VictoriousRaptor marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
| [TestCase("::1")] | ||||||||||||||
VictoriousRaptor marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
| [TestCase("HTTP://EXAMPLE.COM")] | ||||||||||||||
| [TestCase("HTTPS://EXAMPLE.COM")] | ||||||||||||||
| [TestCase("EXAMPLE.COM")] | ||||||||||||||
| [TestCase("LOCALHOST")] | ||||||||||||||
|
||||||||||||||
| [TestCase("LOCALHOST")] | |
| [TestCase("LOCALHOST")] | |
| [TestCase("Http://Example.Com")] | |
| [TestCase("hTTps://ExAmPlE.CoM")] | |
| [TestCase("Example.Com")] | |
| [TestCase("LocalHost")] |
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test cases should include scenarios with query parameters and fragments to ensure the URL validation handles them correctly. For example, add test cases like "192.168.1.1?query=value", "localhost:8080?test=123", "example.com#fragment", etc. These are common URL patterns that should be supported.
VictoriousRaptor marked this conversation as resolved.
Show resolved
Hide resolved
Jack251970 marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test cases for invalid URLs should include more edge cases to ensure robustness. Consider adding tests for: URLs with spaces (e.g., "example .com"), URLs with invalid characters, extremely long TLDs, domains starting with dots (e.g., "..example.com"), multiple consecutive dots (e.g., "example..com"), and malformed IPv6 addresses (e.g., "2001:db8:::1" with three colons).
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,56 +1,37 @@ | ||
| using System; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Text.RegularExpressions; | ||
| using System.Linq; | ||
| using System.Net; | ||
| using System.Windows.Controls; | ||
| using Flow.Launcher.Plugin.SharedCommands; | ||
|
|
||
| namespace Flow.Launcher.Plugin.Url | ||
| { | ||
| public class Main : IPlugin, IPluginI18n, ISettingProvider | ||
| { | ||
| //based on https://gist.github.com/dperini/729294 | ||
| private const string UrlPattern = "^" + | ||
| // protocol identifier | ||
| "(?:(?:https?|ftp)://|)" + | ||
| // user:pass authentication | ||
| "(?:\\S+(?::\\S*)?@)?" + | ||
| "(?:" + | ||
| // IP address exclusion | ||
| // private & local networks | ||
| "(?!(?:10|127)(?:\\.\\d{1,3}){3})" + | ||
| "(?!(?:169\\.254|192\\.168)(?:\\.\\d{1,3}){2})" + | ||
| "(?!172\\.(?:1[6-9]|2\\d|3[0-1])(?:\\.\\d{1,3}){2})" + | ||
| // IP address dotted notation octets | ||
| // excludes loopback network 0.0.0.0 | ||
| // excludes reserved space >= 224.0.0.0 | ||
| // excludes network & broacast addresses | ||
| // (first & last IP address of each class) | ||
| "(?:[1-9]\\d?|1\\d\\d|2[01]\\d|22[0-3])" + | ||
| "(?:\\.(?:1?\\d{1,2}|2[0-4]\\d|25[0-5])){2}" + | ||
| "(?:\\.(?:[1-9]\\d?|1\\d\\d|2[0-4]\\d|25[0-4]))" + | ||
| "|" + | ||
| // host name | ||
| "(?:(?:[a-z\\u00a1-\\uffff0-9]-*)*[a-z\\u00a1-\\uffff0-9]+)" + | ||
| // domain name | ||
| "(?:\\.(?:[a-z\\u00a1-\\uffff0-9]-*)*[a-z\\u00a1-\\uffff0-9]+)*" + | ||
| // TLD identifier | ||
| "(?:\\.(?:[a-z\\u00a1-\\uffff]{2,}))" + | ||
| ")" + | ||
| // port number | ||
| "(?::\\d{2,5})?" + | ||
| // resource path | ||
| "(?:/\\S*)?" + | ||
| "$"; | ||
| private readonly Regex UrlRegex = new(UrlPattern, RegexOptions.Compiled | RegexOptions.IgnoreCase); | ||
| internal static PluginInitContext Context { get; private set; } | ||
| internal static Settings Settings { get; private set; } | ||
|
|
||
| private static readonly string[] UrlSchemes = ["http://", "https://", "ftp://"]; | ||
|
|
||
| public List<Result> Query(Query query) | ||
| { | ||
| var raw = query.Search; | ||
| if (IsURL(raw)) | ||
| if (!IsURL(raw)) | ||
| { | ||
| return | ||
| return []; | ||
| } | ||
VictoriousRaptor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (IPEndPoint.TryParse(raw, out var endpoint)) | ||
| { | ||
| if (endpoint.AddressFamily == System.Net.Sockets.AddressFamily.InterNetworkV6 && raw[0] != '[' && raw[^1] != ']') | ||
| { | ||
| // Enclose IPv6 addresses in brackets for URL formatting | ||
| raw = $"[{raw}]"; | ||
| } | ||
cubic-dev-ai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| return | ||
| [ | ||
| new() | ||
| { | ||
|
|
@@ -60,7 +41,8 @@ public List<Result> Query(Query query) | |
| Score = 8, | ||
| Action = _ => | ||
| { | ||
| if (!raw.StartsWith("http://", StringComparison.OrdinalIgnoreCase) && !raw.StartsWith("https://", StringComparison.OrdinalIgnoreCase)) | ||
| // not a recognized scheme, add preferred http scheme | ||
| if (!UrlSchemes.Any(scheme => raw.StartsWith(scheme, StringComparison.OrdinalIgnoreCase))) | ||
Jack251970 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| raw = GetHttpPreference() + "://" + raw; | ||
VictoriousRaptor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
Comment on lines
+44
to
48
|
||
|
|
@@ -92,9 +74,6 @@ public List<Result> Query(Query query) | |
| } | ||
| } | ||
| ]; | ||
| } | ||
|
|
||
| return []; | ||
| } | ||
|
|
||
| private static string GetHttpPreference() | ||
|
|
@@ -104,19 +83,68 @@ private static string GetHttpPreference() | |
|
|
||
| public bool IsURL(string raw) | ||
| { | ||
| raw = raw.ToLower(); | ||
| if (string.IsNullOrWhiteSpace(raw)) | ||
| return false; | ||
|
|
||
| if (UrlRegex.Match(raw).Value == raw) return true; | ||
| var input = raw.Trim(); | ||
|
|
||
| if (raw == "localhost" || raw.StartsWith("localhost:") || | ||
| raw == "http://localhost" || raw.StartsWith("http://localhost:") || | ||
| raw == "https://localhost" || raw.StartsWith("https://localhost:") | ||
| ) | ||
| // Exclude numbers (e.g. 1.2345) | ||
| if (decimal.TryParse(input, System.Globalization.NumberStyles.Any, System.Globalization.CultureInfo.InvariantCulture, out _)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Using InvariantCulture here ignores locale-specific decimal separators, so numeric inputs in comma-based cultures can slip through as URLs. Use the current culture to keep the numeric exclusion consistent with user input. Prompt for AI agents
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @VictoriousRaptor this is not worth considering?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No. It's funny that AI told me to use invariant culture in previous comments. Use invariant culture can stop parsing when user is typing a number with dot. Users in culture that uses comma as decimal separator won't have this issue. |
||
| return false; | ||
VictoriousRaptor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // Check if it's a bare IP address with optional port, path, query, or fragment | ||
| var ipPart = input.Split('/', '?', '#')[0]; // Remove path, query, and fragment | ||
| if (IPEndPoint.TryParse(ipPart, out var endpoint)) | ||
| { | ||
| switch (endpoint.AddressFamily) | ||
| { | ||
| case System.Net.Sockets.AddressFamily.InterNetwork: | ||
| return !endpoint.Address.Equals(IPAddress.Any); | ||
| case System.Net.Sockets.AddressFamily.InterNetworkV6: | ||
| if (input.Contains('/') || input.Contains('?') || input.Contains('#')) | ||
| { | ||
| // Check if IPv6 address is properly bracketed | ||
| var bracketStart = input.IndexOf('['); | ||
| var bracketEnd = input.IndexOf(']'); | ||
| if (bracketStart == -1 || bracketEnd == -1 || bracketStart > bracketEnd) | ||
| return false; | ||
| } | ||
| return !endpoint.Address.Equals(IPAddress.IPv6Any); | ||
| } | ||
| return true; | ||
VictoriousRaptor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| return false; | ||
| // Add protocol if missing for Uri validation | ||
| var urlToValidate = UrlSchemes.Any(s => input.StartsWith(s, StringComparison.OrdinalIgnoreCase)) | ||
| ? input | ||
| : GetHttpPreference() + "://" + input; | ||
|
|
||
| if (!Uri.TryCreate(urlToValidate, UriKind.Absolute, out var uri)) | ||
| return false; | ||
|
|
||
|
|
||
| // Validate protocol | ||
| if (uri.Scheme != Uri.UriSchemeHttp && uri.Scheme != Uri.UriSchemeHttps && uri.Scheme != Uri.UriSchemeFtp) | ||
| return false; | ||
|
|
||
| var host = uri.Host; | ||
|
|
||
| // localhost is valid | ||
| if (host.Equals("localhost", StringComparison.OrdinalIgnoreCase)) | ||
| return true; | ||
|
|
||
| // Valid IP address (excluding 0.0.0.0) | ||
| if (IPEndPoint.TryParse(host, out endpoint)) | ||
| return !endpoint.Address.Equals(IPAddress.Any) && !endpoint.Address.Equals(IPAddress.IPv6Any); | ||
|
|
||
| // Domain must have valid format with TLD | ||
| var parts = host.Split('.'); | ||
| if (parts.Length < 2 || parts.Any(string.IsNullOrEmpty)) | ||
| return false; | ||
|
|
||
| // TLD must be at least 2 characters, allowing letters and digits | ||
| var tld = parts[^1]; | ||
| return tld.Length >= 2 && tld.All(char.IsLetterOrDigit); | ||
| } | ||
|
|
||
| public void Init(PluginInitContext context) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.