diff --git a/internal/runner/options.go b/internal/runner/options.go index 6128d8c..b60098e 100644 --- a/internal/runner/options.go +++ b/internal/runner/options.go @@ -74,7 +74,7 @@ func ParseOptions() *Options { flagSet.BoolVar(&options.Sandbox, "sandbox", false, "Enable sandbox mode"), flagSet.BoolVar(&options.HTTP1Only, "http1", false, "Enable only HTTP1"), flagSet.IntVar(&options.MaxFileSize, "max-file-size", 50, "Max Upload File Size"), - flagSet.IntVar(&options.MaxDumpBodySize, "max-dump-body-size", -1, "Max Dump Body Size"), + flagSet.IntVar(&options.MaxDumpBodySize, "max-dump-body-size", -1, "Max Dump Body Size in MB (request bodies are logged up to 1 MB when not set)"), flagSet.BoolVar(&options.Python, "py", false, "Emulate Python Style"), flagSet.BoolVar(&options.CORS, "cors", false, "Enable Cross-Origin Resource Sharing (CORS)"), flagSet.Var(&options.HTTPHeaders, "header", "Add HTTP Response Header (name: value), can be used multiple times"), diff --git a/pkg/httpserver/loglayer.go b/pkg/httpserver/loglayer.go index c4fb78d..8c16e8c 100644 --- a/pkg/httpserver/loglayer.go +++ b/pkg/httpserver/loglayer.go @@ -2,6 +2,7 @@ package httpserver import ( "bytes" + "io" "net/http" "net/http/httputil" "time" @@ -16,17 +17,40 @@ var ( EnableLogUA bool ) -func (t *HTTPServer) shouldDumpBody(bodysize int64) bool { - return t.options.MaxDumpBodySize > 0 && bodysize > t.options.MaxDumpBodySize +// defaultMaxDumpBodySize bounds the request body kept for logging when +// -max-dump-body-size is not set. +const defaultMaxDumpBodySize = 1024 * 1024 + +func (t *HTTPServer) maxDumpBodySize() int64 { + if t.options.MaxDumpBodySize > 0 { + return t.options.MaxDumpBodySize + } + return defaultMaxDumpBodySize +} + +// dumpRequest dumps r with at most maxDumpBodySize bytes of its body. The +// loglayer runs before authentication, so the size kept must not depend on +// Content-Length (absent for chunked bodies). The bytes read are put back in +// front of the body, so the next handlers still read all of it. +func (t *HTTPServer) dumpRequest(r *http.Request) []byte { + dump, _ := httputil.DumpRequest(r, false) + if r.Body == nil || r.Body == http.NoBody { + return dump + } + prefix, _ := io.ReadAll(io.LimitReader(r.Body, t.maxDumpBodySize())) + r.Body = struct { + io.Reader + io.Closer + }{io.MultiReader(bytes.NewReader(prefix), r.Body), r.Body} + return append(dump, prefix...) } func (t *HTTPServer) loglayer(handler http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // the body is read only when a log needs it var fullRequest []byte - if t.shouldDumpBody(r.ContentLength) { - fullRequest, _ = httputil.DumpRequest(r, false) - } else { - fullRequest, _ = httputil.DumpRequest(r, true) + if EnableVerbose || t.jsonLogger != nil { + fullRequest = t.dumpRequest(r) } lrw := newLoggingResponseWriter(w, t.options.MaxDumpBodySize) handler.ServeHTTP(lrw, r) diff --git a/pkg/httpserver/loglayer_test.go b/pkg/httpserver/loglayer_test.go new file mode 100644 index 0000000..afe41cb --- /dev/null +++ b/pkg/httpserver/loglayer_test.go @@ -0,0 +1,73 @@ +package httpserver + +import ( + "bytes" + "io" + "net/http" + "net/http/httptest" + "testing" +) + +type countingBody struct { + r io.Reader + read int64 +} + +func (c *countingBody) Read(p []byte) (int, error) { + n, err := c.r.Read(p) + c.read += int64(n) + return n, err +} + +func (c *countingBody) Close() error { return nil } + +// TestLogLayerBoundsChunkedBody checks that a body without Content-Length +// (chunked) is not read past the dump limit before the next handler runs, +// and that the next handler still gets the whole body. +func TestLogLayerBoundsChunkedBody(t *testing.T) { + EnableVerbose = true + defer func() { EnableVerbose = false }() + + const limit = 1024 + payload := bytes.Repeat([]byte("A"), 1<<20) + body := &countingBody{r: bytes.NewReader(payload)} + + server := &HTTPServer{options: &Options{MaxDumpBodySize: limit}} + var readBeforeHandler int64 + var got []byte + handler := server.loglayer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + readBeforeHandler = body.read + got, _ = io.ReadAll(r.Body) + w.WriteHeader(http.StatusUnauthorized) + })) + + req := httptest.NewRequest(http.MethodPost, "http://example.com/", nil) + req.Body = body + req.ContentLength = -1 + req.TransferEncoding = []string{"chunked"} + handler.ServeHTTP(httptest.NewRecorder(), req) + + if readBeforeHandler > limit { + t.Fatalf("loglayer read %d bytes before the next handler, limit %d", readBeforeHandler, limit) + } + if !bytes.Equal(got, payload) { + t.Fatalf("next handler got %d bytes, want %d", len(got), len(payload)) + } +} + +// TestLogLayerSkipsBodyWithoutLog checks that the body is not read when no +// log needs it. +func TestLogLayerSkipsBodyWithoutLog(t *testing.T) { + body := &countingBody{r: bytes.NewReader(bytes.Repeat([]byte("A"), 4096))} + server := &HTTPServer{options: &Options{}} + var readBeforeHandler int64 = -1 + handler := server.loglayer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + readBeforeHandler = body.read + })) + req := httptest.NewRequest(http.MethodPost, "http://example.com/", nil) + req.Body = body + handler.ServeHTTP(httptest.NewRecorder(), req) + if readBeforeHandler != 0 { + t.Fatalf("loglayer read %d bytes with no log enabled", readBeforeHandler) + } +}