fix: harden url conversion and browser handling

Author: zhaochunqi

README.md

@@ -39,17 +39,20 @@ Download the pre-compiled binary for your platform:
 
 ```sh
 # macOS (Intel)
-curl -L https://github.com/zhaochunqi/git-open/releases/latest/download/git-open-darwin-amd64 -o git-open
+curl -L https://github.com/zhaochunqi/git-open/releases/latest/download/git-open_Darwin_x86_64.tar.gz -o git-open.tar.gz
+tar -xzf git-open.tar.gz
 chmod +x git-open
 sudo mv git-open /usr/local/bin/
 
 # macOS (Apple Silicon)
-curl -L https://github.com/zhaochunqi/git-open/releases/latest/download/git-open-darwin-arm64 -o git-open
+curl -L https://github.com/zhaochunqi/git-open/releases/latest/download/git-open_Darwin_arm64.tar.gz -o git-open.tar.gz
+tar -xzf git-open.tar.gz
 chmod +x git-open
 sudo mv git-open /usr/local/bin/
 
 # Linux (x64)
-curl -L https://github.com/zhaochunqi/git-open/releases/latest/download/git-open-linux-amd64 -o git-open
+curl -L https://github.com/zhaochunqi/git-open/releases/latest/download/git-open_Linux_x86_64.tar.gz -o git-open.tar.gz
+tar -xzf git-open.tar.gz
 chmod +x git-open
 sudo mv git-open /usr/local/bin/
 ```

cmd/browser.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"os/exec"
 	"runtime"
+	"strings"
 )
 
 // ErrMockBrowser is used for testing browser errors
@@ -17,9 +18,24 @@ var getPlatform = func() string {
 	return runtime.GOOS
 }
 
+var commandRunner = func(name string, args ...string) error {
+	cmd := exec.Command(name, args...)
+	// Redirect stdout and stderr to /dev/null to suppress output
+	cmd.Stdout = nil
+	cmd.Stderr = nil
+	return cmd.Start()
+}
+
+var BrowserCommand string
+
 func openURLInBrowser(url string) error {
 	platform := getPlatform()
-	
+
+	customBrowser := strings.TrimSpace(BrowserCommand)
+	if customBrowser != "" {
+		return commandRunner(customBrowser, url)
+	}
+
 	// On Linux, use xdg-open with output redirection to suppress messages
 	if platform == "linux" {
 		return openWithXdgOpen(url)
@@ -37,27 +53,15 @@ func openURLInBrowser(url string) error {
 }
 
 func openWithXdgOpen(url string) error {
-	cmd := exec.Command("xdg-open", url)
-	// Redirect stdout and stderr to /dev/null to suppress output
-	cmd.Stdout = nil
-	cmd.Stderr = nil
-	return cmd.Start()
+	return commandRunner("xdg-open", url)
 }
 
 func openWithMacOSOpen(url string) error {
-	cmd := exec.Command("open", url)
-	// Redirect stdout and stderr to /dev/null to suppress output
-	cmd.Stdout = nil
-	cmd.Stderr = nil
-	return cmd.Start()
+	return commandRunner("open", url)
 }
 
 func openWithWindowsStart(url string) error {
-	cmd := exec.Command("cmd", "/c", "start", "", url)
-	// Redirect stdout and stderr to /dev/null to suppress output
-	cmd.Stdout = nil
-	cmd.Stderr = nil
-	return cmd.Start()
+	return commandRunner("cmd", "/c", "start", "", url)
 }
 
 func openURLInBrowserFunc(url string) error {

cmd/browser_test.go

@@ -1,13 +1,30 @@
 package cmd
 
 import (
+	"os/exec"
 	"runtime"
 	"testing"
 )
 
 // mockOpenURL is a mock function for testing
 var mockOpenURL func(string) error
 
+func withNoopCommandRunner(t *testing.T) {
+	t.Helper()
+	t.Cleanup(func() {
+		commandRunner = func(name string, args ...string) error {
+			cmd := exec.Command(name, args...)
+			// Redirect stdout and stderr to /dev/null to suppress output
+			cmd.Stdout = nil
+			cmd.Stderr = nil
+			return cmd.Start()
+		}
+	})
+	commandRunner = func(name string, args ...string) error {
+		return nil
+	}
+}
+
 func Test_openURLInBrowser(t *testing.T) {
 	// Save original function
 	original := OpenURLInBrowser
@@ -76,6 +93,7 @@ func Test_openWithXdgOpen(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			withNoopCommandRunner(t)
 			err := openWithXdgOpen(tt.url)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("openWithXdgOpen() error = %v, wantErr %v", err, tt.wantErr)
@@ -104,6 +122,7 @@ func Test_openWithMacOSOpen(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			withNoopCommandRunner(t)
 			err := openWithMacOSOpen(tt.url)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("openWithMacOSOpen() error = %v, wantErr %v", err, tt.wantErr)
@@ -139,6 +158,7 @@ func Test_openURLInBrowser_PlatformSpecific(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			// Test the actual platform-specific implementation
+			withNoopCommandRunner(t)
 			err := openURLInBrowser(tt.url)
 			if (err != nil) != tt.expectError {
 				t.Errorf("openURLInBrowser() error = %v, expectError %v", err, tt.expectError)
@@ -175,11 +195,12 @@ func Test_openURLInBrowser_AllPlatforms(t *testing.T) {
 			// Save the original runtime.GOOS
 			originalGOOS := runtime.GOOS
 			// We can't actually change runtime.GOOS, but we can test the functions directly
-			
+
 			switch tt.platform {
 			case "linux":
 				// Test openWithXdgOpen directly if not on Linux
 				if runtime.GOOS != "linux" {
+					withNoopCommandRunner(t)
 					err := openWithXdgOpen(tt.url)
 					// On non-Linux systems, this should fail as xdg-open doesn't exist
 					if err == nil {
@@ -189,6 +210,7 @@ func Test_openURLInBrowser_AllPlatforms(t *testing.T) {
 			case "darwin":
 				// Test openWithMacOSOpen directly if not on macOS
 				if runtime.GOOS != "darwin" {
+					withNoopCommandRunner(t)
 					err := openWithMacOSOpen(tt.url)
 					// On non-macOS systems, this should fail as open command might not exist
 					if err == nil {
@@ -198,6 +220,7 @@ func Test_openURLInBrowser_AllPlatforms(t *testing.T) {
 			case "windows":
 				// Test openWithWindowsStart directly if not on Windows
 				if runtime.GOOS != "windows" {
+					withNoopCommandRunner(t)
 					err := openWithWindowsStart(tt.url)
 					// On non-Windows systems, this should fail as cmd doesn't exist
 					if err == nil {
@@ -258,14 +281,14 @@ func Test_openURLInBrowser_UnsupportedPlatform(t *testing.T) {
 			wantErr:  false, // May fail in CI but function should be called
 		},
 		{
-			name:     "macOS platform", 
+			name:     "macOS platform",
 			platform: "darwin",
 			url:      "https://github.com/test/repo",
 			wantErr:  false, // May fail in CI but function should be called
 		},
 		{
 			name:     "Windows platform",
-			platform: "windows", 
+			platform: "windows",
 			url:      "https://github.com/test/repo",
 			wantErr:  false, // May fail in CI but function should be called
 		},
@@ -291,9 +314,10 @@ func Test_openURLInBrowser_UnsupportedPlatform(t *testing.T) {
 			getPlatform = func() string {
 				return tt.platform
 			}
+			withNoopCommandRunner(t)
 
 			err := openURLInBrowser(tt.url)
-			
+
 			if tt.wantErr {
 				if err == nil {
 					t.Errorf("openURLInBrowser() expected error for platform %s, got nil", tt.platform)

cmd/git.go

@@ -3,6 +3,8 @@ package cmd
 import (
 	"errors"
 	"fmt"
+	"net/url"
+	"regexp"
 	"strings"
 
 	"github.com/go-git/go-git/v5"
@@ -22,7 +24,7 @@ const (
 // getCurrentGitDirectoryFunc is a variable that can be replaced for testing
 var getCurrentGitDirectoryFunc = func() (*git.Repository, error) {
 	repo, err := git.PlainOpenWithOptions(".", &git.PlainOpenOptions{
-		DetectDotGit:        true,
+		DetectDotGit:          true,
 		EnableDotGitCommonDir: true,
 	})
 	if err != nil {
@@ -56,26 +58,45 @@ func getRemoteURL(repo *git.Repository) (string, error) {
 	return getRemoteURLFunc(repo)
 }
 
-func convertToWebURL(url string) string {
-	// Validate URL format
-	if !strings.Contains(url, "://") && !strings.Contains(url, "@") {
+var scpRemoteURLPattern = regexp.MustCompile(`^(?:[^@]+@)?([^:]+):(.+)$`)
+
+func convertToWebURL(rawURL string) string {
+	raw := strings.TrimSpace(rawURL)
+	if raw == "" {
 		return ""
 	}
 
-	// If the URL starts with "https://" or "http://", remove the ".git" suffix
-	if strings.HasPrefix(url, "https://") || strings.HasPrefix(url, "http://") {
-		url = strings.TrimSuffix(url, ".git")
-	} else {
-		// Otherwise, assume it's an SSH URL
-		// Remove "ssh://" prefix
-		url = strings.TrimPrefix(url, "ssh://")
-		url = strings.Replace(url, ":", "/", 1)
-		// Replace "git@" or "ssh://git@" with "https://"
-		url = strings.Replace(url, "git@", "https://", 1)
-		// Remove the ".git" suffix
-		url = strings.TrimSuffix(url, ".git")
+	parsedURL, err := url.Parse(raw)
+	if err == nil && parsedURL.Host != "" && parsedURL.Scheme != "" {
+		// URL style: https://, http://, ssh:// or git+ssh://
+		path := strings.TrimPrefix(parsedURL.Path, "/")
+		if path == "" {
+			return ""
+		}
+
+		switch parsedURL.Scheme {
+		case "http", "https":
+			host := parsedURL.Host
+			return fmt.Sprintf("%s://%s/%s", parsedURL.Scheme, strings.TrimSuffix(host, "/"), strings.TrimSuffix(path, ".git"))
+		case "ssh", "git+ssh":
+			return fmt.Sprintf("https://%s/%s", parsedURL.Hostname(), strings.TrimSuffix(path, ".git"))
+		}
+
+		return ""
 	}
-	return url
+
+	matches := scpRemoteURLPattern.FindStringSubmatch(raw)
+	if len(matches) != 3 {
+		return ""
+	}
+
+	host := matches[1]
+	path := strings.TrimPrefix(matches[2], "/")
+	if host == "" || path == "" {
+		return ""
+	}
+
+	return fmt.Sprintf("https://%s/%s", host, strings.TrimSuffix(path, ".git"))
 }
 
 // getBranchNameFunc is a variable that can be replaced for testing

cmd/root.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -42,25 +43,23 @@ and converts it to a web URL. The web URL is then printed to the console.`,
 			return fmt.Errorf("unsupported remote URL format: %s", remoteURL)
 		}
 
+		branchName, err := getBranchName(repo)
+		if err == nil && shouldAppendBranch(branchName) {
+			// For now, we only append branch name if it's not 'main' or 'master'.
+			// This can be improved later to fetch default branch from remote or allow configuration.
+			webURL = buildBranchURL(webURL, branchName, remoteURL)
+		}
+
 		// Open the web URL in the browser if the -o flag is provided
 		plain, _ := cmd.Flags().GetBool("plain")
 		if plain {
 			fmt.Fprintf(cmd.OutOrStdout(), "Web URL: %s\n", webURL)
 			return nil
 		}
 
-		branchName, err := getBranchName(repo)
-		if err == nil {
-			// For now, we only append branch name if it's not 'main' or 'master'.
-			// This can be improved later to fetch default branch from remote or allow configuration.
-			if branchName != "main" && branchName != "master" {
-				webURL = buildBranchURL(webURL, branchName, remoteURL)
-			}
-		}
-
 		err = openURLInBrowserFunc(webURL)
 		if err != nil {
-			fmt.Fprintf(cmd.OutOrStderr(), "Error opening URL in browser: %v\n", err)
+			return fmt.Errorf("error opening URL in browser: %w", err)
 		}
 		return nil
 	},
@@ -86,7 +85,6 @@ func init() {
 
 	// Cobra also supports local flags, which will only run
 	// when this action is called directly.
-	rootCmd.Flags().BoolP("toggle", "t", false, "Help message for toggle")
 	rootCmd.Flags().BoolP("plain", "p", false, "Just print the web url without opening.")
 	rootCmd.Flags().BoolP("version", "v", false, "Show version information")
 }
@@ -113,4 +111,10 @@ func initConfig() {
 	if err := viper.ReadInConfig(); err == nil {
 		fmt.Fprintln(os.Stderr, "Using config file:", viper.ConfigFileUsed())
 	}
+
+	BrowserCommand = strings.TrimSpace(viper.GetString("browser"))
+}
+
+func shouldAppendBranch(branchName string) bool {
+	return branchName != "main" && branchName != "master"
 }