fix: protect shared rules across delivery and recovery (#560)

go-gost/x/api/api.go

@@ -52,7 +52,7 @@ func Register(r *gin.Engine, opts *Options) {
 	router.StaticFS("/docs", http.FS(swaggerDoc))
 
 	config := router.Group("/config")
-	config.Use(mwBasicAuth(opts.Auther))
+	config.Use(mwBasicAuth(opts.Auther), configTransaction())
 
 	config.GET("", getConfig)
 	config.POST("", saveConfig)

go-gost/x/api/config_reload.go

@@ -37,9 +37,12 @@ func reloadConfig(ctx *gin.Context) {
 		return
 	}
 
-	config.Set(cfg)
-
-	if err := loader.Load(cfg); err != nil {
+	if err := loader.Reload(cfg, func(*config.Config) error {
+		for _, svc := range registry.ServiceRegistry().GetAll() {
+			go svc.Serve()
+		}
+		return nil
+	}); err != nil {
 		writeError(ctx, NewError(http.StatusBadRequest, ErrCodeInvalid, err.Error()))
 		return
 	}
@@ -51,13 +54,6 @@ func reloadConfig(ctx *gin.Context) {
 	}
 	xservice.GetGlobalTrafficManager().RetainServices(activeServices)
 
-	for _, svc := range registry.ServiceRegistry().GetAll() {
-		svc := svc
-		go func() {
-			svc.Serve()
-		}()
-	}
-
 	ctx.JSON(http.StatusOK, Response{
 		Msg: "OK",
 	})

go-gost/x/api/config_transaction.go

@@ -0,0 +1,90 @@
+package api
+
+import (
+	"bytes"
+	"errors"
+	"io"
+	"net/http"
+
+	"github.com/gin-gonic/gin"
+	"github.com/go-gost/x/config"
+)
+
+const maxConfigRequestBody = 16 << 20
+
+// Read the complete bounded request before acquiring the runtime transaction
+// lock. Buffer the response until after it is released: neither a slow upload
+// nor a client that stops reading may block panel commands, reload or shutdown.
+func configTransaction() gin.HandlerFunc {
+	return func(ctx *gin.Context) {
+		if ctx.Request.Body != nil {
+			body := http.MaxBytesReader(ctx.Writer, ctx.Request.Body, maxConfigRequestBody)
+			data, err := io.ReadAll(body)
+			body.Close()
+			if err != nil {
+				status := http.StatusBadRequest
+				var tooLarge *http.MaxBytesError
+				if errors.As(err, &tooLarge) {
+					status = http.StatusRequestEntityTooLarge
+				}
+				ctx.AbortWithStatusJSON(status, Response{Code: status, Msg: "Unable to read configuration request"})
+				return
+			}
+			ctx.Request.Body = io.NopCloser(bytes.NewReader(data))
+		}
+
+		writer := ctx.Writer
+		buffered := &configResponseWriter{ResponseWriter: writer, header: writer.Header().Clone(), status: http.StatusOK, size: -1}
+		ctx.Writer = buffered
+		defer func() { ctx.Writer = writer }()
+		func() {
+			unlock := config.LockMutation()
+			defer unlock()
+			// A request waiting behind reload may have been closed during shutdown.
+			if ctx.Request.Context().Err() != nil {
+				ctx.Abort()
+				return
+			}
+			ctx.Next()
+		}()
+		ctx.Writer = writer
+		for key, values := range buffered.header {
+			writer.Header()[key] = values
+		}
+		writer.WriteHeader(buffered.status)
+		writer.Write(buffered.body.Bytes())
+	}
+}
+
+// Config endpoints return JSON rather than streaming. Preserve Gin's response
+// bookkeeping while delaying all network writes until the transaction ends.
+type configResponseWriter struct {
+	gin.ResponseWriter
+	header http.Header
+	body   bytes.Buffer
+	status int
+	size   int
+}
+
+func (w *configResponseWriter) Header() http.Header { return w.header }
+func (w *configResponseWriter) WriteHeader(status int) {
+	if !w.Written() && status > 0 {
+		w.status = status
+	}
+}
+func (w *configResponseWriter) WriteHeaderNow() {
+	if !w.Written() {
+		w.size = 0
+	}
+}
+func (w *configResponseWriter) Write(p []byte) (int, error) {
+	w.WriteHeaderNow()
+	n, err := w.body.Write(p)
+	w.size += n
+	return n, err
+}
+func (w *configResponseWriter) WriteString(s string) (int, error) { return w.Write([]byte(s)) }
+func (w *configResponseWriter) Status() int                       { return w.status }
+func (w *configResponseWriter) Size() int                         { return w.size }
+func (w *configResponseWriter) Written() bool                     { return w.size >= 0 }
+func (w *configResponseWriter) Flush()                            { w.WriteHeaderNow() }

go-gost/x/api/config_transaction_test.go

@@ -0,0 +1,76 @@
+package api
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/gin-gonic/gin"
+	"github.com/go-gost/x/config"
+)
+
+func assertMutationAvailable(t *testing.T) {
+	t.Helper()
+	done := make(chan struct{})
+	go func() { unlock := config.LockMutation(); unlock(); close(done) }()
+	select {
+	case <-done:
+	case <-time.After(time.Second):
+		t.Fatal("network I/O holds runtime mutation lock")
+	}
+}
+
+func TestConfigTransactionDoesNotLockWhileReadingBody(t *testing.T) {
+	router := gin.New()
+	router.Use(configTransaction())
+	router.POST("/config", func(c *gin.Context) { c.JSON(http.StatusOK, Response{Msg: "OK"}) })
+	reader, writer := io.Pipe()
+	defer reader.Close()
+	defer writer.Close()
+	request := httptest.NewRequest(http.MethodPost, "/config", reader)
+	done := make(chan struct{})
+	go func() { defer close(done); router.ServeHTTP(httptest.NewRecorder(), request) }()
+	if _, err := writer.Write([]byte("{")); err != nil {
+		t.Fatal(err)
+	}
+	assertMutationAvailable(t)
+	writer.Close()
+	select {
+	case <-done:
+	case <-time.After(time.Second):
+		t.Fatal("handler did not finish")
+	}
+}
+
+type blockedResponse struct {
+	header  http.Header
+	started chan struct{}
+	release chan struct{}
+}
+
+func (w *blockedResponse) Header() http.Header { return w.header }
+func (w *blockedResponse) WriteHeader(int)     {}
+func (w *blockedResponse) Write(p []byte) (int, error) {
+	close(w.started)
+	<-w.release
+	return len(p), nil
+}
+
+func TestConfigTransactionReleasesLockBeforeSendingResponse(t *testing.T) {
+	router := gin.New()
+	router.Use(configTransaction())
+	router.POST("/config", func(c *gin.Context) { c.JSON(http.StatusOK, Response{Msg: "OK"}) })
+	writer := &blockedResponse{header: make(http.Header), started: make(chan struct{}), release: make(chan struct{})}
+	defer close(writer.release)
+	request := httptest.NewRequest(http.MethodPost, "/config", strings.NewReader("{}"))
+	go router.ServeHTTP(writer, request)
+	select {
+	case <-writer.started:
+	case <-time.After(time.Second):
+		t.Fatal("response did not start")
+	}
+	assertMutationAvailable(t)
+}

go-gost/x/api/service/service.go

@@ -3,6 +3,7 @@ package service
 import (
 	"net"
 	"net/http"
+	"time"
 
 	"github.com/gin-gonic/gin"
 	"github.com/go-gost/core/auth"
@@ -67,7 +68,11 @@ func NewService(network, addr string, opts ...Option) (service.Service, error) {
 
 	return &server{
 		s: &http.Server{
-			Handler: r,
+			Handler:           r,
+			ReadHeaderTimeout: 5 * time.Second,
+			ReadTimeout:       15 * time.Second,
+			WriteTimeout:      30 * time.Second,
+			IdleTimeout:       60 * time.Second,
 		},
 		ln:     ln,
 		cclose: make(chan struct{}),
@@ -83,7 +88,11 @@ func (s *server) Addr() net.Addr {
 }
 
 func (s *server) Close() error {
-	return s.s.Close()
+	// Close can race the goroutine entering Serve during a failed startup.
+	// http.Server.Close alone does not own the listener until Serve starts.
+	err := s.s.Close()
+	s.ln.Close()
+	return err
 }
 
 func (s *server) IsClosed() bool {