Extract web browser launching to a package

This fixes opening URLs with `&` on Windows.

auth/oauth.go

@@ -10,9 +10,8 @@ import (
 	"net/http"
 	"net/url"
 	"os"
-	"os/exec"
-	"runtime"
-	"strings"
+
+	"github.com/cli/cli/pkg/browser"
 )
 
 func randomString(length int) (string, error) {
@@ -123,20 +122,9 @@ func (oa *OAuthFlow) logf(format string, args ...interface{}) {
 }
 
 func openInBrowser(url string) error {
-	var args []string
-	switch runtime.GOOS {
-	case "darwin":
-		args = []string{"open"}
-	case "windows":
-		args = []string{"cmd", "/c", "start"}
-		r := strings.NewReplacer("&", "^&")
-		url = r.Replace(url)
-	default:
-		args = []string{"xdg-open"}
+	cmd, err := browser.Command(url)
+	if err != nil {
+		return err
 	}
-
-	args = append(args, url)
-	cmd := exec.Command(args[0], args[1:]...)
-	cmd.Stderr = os.Stderr
 	return cmd.Run()
 }

pkg/browser/browser.go

@@ -0,0 +1,52 @@
+package browser
+
+import (
+	"os"
+	"os/exec"
+	"runtime"
+	"strings"
+
+	"github.com/google/shlex"
+)
+
+// Command produces an exec.Cmd respecting runtime.GOOS and $BROWSER environment variable
+func Command(url string) (*exec.Cmd, error) {
+	launcher := os.Getenv("BROWSER")
+	if launcher != "" {
+		return FromLauncher(launcher, url)
+	}
+	return ForOS(runtime.GOOS, url), nil
+}
+
+// ForOS produces an exec.Cmd to open the web browser for different OS
+func ForOS(goos, url string) *exec.Cmd {
+	var args []string
+	switch goos {
+	case "darwin":
+		args = []string{"open"}
+	case "windows":
+		args = []string{"cmd", "/c", "start"}
+		r := strings.NewReplacer("&", "^&")
+		url = r.Replace(url)
+	default:
+		args = []string{"xdg-open"}
+	}
+
+	args = append(args, url)
+	cmd := exec.Command(args[0], args[1:]...)
+	cmd.Stderr = os.Stderr
+	return cmd
+}
+
+// FromLauncher parses the launcher string based on shell splitting rules
+func FromLauncher(launcher, url string) (*exec.Cmd, error) {
+	args, err := shlex.Split(launcher)
+	if err != nil {
+		return nil, err
+	}
+
+	args = append(args, url)
+	cmd := exec.Command(args[0], args[1:]...)
+	cmd.Stderr = os.Stderr
+	return cmd, nil
+}

pkg/browser/browser_test.go

@@ -0,0 +1,50 @@
+package browser
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestForOS(t *testing.T) {
+	type args struct {
+		goos string
+		url  string
+	}
+	tests := []struct {
+		name string
+		args args
+		want []string
+	}{
+		{
+			name: "macOS",
+			args: args{
+				goos: "darwin",
+				url:  "https://example.com/path?a=1&b=2",
+			},
+			want: []string{"open", "https://example.com/path?a=1&b=2"},
+		},
+		{
+			name: "Linux",
+			args: args{
+				goos: "linux",
+				url:  "https://example.com/path?a=1&b=2",
+			},
+			want: []string{"xdg-open", "https://example.com/path?a=1&b=2"},
+		},
+		{
+			name: "Windows",
+			args: args{
+				goos: "windows",
+				url:  "https://example.com/path?a=1&b=2&c=3",
+			},
+			want: []string{"cmd", "/c", "start", "https://example.com/path?a=1^&b=2^&c=3"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if cmd := ForOS(tt.args.goos, tt.args.url); !reflect.DeepEqual(cmd.Args, tt.want) {
+				t.Errorf("ForOS() = %v, want %v", cmd.Args, tt.want)
+			}
+		})
+	}
+}

utils/utils.go

@@ -2,60 +2,22 @@ package utils
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
-	"os"
-	"os/exec"
-	"runtime"
 	"time"
 
-	"github.com/kballard/go-shellquote"
+	"github.com/cli/cli/pkg/browser"
 	md "github.com/vilmibm/go-termd"
 )
 
+// OpenInBrowser opens the url in a web browser based on OS and $BROWSER environment variable
 func OpenInBrowser(url string) error {
-	browser := os.Getenv("BROWSER")
-	if browser == "" {
-		browser = searchBrowserLauncher(runtime.GOOS)
-	} else {
-		browser = os.ExpandEnv(browser)
-	}
-
-	if browser == "" {
-		return errors.New("Please set $BROWSER to a web launcher")
-	}
-
-	browserArgs, err := shellquote.Split(browser)
+	browseCmd, err := browser.Command(url)
 	if err != nil {
 		return err
 	}
-
-	endingArgs := append(browserArgs[1:], url)
-	browseCmd := exec.Command(browserArgs[0], endingArgs...)
 	return PrepareCmd(browseCmd).Run()
 }
 
-func searchBrowserLauncher(goos string) (browser string) {
-	switch goos {
-	case "darwin":
-		browser = "open"
-	case "windows":
-		browser = "cmd /c start"
-	default:
-		candidates := []string{"xdg-open", "cygstart", "x-www-browser", "firefox",
-			"opera", "mozilla", "netscape"}
-		for _, b := range candidates {
-			path, err := exec.LookPath(b)
-			if err == nil {
-				browser = path
-				break
-			}
-		}
-	}
-
-	return browser
-}
-
 func normalizeNewlines(d []byte) []byte {
 	d = bytes.Replace(d, []byte("\r\n"), []byte("\n"), -1)
 	d = bytes.Replace(d, []byte("\r"), []byte("\n"), -1)