From 7e3b7229614f50a15f79e015fe914e06c751b39a Mon Sep 17 00:00:00 2001 From: Pascal Bleser Date: Wed, 8 Jul 2026 17:17:49 +0200 Subject: [PATCH] groupware: move plain http functions to pkg/http * introduce a new toplevel package pkg/http for plain HTTP related functions * move these functions from pkg/jmap to pkg/http, including tests: - PeekResponse - DumpHttpRequest - DumpHttpResponse --- pkg/http/http.go | 169 ++++++++++++++++++++++++++++++++++++++++++ pkg/http/http_test.go | 92 +++++++++++++++++++++++ pkg/jmap/http.go | 40 +++------- pkg/jmap/http_test.go | 45 +---------- pkg/jmap/tools.go | 142 ----------------------------------- 5 files changed, 273 insertions(+), 215 deletions(-) create mode 100644 pkg/http/http.go create mode 100644 pkg/http/http_test.go diff --git a/pkg/http/http.go b/pkg/http/http.go new file mode 100644 index 0000000000..1f78137ba4 --- /dev/null +++ b/pkg/http/http.go @@ -0,0 +1,169 @@ +// Provides common utility functions for HTTP. +package http + +import ( + "bytes" + "fmt" + "io" + "net/http" +) + +type readCloser struct { + io.Reader + closer func() error +} + +func (c readCloser) Close() error { + if c.closer != nil { + return c.closer() + } + return nil +} + +func combinedReader(peekBuffer *bytes.Buffer, body io.Reader) io.Reader { + if body != nil && body != http.NoBody { + if peekBuffer != nil { + return io.MultiReader(peekBuffer, body) + } else { + return body + } + } else { + if peekBuffer != nil { + return peekBuffer + } else { + return http.NoBody + } + } +} + +// Reads up to amount of bytes from the response, and returns a ReadCloser that concatenates over those +// bytes as well as the rest of the bytes streamed in , to ensure that the whole response can still be +// read. +// +// Note that if is 0, then this function does nothing. +// +// Furthermore, this function never closes the Reader, that is left to the caller. +// +// # Parameters +// - body: the HTTP response body, which may be nil or http.NoBody +// - size: the number of bytes that will be read from the beginning of the response body, +// which may be 0 (in which case this function won't do anything) +// +// # Return values +// - a Reader that provides the full response, including the first bytes that were peeked +// up to bytes of the beginning of the response body +// - the first bytes of the response +// - a boolean: whether the peeked bytes contain the full response or whether it's truncated +// - an error if an error occured while reading the body response +func PeekResponse(body io.ReadCloser, size int64) (io.ReadCloser, []byte, bool, error) { + var peekBuffer *bytes.Buffer = nil + truncated := false + peek := []byte{} + if body != nil && body != http.NoBody && size > 0 { + peekBuffer = new(bytes.Buffer) + limitedReader := io.LimitReader(body, size) + tee := io.TeeReader(limitedReader, peekBuffer) + if _, err := io.Copy(io.Discard, tee); err != nil { + return nil, peek, false, err + } + + if int64(peekBuffer.Len()) >= size { + truncated = true + } + + peek = peekBuffer.Bytes() + + return readCloser{ + closer: body.Close, + Reader: combinedReader(peekBuffer, body), + }, peek, truncated, nil + } else { + return body, peek, truncated, nil + } +} + +// Extracts information from an HTTP request and passes it to a function, typically for logging. +// +// It also extracts the first bytes of the request body, if maxBodySize is > 0. +func DumpHttpRequest(req *http.Request, maxBodySize int64, closure func(method string, uri string, content string, truncated bool)) error { + var logBuilder bytes.Buffer + uri := "" + if req.URL != nil { + uri = req.URL.String() + } + fmt.Fprintf(&logBuilder, "%s %s\n", req.Method, uri) + for key, values := range req.Header { + if key == "Authorization" { + continue + } + for _, value := range values { + fmt.Fprintf(&logBuilder, "%s: %s\n", key, value) + } + } + truncated := false + if maxBodySize >= 0 && req.Body != nil && req.Body != http.NoBody { + peekBuffer := new(bytes.Buffer) + limitedReader := io.LimitReader(req.Body, maxBodySize) + tee := io.TeeReader(limitedReader, peekBuffer) + if _, err := io.Copy(io.Discard, tee); err != nil { + return fmt.Errorf("failed to peek at the request body for tracing: %v", err) + } else { + logBuilder.Write(peekBuffer.Bytes()) + if int64(peekBuffer.Len()) >= maxBodySize { + truncated = true + } + fullBodyReader := io.MultiReader(peekBuffer, req.Body) + req.Body = io.NopCloser(fullBodyReader) + } + } + closure(req.Method, uri, logBuilder.String(), truncated) + return nil +} + +// Extracts information from an HTTP response and passes it to a function, typically for logging. +// +// It also extracts the first bytes of the response body, if maxBodySize is > 0. +func DumpHttpResponse(resp *http.Response, body io.ReadCloser, maxBodySize int64, + closure func(method string, uri string, content string, truncated bool), +) (io.ReadCloser, error) { + var logBuilder bytes.Buffer + fmt.Fprintf(&logBuilder, "%s\n", resp.Status) + for key, values := range resp.Header { + for _, value := range values { + fmt.Fprintf(&logBuilder, "%s: %s\n", key, value) + } + } + + var peekBuffer *bytes.Buffer = nil + truncated := false + if body != nil && body != http.NoBody && maxBodySize > 0 { + peekBuffer = new(bytes.Buffer) + limitedReader := io.LimitReader(body, maxBodySize) + tee := io.TeeReader(limitedReader, peekBuffer) + if _, err := io.Copy(io.Discard, tee); err != nil { + return nil, err + } + + if int64(peekBuffer.Len()) >= maxBodySize { + truncated = true + } + + logBuilder.Write(peekBuffer.Bytes()) + } + + req := resp.Request + if req != nil { + closure(req.Method, req.URL.String(), logBuilder.String(), truncated) + } else { + closure("", "", logBuilder.String(), truncated) + } + + if peekBuffer != nil { + return readCloser{ + closer: body.Close, + Reader: combinedReader(peekBuffer, body), + }, nil + } else { + return body, nil + } +} diff --git a/pkg/http/http_test.go b/pkg/http/http_test.go new file mode 100644 index 0000000000..bd242df56a --- /dev/null +++ b/pkg/http/http_test.go @@ -0,0 +1,92 @@ +package http + +import ( + "fmt" + "io" + "net/http" + "net/url" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPeekResponse(t *testing.T) { + require := require.New(t) + n := 6 + text := "hello, world" + + require.Less(n, len(text)) + body, peek, truncated, err := PeekResponse(io.NopCloser(strings.NewReader(text)), int64(n)) + require.NoError(err) + require.True(truncated) + require.Equal(text[:n], string(peek)) + buf := new(strings.Builder) + _, err = io.Copy(buf, body) + require.NoError(err) + require.Equal(text, buf.String()) +} + +func TestDumpHttpResponse(t *testing.T) { + require := require.New(t) + n := 6 + text := "hello, world" + + h := http.Header{} + h.Set("testing", "true") + h.Add("aaa", "1") + h.Add("aaa", "2") + + resp := &http.Response{ + Status: "200 OK", + StatusCode: 200, + Header: h, + Body: nil, + } + + body, err := DumpHttpResponse(resp, io.NopCloser(strings.NewReader(text)), int64(n), func(method, uri, content string, truncated bool) { + require.True(truncated) + require.Equal(fmt.Sprintf(`200 OK +Testing: true +Aaa: 1 +Aaa: 2 +%s`, text[:n]), content) + }) + require.NoError(err) + buf := new(strings.Builder) + _, err = io.Copy(buf, body) + require.NoError(err) + require.Equal(text, buf.String()) +} + +func TestDumpHttpRequest(t *testing.T) { + require := require.New(t) + n := 6 + text := "hello, world" + + h := http.Header{} + h.Set("testing", "true") + h.Add("aaa", "1") + h.Add("aaa", "2") + + req := &http.Request{ + Method: "GET", + Header: h, + URL: &url.URL{Scheme: "https", Host: "example.com", Path: "/testing"}, + Body: io.NopCloser(strings.NewReader(text)), + } + + err := DumpHttpRequest(req, int64(n), func(method, uri, content string, truncated bool) { + require.True(truncated) + require.Equal(fmt.Sprintf(`GET https://example.com/testing +Testing: true +Aaa: 1 +Aaa: 2 +%s`, text[:n]), content) + }) + require.NoError(err) + buf := new(strings.Builder) + _, err = io.Copy(buf, req.Body) + require.NoError(err) + require.Equal(text, buf.String()) +} diff --git a/pkg/jmap/http.go b/pkg/jmap/http.go index f576d19cec..8910f5e676 100644 --- a/pkg/jmap/http.go +++ b/pkg/jmap/http.go @@ -17,6 +17,7 @@ import ( "time" "github.com/gorilla/websocket" + ochttp "github.com/opencloud-eu/opencloud/pkg/http" "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/pkg/version" ) @@ -195,7 +196,7 @@ var ( func (h *HttpJmapClient) beforeRequest(_ context.Context, logger *log.Logger, _ string, endpoint string, req *http.Request) Error { l := logger.Trace() if h.traceRequests && l.Enabled() { - if err := dumpHttpRequest(req, h.traceMaxRequestBodySize, func(method string, uri string, content string, truncated bool) { + if err := ochttp.DumpHttpRequest(req, h.traceMaxRequestBodySize, func(method string, uri string, content string, truncated bool) { if truncated { l = l.Bool(logBodyTruncated, true) } @@ -209,39 +210,27 @@ func (h *HttpJmapClient) beforeRequest(_ context.Context, logger *log.Logger, _ return nil } -type readCloser struct { - io.Reader - closer io.ReadCloser -} - -func (c readCloser) Close() error { - if c.closer != nil { - return c.closer.Close() - } - return nil -} - -func (h *HttpJmapClient) response(peekSize int64, _ context.Context, logger *log.Logger, _ string, endpoint string, duration time.Duration, req *http.Request, resp *http.Response) (io.ReadCloser, []byte, bool, error) { +func (h *HttpJmapClient) response(peekSize int64, _ context.Context, logger *log.Logger, _ string, endpoint string, duration time.Duration, resp *http.Response) (io.ReadCloser, []byte, bool, error) { whole := resp.Body peek := []byte{} truncated := false var err error if peekSize > 0 { - whole, peek, truncated, err = peekResponse(resp.Body, peekSize, func(err error) { logger.Error().Err(err).Msg("failed to close response body") }) //NOSONAR + whole, peek, truncated, err = ochttp.PeekResponse(resp.Body, peekSize) if err != nil { return whole, peek, truncated, err } } l := logger.Trace() if h.traceResponses && l.Enabled() { - body, err := dumpHttpResponse(req, resp, whole, h.traceMaxResponseBodySize, func(method, uri, content string, truncated bool) { + body, err := ochttp.DumpHttpResponse(resp, whole, h.traceMaxResponseBodySize, func(method, uri, content string, truncated bool) { l.Str(logProto, logProtoJmap).Str(logType, logTypeResponse).Str(logEndpoint, endpoint). Str(logMethod, method).Str(logUri, uri). Str(logHttpStatus, log.SafeString(resp.Status)).Int(logHttpStatusCode, resp.StatusCode). Dur(logDuration, duration). Msg(content) - }, func(err error) { logger.Error().Err(err).Msg("failed to close response body") }) //NOSONAR + }) if err != nil { return body, peek, truncated, err } else { @@ -296,7 +285,7 @@ func (h *HttpJmapClient) GetSession(ctx context.Context, sessionUrl *url.URL, us } // dump the response regardless of the status code - body, _, _, err := h.response(responseBodyPeekSize, ctx, logger, username, endpoint, duration, req, res) + body, _, _, err := h.response(responseBodyPeekSize, ctx, logger, username, endpoint, duration, res) // since we are not returning a stream from this function, we have to close the response body // before leaving the scope of the function @@ -383,7 +372,7 @@ func (h *HttpJmapClient) Command(operation Operation, request Request, ctx Conte // caller won't be able to read the response; instead, it is up to the caller to close the // ReadCloser we are returning from here - body, _, _, err := h.response(responseBodyPeekSize, cotx, logger, session.Username, endpoint, duration, req, res) + body, _, _, err := h.response(responseBodyPeekSize, cotx, logger, session.Username, endpoint, duration, res) language := Language(res.Header.Get("Content-Language")) //NOSONAR if err != nil { logger.Error().Err(err).Msg("failed to read response body") @@ -442,12 +431,7 @@ func (h *HttpJmapClient) UploadBinary(uploadUrl string, operation Operation, end return UploadedBlob{}, "", jmapError(err, JmapErrorSendingRequest) } - // note that we are omitting the usual deferred response body closer here since we are returning - // an io.ReadCloser to the body and if we close it when leaving the scope of this function, the - // caller won't be able to read the response; instead, it is up to the caller to close the - // ReadCloser we are returning from here - - responseBody, _, _, err := h.response(responseBodyPeekSize, cotx, logger, session.Username, endpoint, duration, req, res) + responseBody, _, _, err := h.response(responseBodyPeekSize, cotx, logger, session.Username, endpoint, duration, res) // since we are not returning a stream from this function, we have to close the response body // before leaving the scope of the function @@ -519,7 +503,7 @@ func (h *HttpJmapClient) DownloadBinary(downloadUrl string, operation Operation, // caller won't be able to read the response; instead, it is up to the caller to close the // ReadCloser we are returning from here - responseBody, _, _, err := h.response(responseBodyPeekSize, cotx, logger, session.Username, endpoint, duration, req, res) + responseBody, _, _, err := h.response(responseBodyPeekSize, cotx, logger, session.Username, endpoint, duration, res) language := Language(res.Header.Get("Content-Language")) if err != nil { @@ -656,14 +640,12 @@ func (w *HttpWsClientFactory) connect(operation Operation, ctx context.Context, { l := logger.Trace() if w.traceWsResponses && l.Enabled() { - body, err = dumpHttpResponse(nil, res, res.Body, w.traceMaxResponseBodySize, func(method, uri, content string, truncated bool) { + body, err = ochttp.DumpHttpResponse(res, res.Body, w.traceMaxResponseBodySize, func(method, uri, content string, truncated bool) { l.Str(logProto, logProtoJmap).Str(logType, logTypeResponse).Str(logEndpoint, endpoint). Str(logMethod, method).Str(logUri, uri). Str(logHttpStatus, log.SafeString(res.Status)).Int(logHttpStatusCode, res.StatusCode). Dur(logDuration, duration). Msg(content) - }, func(err error) { - logger.Error().Err(err).Msg("failed to close response body") //NOSONAR }) } } diff --git a/pkg/jmap/http_test.go b/pkg/jmap/http_test.go index a8e1d4a534..362fd237b9 100644 --- a/pkg/jmap/http_test.go +++ b/pkg/jmap/http_test.go @@ -13,49 +13,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestPeekResponse(t *testing.T) { - require := require.New(t) - n := 6 - text := "hello, world" - - require.Less(n, len(text)) - body, peek, truncated, err := peekResponse(io.NopCloser(strings.NewReader(text)), int64(n), func(err error) { - require.NoError(err) - }) - require.NoError(err) - require.True(truncated) - require.Equal(text[:n], string(peek)) - buf := new(strings.Builder) - _, err = io.Copy(buf, body) - require.NoError(err) - require.Equal(text, buf.String()) -} - -func TestDumpHttpResponse(t *testing.T) { - require := require.New(t) - n := 6 - text := "hello, world" - - resp := &http.Response{ - Status: "200 OK", - StatusCode: 200, - Header: http.Header{}, - Body: nil, - } - - body, err := dumpHttpResponse(nil, resp, io.NopCloser(strings.NewReader(text)), int64(n), func(method, uri, content string, truncated bool) { - require.True(truncated) - require.Equal(fmt.Sprintf("200 OK\n%s", text[:n]), content) - }, func(err error) { - require.NoError(err) - }) - require.NoError(err) - buf := new(strings.Builder) - _, err = io.Copy(buf, body) - require.NoError(err) - require.Equal(text, buf.String()) -} - type testNopReadCloser struct { io.Reader closed bool @@ -119,7 +76,7 @@ func TestResponse(t *testing.T) { traceResponses: true, traceMaxResponseBodySize: int64(tr.traceSize), } - body, peek, truncated, err := client.response(int64(tt.peekSize), context.TODO(), &logger, "username", "https://stalwart", time.Duration(1*time.Second), nil, resp) + body, peek, truncated, err := client.response(int64(tt.peekSize), context.TODO(), &logger, "username", "https://stalwart", time.Duration(1*time.Second), resp) require.NoError(err) if b.withBody { diff --git a/pkg/jmap/tools.go b/pkg/jmap/tools.go index 720e6f3285..3890774c42 100644 --- a/pkg/jmap/tools.go +++ b/pkg/jmap/tools.go @@ -1,12 +1,9 @@ package jmap import ( - "bytes" "encoding/json" "errors" "fmt" - "io" - "net/http" "reflect" "slices" "strings" @@ -450,142 +447,3 @@ func ns(namespaces ...JmapNamespace) []JmapNamespace { func ptr[T any | int | uint | bool | string](t T) *T { return &t } - -type nullReader struct { -} - -func (r nullReader) Read(p []byte) (n int, err error) { - return 0, nil -} - -func (r nullReader) Close() error { - return nil -} - -var _ io.ReadCloser = nullReader{} - -func multiReader(peekBuffer *bytes.Buffer, body io.Reader) io.Reader { - readers := []io.Reader{} - if peekBuffer != nil { - readers = append(readers, peekBuffer) - } - if body != nil && body != http.NoBody { - readers = append(readers, body) - } - switch len(readers) { - case 0: - return http.NoBody - case 1: - return readers[0] - default: - return io.MultiReader(readers...) - } -} - -func peekResponse(body io.ReadCloser, size int64, - closeErrorClosure func(err error)) (io.ReadCloser, []byte, bool, error) { - var peekBuffer *bytes.Buffer = nil - truncated := false - peek := []byte{} - if body != nil && body != http.NoBody && size > 0 { - peekBuffer = new(bytes.Buffer) - limitedReader := io.LimitReader(body, size) - tee := io.TeeReader(limitedReader, peekBuffer) - if _, err := io.Copy(io.Discard, tee); err != nil { - if err := body.Close(); err != nil { - closeErrorClosure(err) - } - return nil, peek, false, err - } - - if int64(peekBuffer.Len()) >= size { - truncated = true - } - - peek = peekBuffer.Bytes() - } - - return readCloser{ - closer: body, - Reader: multiReader(peekBuffer, body), - }, peek, truncated, nil -} - -func dumpHttpRequest(req *http.Request, maxBodySize int64, closure func(method string, uri string, content string, truncated bool)) error { - var logBuilder bytes.Buffer - uri := req.URL.String() - fmt.Fprintf(&logBuilder, "%s %s\n", req.Method, uri) - for key, values := range req.Header { - if key == "Authorization" { - continue - } - for _, value := range values { - fmt.Fprintf(&logBuilder, "%s: %s\n", key, value) - } - } - truncated := false - if maxBodySize >= 0 && req.Body != nil && req.Body != http.NoBody { - peekBuffer := new(bytes.Buffer) - limitedReader := io.LimitReader(req.Body, maxBodySize) - tee := io.TeeReader(limitedReader, peekBuffer) - if _, err := io.Copy(io.Discard, tee); err != nil { - return fmt.Errorf("failed to peek at the request body for tracing: %v", err) - } else { - logBuilder.Write(peekBuffer.Bytes()) - if int64(peekBuffer.Len()) >= maxBodySize { - truncated = true - } - fullBodyReader := io.MultiReader(peekBuffer, req.Body) - req.Body = io.NopCloser(fullBodyReader) - } - } - closure(req.Method, uri, logBuilder.String(), truncated) - return nil -} - -func dumpHttpResponse(req *http.Request, resp *http.Response, body io.ReadCloser, maxBodySize int64, - closure func(method string, uri string, content string, truncated bool), - closeErrorClosure func(err error)) (io.ReadCloser, error) { - var logBuilder bytes.Buffer - fmt.Fprintf(&logBuilder, "%s\n", resp.Status) - for key, values := range resp.Header { - for _, value := range values { - fmt.Fprintf(&logBuilder, "%s: %s\n", key, value) - } - } - - var peekBuffer *bytes.Buffer = nil - truncated := false - if body != nil && body != http.NoBody && maxBodySize > 0 { - peekBuffer = new(bytes.Buffer) - limitedReader := io.LimitReader(body, maxBodySize) - tee := io.TeeReader(limitedReader, peekBuffer) - if _, err := io.Copy(io.Discard, tee); err != nil { - if err := body.Close(); err != nil { - closeErrorClosure(err) - } - return nil, err - } - - if int64(peekBuffer.Len()) >= maxBodySize { - truncated = true - } - - logBuilder.Write(peekBuffer.Bytes()) - } - - if req != nil { - closure(req.Method, req.URL.String(), logBuilder.String(), truncated) - } else { - closure("", "", logBuilder.String(), truncated) - } - - if peekBuffer != nil { - return readCloser{ - closer: body, - Reader: multiReader(peekBuffer, body), - }, nil - } else { - return body, nil - } -}