From 49e0c36a0f0c328ba793e7ee6878e2a8eea7cd43 Mon Sep 17 00:00:00 2001 From: RaduBerinde Date: Tue, 8 Sep 2026 06:52:37 -0700 Subject: [PATCH] s3api: report the bound addresses from the listen hook `WithOnListen` tells an embedder when the S3 server is serving, but not where: the callback takes no arguments, and the server reports the addresses it bound nowhere else. An embedder that wants an ephemeral port therefore cannot ask for port 0; it has to pick a free port itself, release it, and pass it in, which loses to any other process that binds the same port in between. Add `WithOnListenAddrs`, which passes the callback the address of every listener the server bound, in port-specification order, so an embedder can serve on `127.0.0.1:0` and learn the port the kernel chose. `WithOnListen` is unchanged. `MultiListener` gains `Addrs`, the every-listener counterpart of `Addr`, to supply them. --- internal/netutil/multi_listener.go | 17 +++++++++ internal/netutil/multi_listener_test.go | 37 ++++++++++++++++++++ s3api/server.go | 22 ++++++++++-- s3api/server_test.go | 46 +++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 3 deletions(-) diff --git a/internal/netutil/multi_listener.go b/internal/netutil/multi_listener.go index 5a3ec444..045a6d7a 100644 --- a/internal/netutil/multi_listener.go +++ b/internal/netutil/multi_listener.go @@ -124,6 +124,23 @@ func (ml *MultiListener) Addr() net.Addr { return nil } +// Addrs returns the address of every underlying listener, in the order the +// listeners were given, expanding nested MultiListeners. Unlike Addr, which +// reports only the first, this covers every address the MultiListener +// accepts on -- including the ports the kernel chose for specifications +// that asked for port 0. +func (ml *MultiListener) Addrs() []net.Addr { + addrs := make([]net.Addr, 0, len(ml.listeners)) + for _, ln := range ml.listeners { + if inner, ok := ln.(*MultiListener); ok { + addrs = append(addrs, inner.Addrs()...) + continue + } + addrs = append(addrs, ln.Addr()) + } + return addrs +} + func IsUnixSocketPath(addr string) bool { _, _, err := net.SplitHostPort(addr) return err != nil diff --git a/internal/netutil/multi_listener_test.go b/internal/netutil/multi_listener_test.go index cc9b2952..e09a8d24 100644 --- a/internal/netutil/multi_listener_test.go +++ b/internal/netutil/multi_listener_test.go @@ -198,3 +198,40 @@ func issueTestCert(t *testing.T, ca testCA, cn string) tls.Certificate { PrivateKey: key, } } + +// TestMultiListenerAddrs checks that Addrs reports every bound address, +// including those behind a nested MultiListener and the ports the kernel +// chose for port 0, where Addr reports only the first. +func TestMultiListenerAddrs(t *testing.T) { + var inner []net.Listener + for range 2 { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + inner = append(inner, ln) + } + outer, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + ml := NewMultiListener(NewMultiListener(inner...), outer) + defer ml.Close() + + want := []string{inner[0].Addr().String(), inner[1].Addr().String(), outer.Addr().String()} + addrs := ml.Addrs() + if len(addrs) != len(want) { + t.Fatalf("Addrs() returned %d addresses, want %d: %v", len(addrs), len(want), addrs) + } + for i, addr := range addrs { + if addr.String() != want[i] { + t.Errorf("Addrs()[%d] = %s, want %s", i, addr, want[i]) + } + if addr.(*net.TCPAddr).Port == 0 { + t.Errorf("Addrs()[%d] = %s reports port 0, want the bound port", i, addr) + } + } + if ml.Addr().String() != want[0] { + t.Errorf("Addr() = %s, want the first address %s", ml.Addr(), want[0]) + } +} diff --git a/s3api/server.go b/s3api/server.go index fdbd30fd..48543c13 100644 --- a/s3api/server.go +++ b/s3api/server.go @@ -61,6 +61,7 @@ type S3ApiServer struct { middlewares []middlewareMount socketPerm os.FileMode onListen func() + onListenAddrs func(addrs []net.Addr) } type routeMount struct { @@ -355,6 +356,16 @@ func WithOnListen(fn func()) Option { return func(s *S3ApiServer) { s.onListen = fn } } +// WithOnListenAddrs is WithOnListen with the addresses the server bound +// passed to the callback: one per listener, in the order of the port +// specifications given to ServeMultiPort, with a specification that resolves +// to several addresses contributing one each. This is how a caller that asked +// for port 0 learns which port the kernel chose; the server does not report +// it anywhere else. +func WithOnListenAddrs(fn func(addrs []net.Addr)) Option { + return func(s *S3ApiServer) { s.onListenAddrs = fn } +} + // ServeMultiPort creates listeners for multiple port specifications and serves // on all of them simultaneously. This supports listening on multiple ports and/or // addresses (e.g., [":7070", "localhost:8080", "0.0.0.0:9090"]). @@ -389,10 +400,15 @@ func (sa *S3ApiServer) ServeMultiPort(ports []string) error { // Combine all listeners finalListener := netutil.NewMultiListener(listeners...) - if sa.onListen != nil { - fn := sa.onListen + if sa.onListen != nil || sa.onListenAddrs != nil { + fn, fnAddrs := sa.onListen, sa.onListenAddrs sa.app.Hooks().OnListen(func(fiber.ListenData) error { - fn() + if fn != nil { + fn() + } + if fnAddrs != nil { + fnAddrs(finalListener.Addrs()) + } return nil }) } diff --git a/s3api/server_test.go b/s3api/server_test.go index 0a845bb6..0c06c17f 100644 --- a/s3api/server_test.go +++ b/s3api/server_test.go @@ -15,6 +15,7 @@ package s3api import ( + "net" "net/http" "net/http/httptest" "strings" @@ -83,6 +84,51 @@ func TestS3ApiServer_Serve(t *testing.T) { } } +// TestWithOnListenAddrs serves on port 0 and checks that the listen hook +// reports the port the kernel chose, after the bind, for a listener that +// accepts connections. +func TestWithOnListenAddrs(t *testing.T) { + got := make(chan []net.Addr, 1) + sa, err := newTestS3ApiServer(WithOnListenAddrs(func(addrs []net.Addr) { got <- addrs })) + if err != nil { + t.Fatalf("New() error = %v", err) + } + served := make(chan error, 1) + go func() { served <- sa.ServeMultiPort([]string{"127.0.0.1:0"}) }() + + var addrs []net.Addr + select { + case addrs = <-got: + case err := <-served: + t.Fatalf("ServeMultiPort() returned before the listen hook fired: %v", err) + case <-time.After(10 * time.Second): + t.Fatal("listen hook did not fire") + } + if len(addrs) != 1 { + t.Fatalf("hook received %d addresses, want 1: %v", len(addrs), addrs) + } + tcp, ok := addrs[0].(*net.TCPAddr) + if !ok || tcp.Port == 0 || !tcp.IP.Equal(net.IPv4(127, 0, 0, 1)) { + t.Fatalf("hook received %v, want a 127.0.0.1 address with the bound port", addrs[0]) + } + conn, err := net.DialTimeout("tcp", tcp.String(), 5*time.Second) + if err != nil { + t.Fatalf("dialing the reported address: %v", err) + } + conn.Close() + + if err := sa.ShutDown(); err != nil { + t.Fatalf("ShutDown() error = %v", err) + } + // ServeMultiPort reports the closed listener on the way out; only that + // it returns matters here. + select { + case <-served: + case <-time.After(shutDownDuration + time.Second): + t.Fatal("ServeMultiPort() did not return after ShutDown()") + } +} + func TestWithRouteRegistersBeforeMiddleware(t *testing.T) { const routePath = "/custom/route"