-
-
Notifications
You must be signed in to change notification settings - Fork 622
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
Changes from 5 commits
be630f1
e9a68d2
d0a274a
77f81cf
a8e0d65
1872755
4942eab
73cc5a9
0c51c41
ffc9b81
f2f14b1
09d310e
f6ca3c8
74c18d8
9017ce6
5161a83
a9be19b
fe23bfe
4ba16ae
80d4e30
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,73 @@ | ||
| 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 settingsField = typeof(Main).GetField("Settings", BindingFlags.NonPublic | BindingFlags.Static); | ||
| settingsField?.SetValue(null, new Settings()); | ||
|
Jack251970 marked this conversation as resolved.
Outdated
|
||
|
|
||
| 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.
|
||
| [TestCase("::1")] | ||
|
VictoriousRaptor marked this conversation as resolved.
|
||
| [TestCase("HTTP://EXAMPLE.COM")] | ||
| [TestCase("HTTPS://EXAMPLE.COM")] | ||
| [TestCase("EXAMPLE.COM")] | ||
| [TestCase("LOCALHOST")] | ||
|
DavidGBrett marked this conversation as resolved.
|
||
| public void WhenValidUrlThenIsUrlReturnsTrue(string url) | ||
| { | ||
| Assert.That(plugin.IsURL(url), Is.True); | ||
| } | ||
|
DavidGBrett marked this conversation as resolved.
VictoriousRaptor marked this conversation as resolved.
|
||
|
|
||
| ClassicAssert.IsFalse(plugin.IsURL("wwww")); | ||
| ClassicAssert.IsFalse(plugin.IsURL("wwww.c")); | ||
| ClassicAssert.IsFalse(plugin.IsURL("wwww.c")); | ||
| [TestCase("wwww")] | ||
| [TestCase("wwww.c")] | ||
| [TestCase("not a url")] | ||
| [TestCase("just text")] | ||
| [TestCase("http://")] | ||
| [TestCase("://example.com")] | ||
| [TestCase("0.0.0.0")] // Pattern excludes 0.0.0.0 | ||
|
Jack251970 marked this conversation as resolved.
Outdated
|
||
| [TestCase("256.1.1.1")] // Invalid IPv4 | ||
| [TestCase("example")] // No TLD | ||
| [TestCase(".com")] | ||
| [TestCase("http://.com")] | ||
| public void WhenInvalidUrlThenIsUrlReturnsFalse(string url) | ||
| { | ||
| Assert.That(plugin.IsURL(url), Is.False); | ||
| } | ||
|
DavidGBrett marked this conversation as resolved.
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| using System; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Text.RegularExpressions; | ||
| using System.Windows.Controls; | ||
| using Flow.Launcher.Plugin.SharedCommands; | ||
|
|
@@ -15,19 +16,28 @@ public class Main : IPlugin, IPluginI18n, ISettingProvider | |
| // 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]))" + | ||
| // IPv6 address with optional brackets (brackets required if followed by port) | ||
| // IPv6 with brackets | ||
| "(?:\\[(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}\\]|" + // standard IPv6 | ||
| "\\[(?:[0-9a-fA-F]{1,4}:){1,7}:\\]|" + // IPv6 with trailing :: | ||
| "\\[(?:[0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}\\]|" + // IPv6 compressed | ||
| "\\[::(?:[0-9a-fA-F]{1,4}:){0,6}[0-9a-fA-F]{1,4}\\]|" + // IPv6 with leading :: | ||
| "\\[(?:(?:[0-9a-fA-F]{1,4}:){1,6}|:):(?:[0-9a-fA-F]{1,4}:){0,5}[0-9a-fA-F]{1,4}\\]|" + // IPv6 with :: in the middle | ||
| "\\[::1\\])" + // IPv6 loopback | ||
| "|" + | ||
|
VictoriousRaptor marked this conversation as resolved.
Outdated
|
||
| // IPv6 without brackets (only when no port follows) | ||
| "(?:(?:[0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}|" + // standard IPv6 | ||
| "(?:[0-9a-fA-F]{1,4}:){1,7}:|" + // IPv6 with trailing :: | ||
| "(?:[0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|" + // IPv6 compressed | ||
| "::(?:[0-9a-fA-F]{1,4}:){0,6}[0-9a-fA-F]{1,4}|" + // IPv6 with leading :: | ||
| "(?:(?:[0-9a-fA-F]{1,4}:){1,6}|:):(?:[0-9a-fA-F]{1,4}:){0,5}[0-9a-fA-F]{1,4}|" + // IPv6 with :: in the middle | ||
| "::1)(?!:[0-9])" + // IPv6 loopback (not followed by port) | ||
|
Jack251970 marked this conversation as resolved.
Outdated
|
||
| "|" + | ||
| // IPv4 address - all valid addresses including private networks (excluding 0.0.0.0) | ||
| "(?:(?:25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]\\d|[1-9])\\.(?:25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)\\.(?:25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d)\\.(?:25[0-5]|2[0-4]\\d|1\\d\\d|[1-9]?\\d))" + | ||
| "|" + | ||
| // localhost | ||
| "localhost" + | ||
| "|" + | ||
| // host name | ||
| "(?:(?:[a-z\\u00a1-\\uffff0-9]-*)*[a-z\\u00a1-\\uffff0-9]+)" + | ||
|
|
@@ -37,20 +47,25 @@ public class Main : IPlugin, IPluginI18n, ISettingProvider | |
| "(?:\\.(?:[a-z\\u00a1-\\uffff]{2,}))" + | ||
| ")" + | ||
| // port number | ||
| "(?::\\d{2,5})?" + | ||
| "(?::\\d{1,5})?" + | ||
|
Jack251970 marked this conversation as resolved.
Outdated
|
||
| // 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://"]; | ||
|
Contributor
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. As far as I can tell browsers no longer support the ftp protocol for security reasons
Probably worth removing support for that in this case, though this may be beyond the scope of this PR Beyond that it might be worth supporting more scheme types that work for browsers Of note is internal addresses like Beyond those internal addresses we also have ones like Though supporting these could also be considered out of scope and done in a subsequent PR |
||
|
|
||
| public List<Result> Query(Query query) | ||
| { | ||
| var raw = query.Search; | ||
| if (IsURL(raw)) | ||
| if (!IsURL(raw)) | ||
| { | ||
| return | ||
| return []; | ||
| } | ||
|
VictoriousRaptor marked this conversation as resolved.
|
||
|
|
||
| return | ||
| [ | ||
| new() | ||
| { | ||
|
|
@@ -60,7 +75,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.
Outdated
|
||
| { | ||
| raw = GetHttpPreference() + "://" + raw; | ||
|
VictoriousRaptor marked this conversation as resolved.
Outdated
|
||
| } | ||
|
||
|
|
@@ -92,9 +108,6 @@ public List<Result> Query(Query query) | |
| } | ||
| } | ||
| ]; | ||
| } | ||
|
|
||
| return []; | ||
| } | ||
|
|
||
| private static string GetHttpPreference() | ||
|
|
@@ -108,14 +121,6 @@ public bool IsURL(string raw) | |
|
|
||
| if (UrlRegex.Match(raw).Value == raw) return true; | ||
|
|
||
| if (raw == "localhost" || raw.StartsWith("localhost:") || | ||
| raw == "http://localhost" || raw.StartsWith("http://localhost:") || | ||
| raw == "https://localhost" || raw.StartsWith("https://localhost:") | ||
| ) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.