From 3b049201800fe52b270f2c00a08e27501fac818b Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 14 May 2026 18:21:51 +0800 Subject: [PATCH 1/9] feat: support separate bind address and server name Add --server-name flag to 'sth start' that controls the externally visible address while --host controls the actual bind address. Usage: sth start --host 0.0.0.0 --port 3939 --server-name 192.168.2.14 Closes #48 --- internal/cli/cli.go | 9 ++++-- internal/cli/cli_test.go | 2 +- internal/cli/metadata_test.go | 6 ++-- internal/server/server.go | 17 ++++++++--- internal/server/server_test.go | 56 ++++++++++++++++++++++++++-------- 5 files changed, 68 insertions(+), 22 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 02ab492..83767e1 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -54,7 +54,7 @@ func Run(args []string, stdout io.Writer, stderr io.Writer) error { func printUsage(w io.Writer) { fmt.Fprintln(w, `Usage: - sth start [--host 0.0.0.0] [--port 3939] [--db /path/to/sessions.db] + sth start [--host 0.0.0.0] [--port 3939] [--server-name ] [--db /path/to/sessions.db] sth send --tag --category --project [--server http://127.0.0.1:3939] sth tag [--rm] [--db /path/to/sessions.db] [--server http://...] sth categorize [--db /path/to/sessions.db] [--server http://...] @@ -87,12 +87,17 @@ func runStart(args []string, stdout io.Writer) error { return errors.New("port must be a positive integer") } + var serverName string + if value, ok := flags["server-name"]; ok { + serverName = value + } + store, err := openStore(flags) if err != nil { return err } - srv, err := server.New(host, port, store) + srv, err := server.New(host, port, store, serverName) if err != nil { return errors.Join(err, store.Close()) } diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 2e482ae..6cdd018 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -22,7 +22,7 @@ func TestSendPrintsSessionURL(t *testing.T) { t.Parallel() store := newTestStore(t) - srv, err := server.New("127.0.0.1", 0, store) + srv, err := server.New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } diff --git a/internal/cli/metadata_test.go b/internal/cli/metadata_test.go index 4efd071..9a05edf 100644 --- a/internal/cli/metadata_test.go +++ b/internal/cli/metadata_test.go @@ -325,7 +325,7 @@ func TestHomePageShowsMetadata(t *testing.T) { } store := newTestStore(t) - srv, err := server.New("127.0.0.1", 0, store) + srv, err := server.New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } @@ -381,7 +381,7 @@ func TestHomePageFilterByTag(t *testing.T) { } store := newTestStore(t) - srv, err := server.New("127.0.0.1", 0, store) + srv, err := server.New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } @@ -429,7 +429,7 @@ func TestHomePageSearch(t *testing.T) { t.Parallel() store := newTestStore(t) - srv, err := server.New("127.0.0.1", 0, store) + srv, err := server.New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } diff --git a/internal/server/server.go b/internal/server/server.go index 684ed2c..49a0263 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -32,6 +32,7 @@ const ( type Server struct { host string port int + serverName string store *session.Store httpServer *http.Server listener net.Listener @@ -401,7 +402,7 @@ var homePageTemplate = template.Must(template.New("home").Parse(` `)) -func New(host string, port int, store *session.Store) (*Server, error) { +func New(host string, port int, store *session.Store, serverName string) (*Server, error) { if host == "" { host = DefaultHost } @@ -411,9 +412,10 @@ func New(host string, port int, store *session.Store) (*Server, error) { } srv := &Server{ - host: host, - port: port, - store: store, + host: host, + port: port, + serverName: serverName, + store: store, } srv.httpServer = &http.Server{ @@ -475,6 +477,9 @@ func (s *Server) Origin() string { } ip := address.IP + if s.serverName != "" { + return fmt.Sprintf("http://%s:%d", s.serverName, address.Port) + } if ip.IsUnspecified() { ip = net.IPv4(127, 0, 0, 1) } @@ -497,6 +502,10 @@ func (s *Server) Origins() []string { port := address.Port + if s.serverName != "" { + return []string{fmt.Sprintf("http://%s:%d", s.serverName, port)} + } + if address.IP.IsUnspecified() { origins := []string{fmt.Sprintf("http://127.0.0.1:%d", port)} diff --git a/internal/server/server_test.go b/internal/server/server_test.go index d3a3393..8491b94 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -11,6 +11,7 @@ import ( "net/url" "os" "path/filepath" + "strings" "testing" "time" @@ -20,12 +21,43 @@ import ( func TestNewRequiresStore(t *testing.T) { t.Parallel() - _, err := New("127.0.0.1", 0, nil) + _, err := New("127.0.0.1", 0, nil, "") if err == nil { t.Fatal("expected nil store to be rejected") } } +func TestServerNameOverridesOrigin(t *testing.T) { + t.Parallel() + + store := newTestStore(t) + srv, err := New("127.0.0.1", 0, store, "192.168.2.14") + if err != nil { + t.Fatal(err) + } + if err := srv.Start(); err != nil { + t.Fatal(err) + } + defer func() { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + _ = srv.Stop(ctx) + }() + + origin := srv.Origin() + if !strings.HasPrefix(origin, "http://192.168.2.14:") { + t.Fatalf("expected origin to use server-name 192.168.2.14, got %q", origin) + } + + origins := srv.Origins() + if len(origins) != 1 { + t.Fatalf("expected exactly 1 origin with server-name, got %d", len(origins)) + } + if !strings.HasPrefix(origins[0], "http://192.168.2.14:") { + t.Fatalf("expected origins[0] to use server-name 192.168.2.14, got %q", origins[0]) + } +} + func TestCreateSessionAndServeAssets(t *testing.T) { t.Parallel() @@ -36,7 +68,7 @@ func TestCreateSessionAndServeAssets(t *testing.T) { } store := newTestStore(t) - srv, err := New("127.0.0.1", 0, store) + srv, err := New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } @@ -130,7 +162,7 @@ func TestTraversalIsRejected(t *testing.T) { } store := newTestStore(t) - srv, err := New("127.0.0.1", 0, store) + srv, err := New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } @@ -194,7 +226,7 @@ func TestCreateUploadedSessionAndServeAssets(t *testing.T) { } store := newTestStore(t) - srv, err := New("127.0.0.1", 0, store) + srv, err := New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } @@ -372,7 +404,7 @@ func TestDeleteSessionSuccess(t *testing.T) { } store := newTestStore(t) - srv, err := New("127.0.0.1", 0, store) + srv, err := New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } @@ -416,7 +448,7 @@ func TestDeleteSessionNotFound(t *testing.T) { t.Parallel() store := newTestStore(t) - srv, err := New("127.0.0.1", 0, store) + srv, err := New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } @@ -455,7 +487,7 @@ func TestSearchByFileContent(t *testing.T) { } store := newTestStore(t) - srv, err := New("127.0.0.1", 0, store) + srv, err := New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } @@ -500,7 +532,7 @@ func TestSearchNoResults(t *testing.T) { } store := newTestStore(t) - srv, err := New("127.0.0.1", 0, store) + srv, err := New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } @@ -546,7 +578,7 @@ func TestSearchContentNoDuplicate(t *testing.T) { } store := newTestStore(t) - srv, err := New("127.0.0.1", 0, store) + srv, err := New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } @@ -592,7 +624,7 @@ func TestDeleteSessionIdempotent(t *testing.T) { } store := newTestStore(t) - srv, err := New("127.0.0.1", 0, store) + srv, err := New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } @@ -634,7 +666,7 @@ func TestDownloadSession(t *testing.T) { } store := newTestStore(t) - srv, err := New("127.0.0.1", 0, store) + srv, err := New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } @@ -692,7 +724,7 @@ func TestDownloadSessionNotFound(t *testing.T) { t.Parallel() store := newTestStore(t) - srv, err := New("127.0.0.1", 0, store) + srv, err := New("127.0.0.1", 0, store, "") if err != nil { t.Fatal(err) } From 028cca9a8892f452e0b262d3c5f5f2c4cbdda14d Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 14 May 2026 18:25:46 +0800 Subject: [PATCH 2/9] fix: add serverName validation and improve Origins() fallback per review - Validate --server-name rejects spaces, slashes, colons, and protocol prefixes - Origins() now includes 127.0.0.1 as fallback when serverName is set - Add tests for domain serverName, empty string fallback, and validation --- internal/server/server.go | 11 +++++- internal/server/server_test.go | 72 +++++++++++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/internal/server/server.go b/internal/server/server.go index 49a0263..4a5933a 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -407,6 +407,12 @@ func New(host string, port int, store *session.Store, serverName string) (*Serve host = DefaultHost } + if serverName != "" { + if strings.ContainsAny(serverName, " /:") || strings.HasPrefix(serverName, "http://") || strings.HasPrefix(serverName, "https://") { + return nil, errors.New("server: --server-name must be a plain hostname or IP (no spaces, slashes, colons, or protocol prefix)") + } + } + if store == nil { return nil, errors.New("server: store must not be nil") } @@ -503,7 +509,10 @@ func (s *Server) Origins() []string { port := address.Port if s.serverName != "" { - return []string{fmt.Sprintf("http://%s:%d", s.serverName, port)} + return []string{ + fmt.Sprintf("http://%s:%d", s.serverName, port), + fmt.Sprintf("http://127.0.0.1:%d", port), + } } if address.IP.IsUnspecified() { diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 8491b94..01adeb5 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -50,12 +50,80 @@ func TestServerNameOverridesOrigin(t *testing.T) { } origins := srv.Origins() - if len(origins) != 1 { - t.Fatalf("expected exactly 1 origin with server-name, got %d", len(origins)) + if len(origins) != 2 { + t.Fatalf("expected 2 origins with server-name (custom + 127.0.0.1), got %d", len(origins)) } if !strings.HasPrefix(origins[0], "http://192.168.2.14:") { t.Fatalf("expected origins[0] to use server-name 192.168.2.14, got %q", origins[0]) } + if !strings.HasPrefix(origins[1], "http://127.0.0.1:") { + t.Fatalf("expected origins[1] to be 127.0.0.1 fallback, got %q", origins[1]) + } +} + +func TestServerNameDomain(t *testing.T) { + t.Parallel() + + store := newTestStore(t) + srv, err := New("127.0.0.1", 0, store, "myhost.local") + if err != nil { + t.Fatal(err) + } + if err := srv.Start(); err != nil { + t.Fatal(err) + } + defer func() { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + _ = srv.Stop(ctx) + }() + + origin := srv.Origin() + if !strings.HasPrefix(origin, "http://myhost.local:") { + t.Fatalf("expected origin to use server-name myhost.local, got %q", origin) + } +} + +func TestServerNameEmptyFallsBack(t *testing.T) { + t.Parallel() + + store := newTestStore(t) + srv, err := New("127.0.0.1", 0, store, "") + if err != nil { + t.Fatal(err) + } + if err := srv.Start(); err != nil { + t.Fatal(err) + } + defer func() { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + _ = srv.Stop(ctx) + }() + + origin := srv.Origin() + if !strings.HasPrefix(origin, "http://127.0.0.1:") { + t.Fatalf("expected origin to default to 127.0.0.1, got %q", origin) + } +} + +func TestServerNameValidation(t *testing.T) { + t.Parallel() + + store := newTestStore(t) + + for _, invalid := range []string{ + "has space", + "has/slash", + "has:colon", + "http://host", + "https://host", + } { + _, err := New("127.0.0.1", 0, store, invalid) + if err == nil { + t.Errorf("expected serverName %q to be rejected", invalid) + } + } } func TestCreateSessionAndServeAssets(t *testing.T) { From d91a4de6b7f0698e6e513736246c0e01e823e5c0 Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 14 May 2026 18:29:43 +0800 Subject: [PATCH 3/9] refactor: address review feedback on serverName validation and Origins() --- internal/server/server.go | 7 ++++--- internal/server/server_test.go | 7 ++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/internal/server/server.go b/internal/server/server.go index 4a5933a..11341c8 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -408,8 +408,10 @@ func New(host string, port int, store *session.Store, serverName string) (*Serve } if serverName != "" { - if strings.ContainsAny(serverName, " /:") || strings.HasPrefix(serverName, "http://") || strings.HasPrefix(serverName, "https://") { - return nil, errors.New("server: --server-name must be a plain hostname or IP (no spaces, slashes, colons, or protocol prefix)") + // NOTE: this validation rejects IPv6 addresses (which contain colons). + // Supporting IPv6 would require bracketed format ([::1]:port) in URLs. + if strings.ContainsAny(serverName, " /:") { + return nil, errors.New("server: --server-name must be a plain hostname or IPv4 address (no spaces, slashes, colons, or protocol prefix)") } } @@ -511,7 +513,6 @@ func (s *Server) Origins() []string { if s.serverName != "" { return []string{ fmt.Sprintf("http://%s:%d", s.serverName, port), - fmt.Sprintf("http://127.0.0.1:%d", port), } } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 01adeb5..35c829a 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -50,15 +50,12 @@ func TestServerNameOverridesOrigin(t *testing.T) { } origins := srv.Origins() - if len(origins) != 2 { - t.Fatalf("expected 2 origins with server-name (custom + 127.0.0.1), got %d", len(origins)) + if len(origins) != 1 { + t.Fatalf("expected 1 origin with server-name, got %d", len(origins)) } if !strings.HasPrefix(origins[0], "http://192.168.2.14:") { t.Fatalf("expected origins[0] to use server-name 192.168.2.14, got %q", origins[0]) } - if !strings.HasPrefix(origins[1], "http://127.0.0.1:") { - t.Fatalf("expected origins[1] to be 127.0.0.1 fallback, got %q", origins[1]) - } } func TestServerNameDomain(t *testing.T) { From 918c5cb7cd66c1b1ce0cdccfa4185b92309d75f6 Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 14 May 2026 18:34:31 +0800 Subject: [PATCH 4/9] fix: use allowlist validation for serverName and add boundary tests Replace denylist-based serverName validation with positive allowlist using net.ParseIP for IPs and hostname regex for domain names. Add boundary test cases for special characters, leading/trailing dots, and valid hostname patterns. Add HTTP-only scheme comment on Origin(). --- internal/server/server.go | 17 +++++++++++++---- internal/server/server_test.go | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/internal/server/server.go b/internal/server/server.go index 11341c8..ea78deb 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -15,6 +15,7 @@ import ( "os" "path" "path/filepath" + "regexp" "strings" "sync" @@ -29,6 +30,15 @@ const ( maxArchiveFiles = 2048 ) +var hostnamePattern = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9.\-]*[a-zA-Z0-9])?$`) + +func isValidServerName(name string) bool { + if net.ParseIP(name) != nil { + return true + } + return hostnamePattern.MatchString(name) +} + type Server struct { host string port int @@ -408,10 +418,8 @@ func New(host string, port int, store *session.Store, serverName string) (*Serve } if serverName != "" { - // NOTE: this validation rejects IPv6 addresses (which contain colons). - // Supporting IPv6 would require bracketed format ([::1]:port) in URLs. - if strings.ContainsAny(serverName, " /:") { - return nil, errors.New("server: --server-name must be a plain hostname or IPv4 address (no spaces, slashes, colons, or protocol prefix)") + if !isValidServerName(serverName) { + return nil, errors.New("server: --server-name must be a valid IPv4 address or hostname (letters, digits, dots, hyphens)") } } @@ -484,6 +492,7 @@ func (s *Server) Origin() string { return "" } + // NOTE: scheme is hardcoded to http, consistent with the rest of the codebase. ip := address.IP if s.serverName != "" { return fmt.Sprintf("http://%s:%d", s.serverName, address.Port) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 35c829a..2128924 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -115,6 +115,18 @@ func TestServerNameValidation(t *testing.T) { "has:colon", "http://host", "https://host", + "host@evil", + "host#fragment", + "host?query=1", + "host%20name", + "host\nnewline", + "host\rtab", + "host\ttab", + "..", + ".start", + "end.", + "-hyphen", + "hyphen-", } { _, err := New("127.0.0.1", 0, store, invalid) if err == nil { @@ -123,6 +135,29 @@ func TestServerNameValidation(t *testing.T) { } } +func TestServerNameValidationAcceptsValid(t *testing.T) { + t.Parallel() + + store := newTestStore(t) + + for _, valid := range []string{ + "192.168.2.14", + "myhost.local", + "sub.domain.example.com", + "host-with-dashes.local", + "a", + "localhost", + "10.0.0.1", + "255.255.255.255", + "host123", + } { + _, err := New("127.0.0.1", 0, store, valid) + if err != nil { + t.Errorf("expected serverName %q to be accepted, got error: %v", valid, err) + } + } +} + func TestCreateSessionAndServeAssets(t *testing.T) { t.Parallel() From 51a1a37c9fcc070853fabe8c4ad1fe8154abc80a Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 14 May 2026 18:37:56 +0800 Subject: [PATCH 5/9] fix: reject IPv6 in serverName and remove redundant IP assignment - Only accept IPv4 in isValidServerName to avoid generating invalid URLs (IPv6 addresses need bracket wrapping in URLs which is not supported) - Move ip := address.IP after serverName check in Origin() for clarity - Add IPv6 test cases to invalid serverName list --- internal/server/server.go | 6 +++--- internal/server/server_test.go | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/server/server.go b/internal/server/server.go index ea78deb..ad6f2b8 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -33,8 +33,8 @@ const ( var hostnamePattern = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9.\-]*[a-zA-Z0-9])?$`) func isValidServerName(name string) bool { - if net.ParseIP(name) != nil { - return true + if ip := net.ParseIP(name); ip != nil { + return ip.To4() != nil } return hostnamePattern.MatchString(name) } @@ -493,10 +493,10 @@ func (s *Server) Origin() string { } // NOTE: scheme is hardcoded to http, consistent with the rest of the codebase. - ip := address.IP if s.serverName != "" { return fmt.Sprintf("http://%s:%d", s.serverName, address.Port) } + ip := address.IP if ip.IsUnspecified() { ip = net.IPv4(127, 0, 0, 1) } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 2128924..35a758e 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -127,6 +127,9 @@ func TestServerNameValidation(t *testing.T) { "end.", "-hyphen", "hyphen-", + "2001:db8::1", + "::1", + "fe80::1%eth0", } { _, err := New("127.0.0.1", 0, store, invalid) if err == nil { From e334ec8bfbde33bf06b331cca7fb3517a294a926 Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 14 May 2026 19:13:53 +0800 Subject: [PATCH 6/9] fix: address review feedback for --server-name - Reject consecutive dots in server names (e.g. a..b) in isValidServerName - Use serverName in session creation response URLs via serverBaseURL() - Add --bind as alias for --host per issue spec - Add test for serverName used in session URLs - Add a..b and host..domain.com to invalid test cases --- internal/cli/cli.go | 2 ++ internal/server/server.go | 22 +++++++++--- internal/server/server_test.go | 65 ++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 4 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 83767e1..3c47d2f 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -73,6 +73,8 @@ func runStart(args []string, stdout io.Writer) error { host := server.DefaultHost if value, ok := flags["host"]; ok { host = value + } else if value, ok := flags["bind"]; ok { + host = value } port := server.DefaultPort diff --git a/internal/server/server.go b/internal/server/server.go index ad6f2b8..80380f5 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -36,6 +36,9 @@ func isValidServerName(name string) bool { if ip := net.ParseIP(name); ip != nil { return ip.To4() != nil } + if strings.Contains(name, "..") { + return false + } return hostnamePattern.MatchString(name) } @@ -835,10 +838,10 @@ func (s *Server) handleCreatePathSession(w http.ResponseWriter, r *http.Request) return } - baseURL := baseURL(r) + base := s.serverBaseURL(r) response := createSessionResponse{ SessionID: session.ID, - URL: baseURL + "/s/" + session.ID + "/", + URL: base + "/s/" + session.ID + "/", EntryFile: session.EntryFile, RootDir: session.RootDir, } @@ -955,10 +958,10 @@ func (s *Server) handleCreateUploadedSession(w http.ResponseWriter, r *http.Requ return } - baseURL := baseURL(r) + base := s.serverBaseURL(r) response := createSessionResponse{ SessionID: session.ID, - URL: baseURL + "/s/" + session.ID + "/", + URL: base + "/s/" + session.ID + "/", EntryFile: session.EntryFile, RootDir: session.RootDir, } @@ -1331,6 +1334,17 @@ func baseURL(r *http.Request) string { return scheme + "://" + r.Host } +func (s *Server) serverBaseURL(r *http.Request) string { + s.mu.RLock() + defer s.mu.RUnlock() + + if s.serverName != "" { + return fmt.Sprintf("http://%s:%d", s.serverName, s.port) + } + + return baseURL(r) +} + func hasPrefixSuffix(path, prefix, suffix string) bool { return strings.HasPrefix(path, prefix) && strings.HasSuffix(path, suffix) } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 35a758e..d772f63 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -5,8 +5,10 @@ import ( "bytes" "context" "encoding/json" + "fmt" "io" "mime/multipart" + "net" "net/http" "net/url" "os" @@ -130,6 +132,8 @@ func TestServerNameValidation(t *testing.T) { "2001:db8::1", "::1", "fe80::1%eth0", + "a..b", + "host..domain.com", } { _, err := New("127.0.0.1", 0, store, invalid) if err == nil { @@ -138,6 +142,67 @@ func TestServerNameValidation(t *testing.T) { } } +func TestServerNameUsedInSessionURL(t *testing.T) { + t.Parallel() + + fixtureHTML, err := filepath.Abs(filepath.Join("..", "..", "fixtures", "basic", "index.html")) + if err != nil { + t.Fatal(err) + } + + store := newTestStore(t) + srv, err := New("127.0.0.1", 0, store, "192.168.2.14") + if err != nil { + t.Fatal(err) + } + if err := srv.Start(); err != nil { + t.Fatal(err) + } + defer func() { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + _ = srv.Stop(ctx) + }() + + body, err := json.Marshal(map[string]any{ + "filePath": fixtureHTML, + "tags": []string{"test"}, + "category": "test", + "project": "test", + }) + if err != nil { + t.Fatal(err) + } + + srv.mu.RLock() + actualAddr := srv.listener.Addr().(*net.TCPAddr) + srv.mu.RUnlock() + actualURL := fmt.Sprintf("http://127.0.0.1:%d", actualAddr.Port) + + createResp, err := http.Post(actualURL+"/api/sessions", "application/json", bytes.NewReader(body)) + if err != nil { + t.Fatal(err) + } + defer createResp.Body.Close() + + if createResp.StatusCode != http.StatusCreated { + respBody, _ := io.ReadAll(createResp.Body) + t.Fatalf("unexpected status: %d body=%s", createResp.StatusCode, respBody) + } + + var payload struct { + URL string `json:"url"` + } + if err := json.NewDecoder(createResp.Body).Decode(&payload); err != nil { + t.Fatal(err) + } + + expectedPrefix := "http://192.168.2.14:" + if !strings.HasPrefix(payload.URL, expectedPrefix) { + t.Fatalf("expected session URL to use serverName, got %q", payload.URL) + } +} + func TestServerNameValidationAcceptsValid(t *testing.T) { t.Parallel() From ef1a69de56d29a8cc5b6fb1acc344a0fe0813fe7 Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 14 May 2026 19:17:43 +0800 Subject: [PATCH 7/9] fix: address review feedback - use listener port, respect X-Forwarded-Proto, document --bind - serverBaseURL now uses actual listener port instead of configured port - serverBaseURL respects X-Forwarded-Proto header and TLS for scheme - Add comment explaining why '..' check is needed despite regex - Rename host variable to bindAddr in cli.go for clarity - Document --bind flag in usage string --- internal/cli/cli.go | 12 ++++++------ internal/server/server.go | 14 +++++++++++++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 3c47d2f..f89a59d 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -54,7 +54,7 @@ func Run(args []string, stdout io.Writer, stderr io.Writer) error { func printUsage(w io.Writer) { fmt.Fprintln(w, `Usage: - sth start [--host 0.0.0.0] [--port 3939] [--server-name ] [--db /path/to/sessions.db] + sth start [--host 0.0.0.0] [--bind 0.0.0.0] [--port 3939] [--server-name ] [--db /path/to/sessions.db] sth send --tag --category --project [--server http://127.0.0.1:3939] sth tag [--rm] [--db /path/to/sessions.db] [--server http://...] sth categorize [--db /path/to/sessions.db] [--server http://...] @@ -70,11 +70,11 @@ func runStart(args []string, stdout io.Writer) error { return err } - host := server.DefaultHost + bindAddr := server.DefaultHost if value, ok := flags["host"]; ok { - host = value + bindAddr = value } else if value, ok := flags["bind"]; ok { - host = value + bindAddr = value } port := server.DefaultPort @@ -99,7 +99,7 @@ func runStart(args []string, stdout io.Writer) error { return err } - srv, err := server.New(host, port, store, serverName) + srv, err := server.New(bindAddr, port, store, serverName) if err != nil { return errors.Join(err, store.Close()) } @@ -110,7 +110,7 @@ func runStart(args []string, stdout io.Writer) error { origins := srv.Origins() if len(origins) == 0 { - fmt.Fprintf(stdout, "HTML server listening on %s:%d\n", host, port) + fmt.Fprintf(stdout, "HTML server listening on %s:%d\n", bindAddr, port) } else if len(origins) == 1 { fmt.Fprintf(stdout, "HTML server listening on %s\n", origins[0]) } else { diff --git a/internal/server/server.go b/internal/server/server.go index 80380f5..14e295b 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -36,6 +36,8 @@ func isValidServerName(name string) bool { if ip := net.ParseIP(name); ip != nil { return ip.To4() != nil } + // Block consecutive dots explicitly; the hostname regex allows '.' in + // the character class which would otherwise match inputs like "a..b". if strings.Contains(name, "..") { return false } @@ -1339,7 +1341,17 @@ func (s *Server) serverBaseURL(r *http.Request) string { defer s.mu.RUnlock() if s.serverName != "" { - return fmt.Sprintf("http://%s:%d", s.serverName, s.port) + port := s.port + if s.listener != nil { + if addr, ok := s.listener.Addr().(*net.TCPAddr); ok { + port = addr.Port + } + } + scheme := "http" + if r.Header.Get("X-Forwarded-Proto") == "https" || r.TLS != nil { + scheme = "https" + } + return fmt.Sprintf("%s://%s:%d", scheme, s.serverName, port) } return baseURL(r) From d1000fd8566e2296914a77a860ab6b26729520ba Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 14 May 2026 19:22:15 +0800 Subject: [PATCH 8/9] fix: address review feedback - omit default ports, unify scheme logic, improve regex - serverBaseURL omits port 80 (http) and 443 (https) in generated URLs - Extract determineScheme() for consistent scheme detection across baseURL and serverBaseURL - Improve hostnamePattern regex to natively reject consecutive dots - Warn when both --host and --bind are specified - Add unit tests for serverBaseURL port handling, fallback, and X-Forwarded-Proto --- internal/cli/cli.go | 5 +++ internal/server/server.go | 29 +++++++------- internal/server/server_test.go | 73 ++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 14 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index f89a59d..e9e5e4c 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -71,6 +71,11 @@ func runStart(args []string, stdout io.Writer) error { } bindAddr := server.DefaultHost + _, hasHost := flags["host"] + _, hasBind := flags["bind"] + if hasHost && hasBind { + fmt.Fprintln(os.Stderr, "warning: both --host and --bind specified; --host takes precedence") + } if value, ok := flags["host"]; ok { bindAddr = value } else if value, ok := flags["bind"]; ok { diff --git a/internal/server/server.go b/internal/server/server.go index 14e295b..648ff09 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -30,17 +30,16 @@ const ( maxArchiveFiles = 2048 ) -var hostnamePattern = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9.\-]*[a-zA-Z0-9])?$`) +// hostnamePattern matches valid hostnames with dot-separated labels. +// Each label starts and ends with [a-zA-Z0-9]; inner characters allow hyphens. +// Consecutive dots are inherently rejected because each label requires +// at least one alphanumeric character between dots. +var hostnamePattern = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9\-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9\-]*[a-zA-Z0-9])?)*$`) func isValidServerName(name string) bool { if ip := net.ParseIP(name); ip != nil { return ip.To4() != nil } - // Block consecutive dots explicitly; the hostname regex allows '.' in - // the character class which would otherwise match inputs like "a..b". - if strings.Contains(name, "..") { - return false - } return hostnamePattern.MatchString(name) } @@ -1327,13 +1326,15 @@ func writeJSON(w http.ResponseWriter, status int, payload any) { _ = json.NewEncoder(w).Encode(payload) } -func baseURL(r *http.Request) string { - scheme := "http" - if r.TLS != nil { - scheme = "https" +func determineScheme(r *http.Request) string { + if r.Header.Get("X-Forwarded-Proto") == "https" || r.TLS != nil { + return "https" } + return "http" +} - return scheme + "://" + r.Host +func baseURL(r *http.Request) string { + return determineScheme(r) + "://" + r.Host } func (s *Server) serverBaseURL(r *http.Request) string { @@ -1347,9 +1348,9 @@ func (s *Server) serverBaseURL(r *http.Request) string { port = addr.Port } } - scheme := "http" - if r.Header.Get("X-Forwarded-Proto") == "https" || r.TLS != nil { - scheme = "https" + scheme := determineScheme(r) + if (scheme == "http" && port == 80) || (scheme == "https" && port == 443) { + return fmt.Sprintf("%s://%s", scheme, s.serverName) } return fmt.Sprintf("%s://%s:%d", scheme, s.serverName, port) } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index d772f63..3e68b9d 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -226,6 +226,79 @@ func TestServerNameValidationAcceptsValid(t *testing.T) { } } + +func TestServerBaseURLDefaultPort(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + serverName string + port int + scheme string + want string + }{ + {"http port 80 omits port", "example.com", 80, "http", "http://example.com"}, + {"https port 443 omits port", "example.com", 443, "https", "https://example.com"}, + {"http port 8080 includes port", "example.com", 8080, "http", "http://example.com:8080"}, + {"https port 8443 includes port", "example.com", 8443, "https", "https://example.com:8443"}, + {"http port 3939 includes port", "192.168.2.14", 3939, "http", "http://192.168.2.14:3939"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store := newTestStore(t) + srv, err := New("127.0.0.1", tt.port, store, tt.serverName) + if err != nil { + t.Fatal(err) + } + + r := &http.Request{TLS: nil, Header: http.Header{}} + if tt.scheme == "https" { + r.Header.Set("X-Forwarded-Proto", "https") + } + + got := srv.serverBaseURL(r) + if got != tt.want { + t.Errorf("serverBaseURL() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestServerBaseURLFallback(t *testing.T) { + t.Parallel() + + store := newTestStore(t) + srv, err := New("127.0.0.1", 3939, store, "") + if err != nil { + t.Fatal(err) + } + + r := &http.Request{TLS: nil, Host: "127.0.0.1:3939", Header: http.Header{}} + got := srv.serverBaseURL(r) + if got != "http://127.0.0.1:3939" { + t.Errorf("serverBaseURL() fallback = %q, want http://127.0.0.1:3939", got) + } +} + +func TestServerBaseURLForwardedProto(t *testing.T) { + t.Parallel() + + store := newTestStore(t) + srv, err := New("127.0.0.1", 443, store, "secure.example.com") + if err != nil { + t.Fatal(err) + } + + r := &http.Request{TLS: nil, Host: "secure.example.com", Header: http.Header{}} + r.Header.Set("X-Forwarded-Proto", "https") + + got := srv.serverBaseURL(r) + if got != "https://secure.example.com" { + t.Errorf("serverBaseURL() with X-Forwarded-Proto = %q, want https://secure.example.com", got) + } +} + func TestCreateSessionAndServeAssets(t *testing.T) { t.Parallel() From 29391a92bbbb8b0398b2130e8c04ee803dd5fb95 Mon Sep 17 00:00:00 2001 From: Agent Date: Thu, 14 May 2026 19:27:39 +0800 Subject: [PATCH 9/9] fix: address review suggestions - doc determineScheme, deprecate --host, safe type assertion --- internal/cli/cli.go | 2 +- internal/server/server.go | 3 +++ internal/server/server_test.go | 5 ++++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index e9e5e4c..48d3517 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -74,7 +74,7 @@ func runStart(args []string, stdout io.Writer) error { _, hasHost := flags["host"] _, hasBind := flags["bind"] if hasHost && hasBind { - fmt.Fprintln(os.Stderr, "warning: both --host and --bind specified; --host takes precedence") + fmt.Fprintln(os.Stderr, "warning: --host is deprecated, use --bind instead; --host takes precedence") } if value, ok := flags["host"]; ok { bindAddr = value diff --git a/internal/server/server.go b/internal/server/server.go index 648ff09..74fc184 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -1326,6 +1326,9 @@ func writeJSON(w http.ResponseWriter, status int, payload any) { _ = json.NewEncoder(w).Encode(payload) } +// determineScheme returns the URL scheme for the request. +// It trusts the X-Forwarded-Proto header, so the server must +// run behind a trusted reverse proxy when relying on this logic. func determineScheme(r *http.Request) string { if r.Header.Get("X-Forwarded-Proto") == "https" || r.TLS != nil { return "https" diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 3e68b9d..6546a99 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -175,7 +175,10 @@ func TestServerNameUsedInSessionURL(t *testing.T) { } srv.mu.RLock() - actualAddr := srv.listener.Addr().(*net.TCPAddr) + actualAddr, ok := srv.listener.Addr().(*net.TCPAddr) + if !ok { + t.Fatalf("listener address is not TCP: %T", srv.listener.Addr()) + } srv.mu.RUnlock() actualURL := fmt.Sprintf("http://127.0.0.1:%d", actualAddr.Port)