Преглед на файлове

fix(web): tighten database restore body-cap exemption (#5609)

n0ctal преди 14 часа
родител
ревизия
7f8cbf4c4b
променени са 3 файла, в които са добавени 19 реда и са изтрити 18 реда
  1. 4 5
      internal/web/middleware/bodylimit.go
  2. 12 10
      internal/web/middleware/bodylimit_test.go
  3. 3 3
      internal/web/web.go

+ 4 - 5
internal/web/middleware/bodylimit.go

@@ -23,7 +23,7 @@ func MaxBodyBytes(limit int64, skipSuffixes ...string) gin.HandlerFunc {
 			switch c.Request.Method {
 			case http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodTrace:
 			default:
-				if c.Request.Body != nil && !hasSuffix(c.Request.URL.Path, skipSuffixes) {
+				if c.Request.Body != nil && !hasAnySuffix(c.Request.URL.Path, skipSuffixes) {
 					c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, limit)
 				}
 			}
@@ -32,10 +32,9 @@ func MaxBodyBytes(limit int64, skipSuffixes ...string) gin.HandlerFunc {
 	}
 }
 
-// hasSuffix reports whether path ends in any of the given suffixes.
-func hasSuffix(path string, suffixes []string) bool {
-	for _, s := range suffixes {
-		if strings.HasSuffix(path, s) {
+func hasAnySuffix(path string, suffixes []string) bool {
+	for _, suffix := range suffixes {
+		if suffix != "" && strings.HasSuffix(path, suffix) {
 			return true
 		}
 	}

+ 12 - 10
internal/web/middleware/bodylimit_test.go

@@ -50,7 +50,7 @@ func TestMaxBodyBytes(t *testing.T) {
 
 func TestMaxBodyBytesSkipSuffix(t *testing.T) {
 	gin.SetMode(gin.TestMode)
-	const limit = 16
+	const limit = 10 << 20
 
 	r := gin.New()
 	r.Use(MaxBodyBytes(limit, "/server/importDB"))
@@ -61,20 +61,22 @@ func TestMaxBodyBytesSkipSuffix(t *testing.T) {
 		}
 		c.String(http.StatusOK, "ok")
 	}
-	r.POST("/server/importDB", read)
+	r.POST("/prefix/panel/api/server/importDB", read)
+	r.POST("/prefix/panel/api/server/importDB/other", read)
 	r.POST("/x", read)
 
-	// Exempt route reads an over-limit body without error.
+	large := bytes.Repeat([]byte("x"), limit+1)
 	w := httptest.NewRecorder()
-	r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/server/importDB", bytes.NewReader(make([]byte, limit*4))))
+	r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/prefix/panel/api/server/importDB", bytes.NewReader(large)))
 	if w.Code != http.StatusOK {
-		t.Errorf("exempt route should pass through over-limit body, got %d", w.Code)
+		t.Fatalf("restore route should accept an over-limit body, got %d", w.Code)
 	}
 
-	// Non-exempt route is still capped.
-	w = httptest.NewRecorder()
-	r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/x", bytes.NewReader(make([]byte, limit*4))))
-	if w.Code == http.StatusOK {
-		t.Errorf("non-exempt over-limit POST should not succeed, got 200")
+	for _, path := range []string{"/x", "/prefix/panel/api/server/importDB/other"} {
+		w = httptest.NewRecorder()
+		r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, path, bytes.NewReader(large)))
+		if w.Code == http.StatusOK {
+			t.Fatalf("non-exempt path %q accepted an over-limit body", path)
+		}
 	}
 }

+ 3 - 3
internal/web/web.go

@@ -168,9 +168,9 @@ func (s *Server) initRouter() (*gin.Engine, error) {
 	// Cap request bodies on state-changing requests so a stolen session/API
 	// token or a buggy client can't force large allocations or long DB
 	// transactions via bulk create/attach/import endpoints. GET/HEAD/OPTIONS
-	// carry no body and are left untouched. importDB restores a full SQLite
-	// backup that legitimately exceeds the cap, so it's exempt. Follow-up: make
-	// the limit a setting.
+	// carry no body and are left untouched. Database restore legitimately accepts
+	// large backups and streams them to disk, so only its exact route suffix is
+	// exempt. Follow-up: make the limit a setting.
 	const maxRequestBodyBytes = 10 << 20 // 10 MiB
 	engine.Use(middleware.MaxBodyBytes(maxRequestBodyBytes, "/panel/api/server/importDB"))