diff --git a/v2/internal/frontend/desktop/darwin/browser.go b/v2/internal/frontend/desktop/darwin/browser.go index c865ab6d9..2c9aaa94e 100644 --- a/v2/internal/frontend/desktop/darwin/browser.go +++ b/v2/internal/frontend/desktop/darwin/browser.go @@ -5,20 +5,20 @@ package darwin import ( "fmt" + "net/url" + "github.com/pkg/browser" - "github.com/wailsapp/wails/v2/internal/frontend/utils" ) // BrowserOpenURL Use the default browser to open the url func (f *Frontend) BrowserOpenURL(rawURL string) { - url, err := utils.ValidateAndSanitizeURL(rawURL) + parsed, err := url.Parse(rawURL) if err != nil { - f.logger.Error(fmt.Sprintf("Invalid URL %s", err.Error())) + f.logger.Error(fmt.Sprintf("BrowserOpenURL cannot parse url: %s", err.Error())) return } - // Specific method implementation - if err := browser.OpenURL(url); err != nil { + if err = browser.OpenURL(parsed.String()); err != nil { f.logger.Error("Unable to open default system browser") } } diff --git a/v2/internal/frontend/desktop/linux/browser.go b/v2/internal/frontend/desktop/linux/browser.go index 962e3b28a..357657b3f 100644 --- a/v2/internal/frontend/desktop/linux/browser.go +++ b/v2/internal/frontend/desktop/linux/browser.go @@ -5,19 +5,20 @@ package linux import ( "fmt" + "net/url" + "github.com/pkg/browser" - "github.com/wailsapp/wails/v2/internal/frontend/utils" ) // BrowserOpenURL Use the default browser to open the url func (f *Frontend) BrowserOpenURL(rawURL string) { - url, err := utils.ValidateAndSanitizeURL(rawURL) + parsed, err := url.Parse(rawURL) if err != nil { - f.logger.Error(fmt.Sprintf("Invalid URL %s", err.Error())) + f.logger.Error(fmt.Sprintf("BrowserOpenURL cannot parse url: %s", err.Error())) return } // Specific method implementation - if err := browser.OpenURL(url); err != nil { + if err := browser.OpenURL(parsed.String()); err != nil { f.logger.Error("Unable to open default system browser") } } diff --git a/v2/internal/frontend/desktop/windows/browser.go b/v2/internal/frontend/desktop/windows/browser.go index 13d037b14..24f7d28a8 100644 --- a/v2/internal/frontend/desktop/windows/browser.go +++ b/v2/internal/frontend/desktop/windows/browser.go @@ -5,8 +5,9 @@ package windows import ( "fmt" + "net/url" + "github.com/pkg/browser" - "github.com/wailsapp/wails/v2/internal/frontend/utils" "golang.org/x/sys/windows" ) @@ -19,19 +20,19 @@ var fallbackBrowserPaths = []string{ // BrowserOpenURL Use the default browser to open the url func (f *Frontend) BrowserOpenURL(rawURL string) { - url, err := utils.ValidateAndSanitizeURL(rawURL) + parsed, err := url.Parse(rawURL) if err != nil { - f.logger.Error(fmt.Sprintf("Invalid URL %s", err.Error())) + f.logger.Error(fmt.Sprintf("BrowserOpenURL cannot parse url: %s", err.Error())) return } // Specific method implementation - err = browser.OpenURL(url) + err = browser.OpenURL(parsed.String()) if err == nil { return } for _, fallback := range fallbackBrowserPaths { - if err := openBrowser(fallback, url); err == nil { + if err := openBrowser(fallback, parsed.String()); err == nil { return } } diff --git a/v2/internal/frontend/utils/urlValidator.go b/v2/internal/frontend/utils/urlValidator.go deleted file mode 100644 index 76ba216ce..000000000 --- a/v2/internal/frontend/utils/urlValidator.go +++ /dev/null @@ -1,58 +0,0 @@ -package utils - -import ( - "errors" - "fmt" - "net/url" - "regexp" - "strings" -) - -func ValidateAndSanitizeURL(rawURL string) (string, error) { - // Check for null bytes (can cause truncation issues in some systems) - if strings.Contains(rawURL, "\x00") { - return "", errors.New("null bytes not allowed in URL") - } - - // Parse URL first - this handles most malformed URLs - parsedURL, err := url.Parse(rawURL) - if err != nil { - return "", fmt.Errorf("invalid URL format: %v", err) - } - - scheme := strings.ToLower(parsedURL.Scheme) - - if scheme == "javascript" || scheme == "data" || scheme == "file" || scheme == "ftp" || scheme == "" { - return "", errors.New("scheme not allowed") - } - - // Ensure there's actually a host for http/https URLs - if (scheme == "http" || scheme == "https") && parsedURL.Host == "" { - return "", fmt.Errorf("missing host for %s URL", scheme) - } - - sanitizedURL := parsedURL.String() - - // Check for control characters that might cause issues - // (but allow legitimate URL characters like &, ;, etc.) - for i, r := range sanitizedURL { - // Block control characters except tab, but allow other printable chars - if r < 32 && r != 9 { // 9 is tab, which might be legitimate - return "", fmt.Errorf("control character at position %d not allowed", i) - } - } - - // Shell metacharacter check - shellDangerous := `[;\|` + "`" + `$\\<>*{}\[\]()~! \t\n\r]` - if matched, _ := regexp.MatchString(shellDangerous, sanitizedURL); matched { - return "", errors.New("shell metacharacters not allowed") - } - - // Unicode danger check - unicodeDangerous := "[\u0000-\u001F\u007F\u00A0\u1680\u2000-\u200F\u2028-\u202F\u205F\u2060\u3000\uFEFF]" - if matched, _ := regexp.MatchString(unicodeDangerous, sanitizedURL); matched { - return "", errors.New("unicode dangerous characters not allowed") - } - - return sanitizedURL, nil -} diff --git a/v2/internal/frontend/utils/urlValidator_test.go b/v2/internal/frontend/utils/urlValidator_test.go deleted file mode 100644 index b385ccec1..000000000 --- a/v2/internal/frontend/utils/urlValidator_test.go +++ /dev/null @@ -1,207 +0,0 @@ -package utils_test - -import ( - "strings" - "testing" - - "github.com/wailsapp/wails/v2/internal/frontend/utils" -) - -// Test cases for ValidateAndOpenURL -func TestValidateURL(t *testing.T) { - testCases := []struct { - name string - url string - shouldErr bool - errMsg string - expected string - }{ - // Valid URLs - { - name: "valid https URL", - url: "https://www.example.com", - shouldErr: false, - expected: "https://www.example.com", - }, - { - name: "valid http URL", - url: "http://example.com", - shouldErr: false, - expected: "http://example.com", - }, - { - name: "URL with query parameters", - url: "https://example.com/search?q=cats&dogs", - shouldErr: false, - expected: "https://example.com/search?q=cats&dogs", - }, - { - name: "URL with port", - url: "https://example.com:8080/path", - shouldErr: false, - expected: "https://example.com:8080/path", - }, - { - name: "URL with fragment", - url: "https://example.com/page#section", - shouldErr: false, - expected: "https://example.com/page#section", - }, - { - name: "urlencode params", - url: "http://google.com/ ----browser-subprocess-path=C:\\\\Users\\\\Public\\\\test.bat", - shouldErr: false, - expected: "http://google.com/%20----browser-subprocess-path=C:%5C%5CUsers%5C%5CPublic%5C%5Ctest.bat", - }, - - // Invalid schemes - { - name: "javascript scheme", - url: "javascript:alert('xss')", - shouldErr: true, - errMsg: "scheme not allowed", - }, - { - name: "data scheme", - url: "data:text/html,", - shouldErr: true, - errMsg: "scheme not allowed", - }, - { - name: "file scheme", - url: "file:///etc/passwd", - shouldErr: true, - errMsg: "scheme not allowed", - }, - { - name: "ftp scheme", - url: "ftp://files.example.com/file.txt", - shouldErr: true, - errMsg: "scheme not allowed", - }, - - // Malformed URLs - { - name: "not a URL", - url: "not-a-url", - shouldErr: true, - errMsg: "scheme not allowed", // will have empty scheme - }, - { - name: "missing scheme", - url: "example.com", - shouldErr: true, - errMsg: "scheme not allowed", - }, - { - name: "malformed URL", - url: "https://", - shouldErr: true, - errMsg: "missing host", - }, - { - name: "empty host", - url: "http:///path", - shouldErr: true, - errMsg: "missing host", - }, - - // Security issues - { - name: "null byte in URL", - url: "https://example.com\x00/hidden", - shouldErr: true, - errMsg: "null bytes not allowed", - }, - { - name: "control characters", - url: "https://example.com\n/path", - shouldErr: true, - errMsg: "control character", - }, - { - name: "carriage return", - url: "https://example.com\r/path", - shouldErr: true, - errMsg: "control character", - }, - { - name: "URL with tab character", - url: "https://example.com/path?q=hello\tworld", - shouldErr: true, - errMsg: "control character", - }, - { - name: "URL with path parameters", - url: "https://example.com/path;param=value", - shouldErr: true, - errMsg: "shell metacharacters not allowed", - }, - { - name: "URL with special characters in query", - url: "https://example.com/search?q=hello world&filter=price>100", - shouldErr: true, - errMsg: "shell metacharacters not allowed", - }, - { - name: "URL with special characters in query and params", - url: "https://example.com/search?q=hello ----browser-subprocess-path=C:\\\\Users\\\\Public\\\\test.bat", - shouldErr: true, - errMsg: "shell metacharacters not allowed", - }, - { - name: "URL with dollar sign in query", - url: "https://example.com/search?price=$100", - shouldErr: true, - errMsg: "shell metacharacters not allowed", - }, - { - name: "URL with parentheses", - url: "https://example.com/file(1).html", - shouldErr: true, - errMsg: "shell metacharacters not allowed", - }, - { - name: "URL with unicode", - url: "https://example.com/search?q=hello\u2001foo", - shouldErr: true, - errMsg: "unicode dangerous characters not allowed", - }, - - // Edge cases - { - name: "international domain", - url: "https://例え.テスト/path", - shouldErr: false, - expected: "https://%E4%BE%8B%E3%81%88.%E3%83%86%E3%82%B9%E3%83%88/path", - }, - { - name: "URL with pipe character", - url: "https://example.com/user/123|admin", - shouldErr: false, - expected: "https://example.com/user/123%7Cadmin", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - // We'll test only the validation part to avoid actually opening URLs - sanitized, err := utils.ValidateAndSanitizeURL(tc.url) - - if tc.shouldErr { - if err == nil { - t.Errorf("expected error for URL %q, but got none", tc.url) - } else if tc.errMsg != "" && !strings.Contains(err.Error(), tc.errMsg) { - t.Errorf("expected error containing %q, got %q", tc.errMsg, err.Error()) - } - } else { - if err != nil { - t.Errorf("expected no error for URL %q, but got: %v", tc.url, err) - } - if sanitized != tc.expected { - t.Errorf("unexpected sanitized URL for %q: expected %q, got %q", tc.url, tc.expected, sanitized) - } - } - }) - } -} diff --git a/v2/pkg/runtime/browser.go b/v2/pkg/runtime/browser.go index 28ffc5e05..4aa39ef9d 100644 --- a/v2/pkg/runtime/browser.go +++ b/v2/pkg/runtime/browser.go @@ -2,10 +2,19 @@ package runtime import ( "context" + "fmt" + "net/url" ) // BrowserOpenURL uses the system default browser to open the url -func BrowserOpenURL(ctx context.Context, url string) { +func BrowserOpenURL(ctx context.Context, rawURL string) { appFrontend := getFrontend(ctx) - appFrontend.BrowserOpenURL(url) + + parsed, err := url.Parse(rawURL) + if err != nil { + LogError(ctx, fmt.Sprintf("BrowserOpenURL cannot parse url: %s", err.Error())) + return + } + + appFrontend.BrowserOpenURL(parsed.String()) }