Address HTTP scheme review feedback

Author: andrew

internal/cli/auth.go

@@ -70,6 +70,7 @@ func authLoginCmd() *cobra.Command {
 				}
 			}
 
+			scheme = strings.ToLower(scheme)
 			if scheme != "" && scheme != "http" && scheme != "https" {
 				return fmt.Errorf("invalid --scheme %q: must be http or https", scheme)
 			}

internal/cli/auth_test.go

@@ -113,6 +113,33 @@ func TestAuthLoginNonInteractive(t *testing.T) {
 	}
 }
 
+func TestAuthLoginNormalizesScheme(t *testing.T) {
+	resetCmd(rootCmd)
+	dir := t.TempDir()
+	t.Setenv("XDG_CONFIG_HOME", dir)
+	config.ResetCache()
+	defer config.ResetCache()
+
+	rootCmd.SetArgs([]string{
+		"auth", "login",
+		"--domain", "forgejo.example.com",
+		"--token", "test_token_123",
+		"--scheme", "HTTP",
+	})
+
+	if err := rootCmd.Execute(); err != nil {
+		t.Fatalf("auth login: %v", err)
+	}
+
+	data, err := os.ReadFile(filepath.Join(dir, "forge", "config"))
+	if err != nil {
+		t.Fatalf("reading config: %v", err)
+	}
+	if !strings.Contains(string(data), "scheme = http") {
+		t.Errorf("expected normalized scheme, got:\n%s", data)
+	}
+}
+
 func TestAuthLoginTokenCmd(t *testing.T) {
 	resetCmd(rootCmd)
 	dir := t.TempDir()

internal/config/config.go

@@ -339,6 +339,14 @@ func findProjectConfig(dir string) string {
 // Creates the config directory if needed. Sets file permissions to 0600
 // since the file may contain tokens.
 func SetDomain(domain, token, tokenCmd, forgeType, scheme string) error {
+	if scheme != "" {
+		normalizedScheme, err := parseScheme(scheme)
+		if err != nil {
+			return err
+		}
+		scheme = normalizedScheme
+	}
+
 	path := UserConfigPath()
 	if path == "" {
 		return fmt.Errorf("cannot determine config path")

internal/config/config_test.go

@@ -498,7 +498,7 @@ func TestSetDomainScheme(t *testing.T) {
 	dir := t.TempDir()
 	t.Setenv("XDG_CONFIG_HOME", dir)
 
-	if err := SetDomain("forgejo.local:3000", "tok", "", "forgejo", "http"); err != nil {
+	if err := SetDomain("forgejo.local:3000", "tok", "", "forgejo", "HTTP"); err != nil {
 		t.Fatalf("SetDomain: %v", err)
 	}
 
@@ -518,6 +518,22 @@ func TestSetDomainScheme(t *testing.T) {
 	}
 }
 
+func TestSetDomainRejectsInvalidScheme(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("XDG_CONFIG_HOME", dir)
+
+	err := SetDomain("forgejo.local", "tok", "", "forgejo", "ftp")
+	if err == nil {
+		t.Fatal("expected invalid scheme error")
+	}
+	if !strings.Contains(err.Error(), "invalid scheme") {
+		t.Errorf("expected scheme error, got %v", err)
+	}
+	if _, statErr := os.Stat(filepath.Join(dir, "forge", "config")); !os.IsNotExist(statErr) {
+		t.Errorf("config should not be written for an invalid scheme, stat error: %v", statErr)
+	}
+}
+
 func TestLoadFileInvalidScheme(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "config")

internal/resolve/resolve.go

@@ -64,13 +64,17 @@ func SetHost(host string) {
 // splitScheme splits a leading http:// or https:// off s and returns
 // (scheme, host). If s has no scheme, scheme is "".
 func splitScheme(s string) (scheme, host string) {
-	if h, ok := strings.CutPrefix(s, "http://"); ok {
-		return "http", strings.TrimRight(h, "/")
-	}
-	if h, ok := strings.CutPrefix(s, "https://"); ok {
-		return "https", strings.TrimRight(h, "/")
+	s = strings.TrimRight(s, "/")
+	u, err := url.Parse(s)
+	if err == nil && u.Host != "" {
+		switch strings.ToLower(u.Scheme) {
+		case "http":
+			return "http", u.Host
+		case "https":
+			return "https", u.Host
+		}
 	}
-	return "", strings.TrimRight(s, "/")
+	return "", s
 }
 
 // SetForgeType forces the API client implementation for the resolved domain,

internal/resolve/resolve_test.go

@@ -403,7 +403,7 @@ func TestSetHostWithScheme(t *testing.T) {
 	oldH, oldS := hostOverride, schemeOverride
 	defer func() { hostOverride, schemeOverride = oldH, oldS }()
 
-	SetHost("http://172.30.0.10:3000")
+	SetHost("HTTP://172.30.0.10:3000/forge/")
 	if hostOverride != "172.30.0.10:3000" {
 		t.Errorf("SetHost with URL should store bare host, got %q", hostOverride)
 	}