feat(gitter): parse repo url and enable more ways to validate

go/cmd/gitter/gitter.go

@@ -14,6 +14,7 @@ import (
 	"math"
 	"net"
 	"net/http"
+	"net/url"
 	"os"
 	"os/exec"
 	"os/signal"
@@ -223,32 +224,35 @@ func isLocalRequest(r *http.Request) bool {
 	return ip.IsLoopback()
 }
 
-func prepareURL(r *http.Request, url string) (string, error) {
-	if url == "" {
+func prepareURL(r *http.Request, repoURL string) (string, error) {
+	if repoURL == "" {
 		return "", errors.New("missing url parameter")
 	}
 
-	url = normalizeURL(url)
+	u, err := url.Parse(repoURL)
+	if err != nil {
+		return "", fmt.Errorf("error parsing url: %w", err)
+	}
 
-	// If request came from a local ip, don't do the check
-	if !isLocalRequest(r) {
-		// Check if url starts with protocols: http(s)://, git://
-		if !validURLRegex.MatchString(url) {
-			return "", errors.New("invalid url parameter")
-		}
+	// Convert git://github.com to https://github.com because it times out for some reason
+	// git protocol on non-github urls works fine
+	if u.Scheme == "git" && u.Host == "github.com" {
+		u.Scheme = "https"
 	}
 
-	return url, nil
-}
+	// Remove query and fragment from the URL
+	u.RawQuery = ""
+	u.Fragment = ""
 
-func normalizeURL(url string) string {
-	// Convert git://github.com/ to https://github.com/ because it times out for some reason
-	// git protocol on non-github urls works fine
-	if s, ok := strings.CutPrefix(url, "git://github.com/"); ok {
-		return "https://github.com/" + s
+	if !isLocalRequest(r) {
+		if u.Scheme != "http" && u.Scheme != "https" && u.Scheme != "git" {
+			return "", fmt.Errorf("unsupported protocol: %s", u.Scheme)
+		}
 	}
 
-	return url
+	logger.Info("Prepared URL", slog.String("from", repoURL), slog.String("to", u.String()))
+
+	return u.String(), nil
 }
 
 func getRepoDirName(url string) string {

go/cmd/gitter/gitter_test.go

@@ -38,22 +38,86 @@ func TestGetRepoDirName(t *testing.T) {
 	}
 }
 
-func TestNormalizeURL(t *testing.T) {
+func TestPrepareURL(t *testing.T) {
 	tests := []struct {
-		url      string
-		expected string
+		name      string
+		url       string
+		expected  string
+		expectErr bool
 	}{
-		{"git://github.com/google/osv.dev.git", "https://github.com/google/osv.dev.git"},
-		{"https://github.com/google/osv.dev.git", "https://github.com/google/osv.dev.git"},
-		{"git://github.com/google/osv.dev", "https://github.com/google/osv.dev"},
-		{"http://github.com/google/osv.dev", "http://github.com/google/osv.dev"},
-		{"git://gitlab.com/google/osv.dev", "git://gitlab.com/google/osv.dev"},
+		{
+			name:      "Normal https protocol",
+			url:       "https://github.com/google/osv.dev.git",
+			expected:  "https://github.com/google/osv.dev.git",
+			expectErr: false,
+		},
+		{
+			name:      "GitHub http protocol",
+			url:       "http://github.com/google/osv.dev",
+			expected:  "http://github.com/google/osv.dev",
+			expectErr: false,
+		},
+		{
+			name:      "GitHub git protocol, change to https",
+			url:       "git://github.com/google/osv.dev.git",
+			expected:  "https://github.com/google/osv.dev.git",
+			expectErr: false,
+		},
+		{
+			name:      "Non-github git protocol, no change",
+			url:       "git://code.qt.io/qt/qt5.git",
+			expected:  "git://code.qt.io/qt/qt5.git",
+			expectErr: false,
+		},
+		{
+			name:      "Non github url",
+			url:       "https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux.git",
+			expected:  "https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux.git",
+			expectErr: false,
+		},
+		{
+			name:      "URL with query parameters",
+			url:       "https://github.com/google/osv.dev.git?q=value",
+			expected:  "https://github.com/google/osv.dev.git",
+			expectErr: false,
+		},
+		{
+			name:      "URL with fragment",
+			url:       "https://github.com/lxml/lxml#diff-59130575b4fb2932c957db2922977d7d89afb0b2085357db1a14615a2fcad776",
+			expected:  "https://github.com/lxml/lxml",
+			expectErr: false,
+		},
+		{
+			name:      "Empty URL",
+			url:       "",
+			expected:  "",
+			expectErr: true,
+		},
+		{
+			name:      "Unsupported protocol file://",
+			url:       "file://this-is-invalid",
+			expected:  "",
+			expectErr: true,
+		},
 	}
 
 	for _, tt := range tests {
-		if result := normalizeURL(tt.url); result != tt.expected {
-			t.Errorf("normalizeURL(%s) = %s, expected %s", tt.url, result, tt.expected)
-		}
+		t.Run(tt.name, func(t *testing.T) {
+			req := httptest.NewRequest("GET", "/test", nil)
+			result, err := prepareURL(req, tt.url)
+			if tt.expectErr {
+				if err == nil {
+					t.Errorf("prepareURL() expected error, got nil")
+				}
+			} else {
+				if err != nil {
+					t.Errorf("prepareURL() unexpected error: %v", err)
+				}
+				if result != tt.expected {
+					t.Errorf("prepareURL() = %s, expected %s", result, tt.expected)
+				}
+			}
+		})
 	}
 }