From f797565388c582cce7d53651534418f8fead2569 Mon Sep 17 00:00:00 2001 From: Dylan Arbour Date: Fri, 8 May 2026 18:28:15 -0400 Subject: [PATCH] Warn when config contains malformed URL Prints a warning to stderr when a user Provider contains a malformed and non-functional URL. Adds tests for DefaultProviders. All errors and warnings are now piped to stderr over stdout. --- main.go | 6 +++--- open/provider.go | 12 +++++++++++- open/provider_test.go | 37 ++++++++++++++++++++++++++++++++----- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/main.go b/main.go index 7929dcf..01c2451 100644 --- a/main.go +++ b/main.go @@ -10,20 +10,20 @@ import ( func main() { arg, err := processArgs(os.Args) if err != nil { - fmt.Printf("error: \"%s\"\n", err) + fmt.Fprintf(os.Stderr, "error: \"%s\"\n", err) os.Exit(1) } url, err := open.GetURL(arg) if err != nil { - fmt.Printf("error: \"%s\"\n", err) + fmt.Fprintf(os.Stderr, "error: \"%s\"\n", err) os.Exit(1) } fmt.Printf("Opening %s in your browser.\n", url) err = open.InBrowser(url) if err != nil { - fmt.Printf("error: unable to open in browser: \"%s\"\n", err) + fmt.Fprintf(os.Stderr, "error: unable to open in browser: \"%s\"\n", err) os.Exit(1) } } diff --git a/open/provider.go b/open/provider.go index 1ac4726..96b7de5 100644 --- a/open/provider.go +++ b/open/provider.go @@ -1,7 +1,9 @@ package open import ( + "fmt" "net/url" + "os" "strings" "github.com/ldez/go-git-cmd-wrapper/v2/config" @@ -71,6 +73,7 @@ func LoadProviders() []Provider { return p } + var order []string urls := make(map[string]*Provider) for line := range strings.SplitSeq(out, "\n") { s := strings.SplitN(line, " ", 2) @@ -95,6 +98,7 @@ func LoadProviders() []Provider { if entry == nil { entry = &Provider{} urls[rawURL] = entry + order = append(order, rawURL) } switch key { @@ -105,7 +109,13 @@ func LoadProviders() []Provider { } } - for k, v := range urls { + for _, k := range order { + u, err := url.Parse(k) + if err != nil || u.Host == "" || (u.Scheme != "http" && u.Scheme != "https") { + fmt.Fprintf(os.Stderr, "warning: invalid provider URL in git config: %q\n", k) + continue + } + v := urls[k] v.BaseURL = k p = append(p, *v) } diff --git a/open/provider_test.go b/open/provider_test.go index 0857872..b02a628 100644 --- a/open/provider_test.go +++ b/open/provider_test.go @@ -1,6 +1,7 @@ package open import ( + "net/url" "os" "path/filepath" "runtime" @@ -15,6 +16,19 @@ import ( const repo = "arbourd/git-open" +func TestDefaultProviders(t *testing.T) { + for _, p := range DefaultProviders { + u, err := url.Parse(p.BaseURL) + if err != nil { + t.Errorf("provider %q: invalid BaseURL: %v", p.BaseURL, err) + continue + } + if u.Host == "" { + t.Errorf("provider %q: BaseURL has no host", p.BaseURL) + } + } +} + func TestCommitURL(t *testing.T) { cases := map[string]struct { p Provider @@ -208,21 +222,34 @@ func TestLoadProviders(t *testing.T) { {BaseURL: "https://git.internal.corp.com", CommitPrefix: "commit", PathPrefix: "tree"}, }, }, + "invalid url": { + config: []string{ + "open.not-a-url.commitprefix commit", + "open.not-a-url.pathprefix tree", + }, + expectedProviders: []Provider{}, + }, + "non-web scheme": { + config: []string{ + "open.ssh://git.example.dev.commitprefix commit", + "open.ssh://git.example.dev.pathprefix tree", + }, + expectedProviders: []Provider{}, + }, } for name, c := range cases { t.Run(name, func(t *testing.T) { // removes all `open.https://` Git config entries out, _ := git.Config(config.Global, config.GetRegexp(getRegex, "")) - for _, v := range strings.Split(strings.TrimSpace(out), "\n") { - key := strings.Split(strings.TrimSpace(v), " ")[0] - + for v := range strings.SplitSeq(strings.TrimSpace(out), "\n") { + key, _, _ := strings.Cut(strings.TrimSpace(v), " ") git.Config(config.Global, config.Unset(key, "")) } for _, v := range c.config { - s := strings.Split(v, " ") - git.Config(config.Global, config.Entry(s[0], s[1])) + key, val, _ := strings.Cut(v, " ") + git.Config(config.Global, config.Entry(key, val)) } p := LoadProviders()