mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-06-28 00:24:19 +00:00
fix(runtime): cap remote node response size to bound master memory (#5361)
Remote node HTTP responses were read with an unbounded io.ReadAll, so a
broken or hostile node could force the master panel to buffer an arbitrarily
large body. The single Remote.do choke point that all node calls funnel
through now:
- validates the HTTP status before reading any success payload (a non-OK
body is only read up to a small bounded diagnostic snippet, so a node
cannot make the master buffer a large body just to return an error);
- fast-fails on an honestly-declared oversize Content-Length;
- reads the success body through readCappedBody, an io.LimitReader cap
(64 MiB) that rejects oversize with a typed error.
The 64 MiB cap bounds one response's wire/decompressed size; it is documented
as not a process-wide memory bound (endpoint-specific caps and a concurrency
budget remain follow-ups).
Tests cover the cap+1 boundary, an oversize streamed body, a normal envelope,
and non-OK status precedence.
This commit is contained in:
@@ -28,6 +28,38 @@ const remoteHTTPTimeout = 10 * time.Second
|
||||
// overhead can outweigh the savings.
|
||||
const zstdMinBodyBytes = 1024
|
||||
|
||||
// maxRemoteResponseBytes caps a single node RPC's response body. It bounds the
|
||||
// wire/decompressed size of one response — the real guard against a broken or
|
||||
// hostile node streaming an unbounded body. It is NOT a process-wide memory
|
||||
// bound: concurrent RPCs and the decoded JSON can each exceed it, so
|
||||
// endpoint-specific caps and a concurrency budget remain follow-ups. Node
|
||||
// responses (traffic snapshots, client-IP lists, inbound options) are JSON and
|
||||
// stay well under it.
|
||||
const maxRemoteResponseBytes = 64 << 20 // 64 MiB
|
||||
|
||||
// errBodyDiagBytes bounds how much of a non-OK error body we read for a
|
||||
// diagnostic snippet (and to let small-error connections be reused) without
|
||||
// buffering a potentially huge or hostile error payload.
|
||||
const errBodyDiagBytes = 8 << 10 // 8 KiB
|
||||
|
||||
// errRemoteResponseTooLarge is returned when a node response exceeds the cap.
|
||||
var errRemoteResponseTooLarge = errors.New("remote response exceeds size limit")
|
||||
|
||||
// readCappedBody reads all of r but rejects bodies larger than limit, returning
|
||||
// errRemoteResponseTooLarge. It reads at most limit+1 bytes so a body of exactly
|
||||
// limit is accepted and the first oversize byte is detected without buffering
|
||||
// more.
|
||||
func readCappedBody(r io.Reader, limit int64) ([]byte, error) {
|
||||
raw, err := io.ReadAll(io.LimitReader(r, limit+1))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if int64(len(raw)) > limit {
|
||||
return nil, errRemoteResponseTooLarge
|
||||
}
|
||||
return raw, nil
|
||||
}
|
||||
|
||||
type envelope struct {
|
||||
Success bool `json:"success"`
|
||||
Msg string `json:"msg"`
|
||||
@@ -203,14 +235,34 @@ func (r *Remote) do(ctx context.Context, method, path string, body any) (*envelo
|
||||
defer resp.Body.Close()
|
||||
r.recordCaps(resp.Header)
|
||||
|
||||
raw, err := io.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("read body: %w", err)
|
||||
}
|
||||
// Validate status before reading a success payload: a non-OK response's
|
||||
// body is never used beyond a short diagnostic, so don't let a node force us
|
||||
// to buffer a large body just to return an HTTP error.
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
snippet, _ := io.ReadAll(io.LimitReader(resp.Body, errBodyDiagBytes))
|
||||
if msg := bytes.TrimSpace(snippet); len(msg) > 0 {
|
||||
// %q quotes/escapes the untrusted node body so control characters or
|
||||
// newlines in it can't garble or inject into the error/log output.
|
||||
return nil, fmt.Errorf("%s %s: HTTP %d: %q", method, path, resp.StatusCode, msg)
|
||||
}
|
||||
return nil, fmt.Errorf("%s %s: HTTP %d", method, path, resp.StatusCode)
|
||||
}
|
||||
|
||||
// Fast-fail on an honestly-declared oversize body; the LimitReader below is
|
||||
// the real guard since Content-Length is untrusted, may be absent, or is -1
|
||||
// under transparent decompression.
|
||||
if resp.ContentLength > maxRemoteResponseBytes {
|
||||
return nil, fmt.Errorf("%s %s: %w (content-length %d, cap %d)", method, path, errRemoteResponseTooLarge, resp.ContentLength, maxRemoteResponseBytes)
|
||||
}
|
||||
|
||||
raw, err := readCappedBody(resp.Body, maxRemoteResponseBytes)
|
||||
if err != nil {
|
||||
if errors.Is(err, errRemoteResponseTooLarge) {
|
||||
return nil, fmt.Errorf("%s %s: %w (cap %d bytes)", method, path, err, maxRemoteResponseBytes)
|
||||
}
|
||||
return nil, fmt.Errorf("read body: %w", err)
|
||||
}
|
||||
|
||||
var env envelope
|
||||
if err := json.Unmarshal(raw, &env); err != nil {
|
||||
return nil, fmt.Errorf("decode envelope: %w", err)
|
||||
|
||||
@@ -1,16 +1,112 @@
|
||||
package runtime
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
|
||||
)
|
||||
|
||||
// TestRemoteDo_RejectsOversizeResponse: a node streaming a body larger than
|
||||
// maxRemoteResponseBytes must error out instead of the master buffering it
|
||||
// unbounded.
|
||||
func TestRemoteDo_RejectsOversizeResponse(t *testing.T) {
|
||||
srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
chunk := bytes.Repeat([]byte("a"), 1<<20) // 1 MiB
|
||||
for written := 0; written <= maxRemoteResponseBytes; written += len(chunk) {
|
||||
if _, err := w.Write(chunk); err != nil {
|
||||
return // client stopped reading at the cap
|
||||
}
|
||||
}
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
r := NewRemote(nodeForServer(t, srv, "skip", ""), nil)
|
||||
if _, err := r.do(context.Background(), http.MethodGet, "/probe", nil); !errors.Is(err, errRemoteResponseTooLarge) {
|
||||
t.Fatalf("do() error = %v, want errRemoteResponseTooLarge", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRemoteDo_AcceptsNormalResponse confirms the cap does not break a normal
|
||||
// under-limit envelope.
|
||||
func TestRemoteDo_AcceptsNormalResponse(t *testing.T) {
|
||||
srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(`{"success":true,"msg":"ok","obj":{"x":1}}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
r := NewRemote(nodeForServer(t, srv, "skip", ""), nil)
|
||||
env, err := r.do(context.Background(), http.MethodGet, "/probe", nil)
|
||||
if err != nil {
|
||||
t.Fatalf("do() unexpected error: %v", err)
|
||||
}
|
||||
if env == nil || !env.Success {
|
||||
t.Fatalf("env = %+v, want Success=true", env)
|
||||
}
|
||||
}
|
||||
|
||||
// TestReadCappedBody_Boundary pins the cap+1 contract cheaply (no large allocs):
|
||||
// a body of exactly limit is accepted; limit+1 and beyond are rejected.
|
||||
func TestReadCappedBody_Boundary(t *testing.T) {
|
||||
const limit = 8
|
||||
cases := []struct {
|
||||
name string
|
||||
n int
|
||||
wantErr bool
|
||||
}{
|
||||
{"under", limit - 1, false},
|
||||
{"exact", limit, false},
|
||||
{"over-by-one", limit + 1, true},
|
||||
{"way-over", limit * 4, true},
|
||||
}
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
raw, err := readCappedBody(bytes.NewReader(bytes.Repeat([]byte("x"), c.n)), limit)
|
||||
if c.wantErr {
|
||||
if !errors.Is(err, errRemoteResponseTooLarge) {
|
||||
t.Fatalf("n=%d: err=%v, want errRemoteResponseTooLarge", c.n, err)
|
||||
}
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatalf("n=%d: unexpected err %v", c.n, err)
|
||||
}
|
||||
if len(raw) != c.n {
|
||||
t.Fatalf("n=%d: read %d bytes, want %d", c.n, len(raw), c.n)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestRemoteDo_NonOKStatusReturnsHTTPError confirms a non-OK status is reported
|
||||
// as an HTTP error (with a bounded diagnostic snippet) rather than being read as
|
||||
// a success payload — i.e. status precedence over the body.
|
||||
func TestRemoteDo_NonOKStatusReturnsHTTPError(t *testing.T) {
|
||||
srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
_, _ = w.Write([]byte("boom"))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
r := NewRemote(nodeForServer(t, srv, "skip", ""), nil)
|
||||
_, err := r.do(context.Background(), http.MethodGet, "/probe", nil)
|
||||
if err == nil {
|
||||
t.Fatal("do() error = nil, want HTTP 500 error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "HTTP 500") || !strings.Contains(err.Error(), "boom") {
|
||||
t.Fatalf("error = %q, want it to mention HTTP 500 and the body snippet", err)
|
||||
}
|
||||
}
|
||||
|
||||
type stubEgress struct{ url string }
|
||||
|
||||
func (s stubEgress) NodeEgressProxyURL(int) string { return s.url }
|
||||
|
||||
Reference in New Issue
Block a user