From de0e66a14419b608f6d81ebd12598fceb07a91ea Mon Sep 17 00:00:00 2001 From: Max Resnick Date: Sat, 20 Sep 2025 23:59:46 -0700 Subject: fix: some things --- internal/modules/handler.go | 107 +++++++++---- internal/modules/handler_test.go | 316 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 396 insertions(+), 27 deletions(-) (limited to 'internal/modules') diff --git a/internal/modules/handler.go b/internal/modules/handler.go index 9fa1094..c01f716 100644 --- a/internal/modules/handler.go +++ b/internal/modules/handler.go @@ -5,6 +5,7 @@ import ( "fmt" "log/slog" "net/http" + "net/url" "path/filepath" "strings" @@ -18,6 +19,74 @@ type ModuleHandler struct { serverHost string } +// normalizeImportPath creates a properly formatted full import path +func (h *ModuleHandler) normalizeImportPath(modulePath string) (string, error) { + if h.serverHost == "" { + return "", fmt.Errorf("serverHost cannot be empty") + } + if modulePath == "" { + return "", fmt.Errorf("modulePath cannot be empty") + } + if strings.Contains(modulePath, "..") { + return "", fmt.Errorf("modulePath cannot contain '..'") + } + + // Clean the inputs + host := strings.TrimRight(h.serverHost, "/") + module := strings.Trim(modulePath, "/") + + // Validate that module path doesn't contain invalid characters + if strings.ContainsAny(module, " \t\n\r") { + return "", fmt.Errorf("modulePath cannot contain whitespace characters") + } + + return host + "/" + module, nil +} + +// buildRepoURL creates a proper repository URL +func (h *ModuleHandler) buildRepoURL(modulePath string) (string, error) { + _, err := h.normalizeImportPath(modulePath) + if err != nil { + return "", err + } + + // Use proper URL building to ensure valid URLs + repoURL := &url.URL{ + Scheme: "https", + Host: h.serverHost, + Path: "/" + strings.Trim(modulePath, "/"), + } + + return repoURL.String(), nil +} + +// generateGoImportHTML creates the HTML response for go-import requests +func (h *ModuleHandler) generateGoImportHTML(modulePath string) (string, error) { + fullImportPath, err := h.normalizeImportPath(modulePath) + if err != nil { + return "", fmt.Errorf("failed to normalize import path: %w", err) + } + + repoURL, err := h.buildRepoURL(modulePath) + if err != nil { + return "", fmt.Errorf("failed to build repository URL: %w", err) + } + + return fmt.Sprintf(` + + + + + + + go get %s + +`, + fullImportPath, repoURL, + fullImportPath, repoURL, repoURL, repoURL, + fullImportPath), nil +} + // NewModuleHandler creates a new module handler with explicit routes for known repos func NewModuleHandler(reposDir, serverHost string, config *admin.ServerRepos) http.Handler { handler := &ModuleHandler{ @@ -266,19 +335,12 @@ func (h *ModuleHandler) handleGoImportForModule(w http.ResponseWriter, r *http.R } // Generate HTML with go-import meta tag - html := fmt.Sprintf(` - - - - - - - go get %s - -`, - modulePath, h.serverHost, modulePath, - modulePath, h.serverHost, modulePath, h.serverHost, modulePath, h.serverHost, modulePath, - modulePath) + html, err := h.generateGoImportHTML(modulePath) + if err != nil { + slog.Error("failed to generate go-import HTML", "module", modulePath, "error", err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } w.Header().Set("Content-Type", "text/html; charset=utf-8") w.WriteHeader(http.StatusOK) @@ -300,19 +362,12 @@ func (h *ModuleHandler) handleGoImport(w http.ResponseWriter, r *http.Request) { modulePath := ExtractModulePath(r.URL.Path) // Generate HTML with go-import meta tag - html := fmt.Sprintf(` - - - - - - - go get %s - -`, - modulePath, h.serverHost, modulePath, - modulePath, h.serverHost, modulePath, h.serverHost, modulePath, h.serverHost, modulePath, - modulePath) + html, err := h.generateGoImportHTML(modulePath) + if err != nil { + slog.Error("failed to generate go-import HTML", "module", modulePath, "error", err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } w.Header().Set("Content-Type", "text/html; charset=utf-8") w.WriteHeader(http.StatusOK) diff --git a/internal/modules/handler_test.go b/internal/modules/handler_test.go index b173f98..943c74c 100644 --- a/internal/modules/handler_test.go +++ b/internal/modules/handler_test.go @@ -65,7 +65,7 @@ func TestHandleGoImport_ConfiguredModule(t *testing.T) { } // Check for correct module path and VCS info - expectedContent := `content="mymodule git https://git.example.com/mymodule"` + expectedContent := `content="git.example.com/mymodule git https://git.example.com/mymodule"` if !strings.Contains(body, expectedContent) { t.Errorf("response should contain %s", expectedContent) } @@ -548,3 +548,317 @@ func TestModuleMiddleware(t *testing.T) { } }) } + +// Tests for new helper functions +func TestNormalizeImportPath(t *testing.T) { + handler := &ModuleHandler{ + reposDir: "/tmp/repos", + serverHost: "git.example.com", + } + + tests := []struct { + name string + serverHost string + modulePath string + expected string + shouldErr bool + }{ + { + name: "valid_simple_path", + serverHost: "git.example.com", + modulePath: "mymodule", + expected: "git.example.com/mymodule", + shouldErr: false, + }, + { + name: "trailing_slash_on_host", + serverHost: "git.example.com/", + modulePath: "mymodule", + expected: "git.example.com/mymodule", + shouldErr: false, + }, + { + name: "leading_slash_on_module", + serverHost: "git.example.com", + modulePath: "/mymodule", + expected: "git.example.com/mymodule", + shouldErr: false, + }, + { + name: "nested_module_path", + serverHost: "git.example.com", + modulePath: "company/mymodule", + expected: "git.example.com/company/mymodule", + shouldErr: false, + }, + { + name: "empty_server_host", + serverHost: "", + modulePath: "mymodule", + expected: "", + shouldErr: true, + }, + { + name: "empty_module_path", + serverHost: "git.example.com", + modulePath: "", + expected: "", + shouldErr: true, + }, + { + name: "path_traversal_attempt", + serverHost: "git.example.com", + modulePath: "../../../etc/passwd", + expected: "", + shouldErr: true, + }, + { + name: "whitespace_in_module_path", + serverHost: "git.example.com", + modulePath: "my module", + expected: "", + shouldErr: true, + }, + { + name: "tab_in_module_path", + serverHost: "git.example.com", + modulePath: "my\tmodule", + expected: "", + shouldErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Update handler for this test + handler.serverHost = tt.serverHost + + result, err := handler.normalizeImportPath(tt.modulePath) + + if tt.shouldErr { + if err == nil { + t.Errorf("expected error but got none") + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if result != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, result) + } + } + }) + } +} + +func TestBuildRepoURL(t *testing.T) { + handler := &ModuleHandler{ + reposDir: "/tmp/repos", + serverHost: "git.example.com", + } + + tests := []struct { + name string + modulePath string + expected string + shouldErr bool + }{ + { + name: "simple_module", + modulePath: "mymodule", + expected: "https://git.example.com/mymodule", + shouldErr: false, + }, + { + name: "nested_module", + modulePath: "company/mymodule", + expected: "https://git.example.com/company/mymodule", + shouldErr: false, + }, + { + name: "invalid_module_path", + modulePath: "", + expected: "", + shouldErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := handler.buildRepoURL(tt.modulePath) + + if tt.shouldErr { + if err == nil { + t.Errorf("expected error but got none") + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if result != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, result) + } + } + }) + } +} + +func TestGenerateGoImportHTML(t *testing.T) { + handler := &ModuleHandler{ + reposDir: "/tmp/repos", + serverHost: "git.example.com", + } + + tests := []struct { + name string + modulePath string + shouldErr bool + expectedHTML []string // Strings that should be present in the HTML + }{ + { + name: "valid_module", + modulePath: "mymodule", + shouldErr: false, + expectedHTML: []string{ + ``, + `