Merge pull request 'fix: artifacts/server: properly format IP/port for listen address' (#135) from c8h4/act:fix-artifact-ip into main

Reviewed-on: https://code.forgejo.org/forgejo/act/pulls/135
Reviewed-by: earl-warren <earl-warren@noreply.code.forgejo.org>

act/artifactcache/handler.go

@@ -115,10 +115,10 @@ func StartHandler(dir, outboundIP string, port uint16, secret string, logger log
 }
 
 func (h *Handler) ExternalURL() string {
+	port := strconv.Itoa(h.listener.Addr().(*net.TCPAddr).Port)
+
 	// TODO: make the external url configurable if necessary
-	return fmt.Sprintf("http://%s:%d",
+	return fmt.Sprintf("http://%s", net.JoinHostPort(h.outboundIP, port))
-		h.outboundIP,
-		h.listener.Addr().(*net.TCPAddr).Port)
 }
 
 func (h *Handler) Close() error {

act/artifactcache/handler_test.go

@@ -746,3 +746,38 @@ func TestHandler_gcCache(t *testing.T) {
 	}
 	require.NoError(t, db.Close())
 }
+
+func TestHandler_ExternalURL(t *testing.T) {
+	t.Run("reports correct URL on IPv4", func(t *testing.T) {
+		dir := filepath.Join(t.TempDir(), "artifactcache")
+		handler, err := StartHandler(dir, "127.0.0.1", 34567, "secret", nil)
+		require.NoError(t, err)
+
+		assert.Equal(t, handler.ExternalURL(), "http://127.0.0.1:34567")
+		require.NoError(t, handler.Close())
+		assert.Nil(t, handler.server)
+		assert.Nil(t, handler.listener)
+	})
+
+	t.Run("reports correct URL on IPv6 zero host", func(t *testing.T) {
+		dir := filepath.Join(t.TempDir(), "artifactcache")
+		handler, err := StartHandler(dir, "2001:db8::", 34567, "secret", nil)
+		require.NoError(t, err)
+
+		assert.Equal(t, handler.ExternalURL(), "http://[2001:db8::]:34567")
+		require.NoError(t, handler.Close())
+		assert.Nil(t, handler.server)
+		assert.Nil(t, handler.listener)
+	})
+
+	t.Run("reports correct URL on IPv6", func(t *testing.T) {
+		dir := filepath.Join(t.TempDir(), "artifactcache")
+		handler, err := StartHandler(dir, "2001:db8::1:2:3:4", 34567, "secret", nil)
+		require.NoError(t, err)
+
+		assert.Equal(t, handler.ExternalURL(), "http://[2001:db8::1:2:3:4]:34567")
+		require.NoError(t, handler.Close())
+		assert.Nil(t, handler.server)
+		assert.Nil(t, handler.listener)
+	})
+}

act/artifacts/server.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"net"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -291,14 +292,14 @@ func Serve(ctx context.Context, artifactPath string, addr string, port string) c
 	downloads(router, artifactPath, fsys)
 
 	server := &http.Server{
-		Addr:              fmt.Sprintf("%s:%s", addr, port),
+		Addr:              net.JoinHostPort(addr, port),
 		ReadHeaderTimeout: 2 * time.Second,
 		Handler:           router,
 	}
 
 	// run server
 	go func() {
-		logger.Infof("Start server on http://%s:%s", addr, port)
+		logger.Infof("Start server on http://%s", server.Addr)
 		if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
 			logger.Fatalf("http listener: %v", err)
 		}