From 2e2451f8ec60e166e2f255a76e7dc38078bf0e91 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Fri, 24 Mar 2023 10:05:27 +0000 Subject: [PATCH] lib/rest: fix problems re-using HTTP connections Before this fix, it was noticed that the rclone webdav client did not re-use HTTP connections when it should have been. This turned out to be because rclone was not draining the HTTP bodies when it was not expecting a response. From the Go docs: > If the returned error is nil, the Response will contain a non-nil > Body which the user is expected to close. If the Body is not both > read to EOF and closed, the Client's underlying RoundTripper > (typically Transport) may not be able to re-use a persistent TCP > connection to the server for a subsequent "keep-alive" request. This fixes the problem by draining up to 10MB of data from an HTTP response if the NoResponse flag is set, or at the end of a JSON or XML response (which could have some whitespace on the end). See: https://forum.rclone.org/t/webdav-with-persistent-connections/37024/ --- lib/rest/rest.go | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/lib/rest/rest.go b/lib/rest/rest.go index 7303ec184..f248d441d 100644 --- a/lib/rest/rest.go +++ b/lib/rest/rest.go @@ -11,6 +11,7 @@ import ( "errors" "fmt" "io" + "io/ioutil" "mime/multipart" "net/http" "net/url" @@ -157,16 +158,41 @@ func (o *Opts) Copy() *Opts { return &newOpts } +const drainLimit = 10 * 1024 * 1024 + +// drainAndClose discards up to drainLimit bytes from r and closes +// it. Any errors from the Read or Close are returned. +func drainAndClose(r io.ReadCloser) (err error) { + _, readErr := io.CopyN(ioutil.Discard, r, drainLimit) + if readErr == io.EOF { + readErr = nil + } + err = r.Close() + if readErr != nil { + return readErr + } + return err +} + +// checkDrainAndClose is a utility function used to check the return +// from drainAndClose in a defer statement. +func checkDrainAndClose(r io.ReadCloser, err *error) { + cerr := drainAndClose(r) + if *err == nil { + *err = cerr + } +} + // DecodeJSON decodes resp.Body into result func DecodeJSON(resp *http.Response, result interface{}) (err error) { - defer fs.CheckClose(resp.Body, &err) + defer checkDrainAndClose(resp.Body, &err) decoder := json.NewDecoder(resp.Body) return decoder.Decode(result) } // DecodeXML decodes resp.Body into result func DecodeXML(resp *http.Response, result interface{}) (err error) { - defer fs.CheckClose(resp.Body, &err) + defer checkDrainAndClose(resp.Body, &err) decoder := xml.NewDecoder(resp.Body) // MEGAcmd has included escaped HTML entities in its XML output, so we have to be able to // decode them. @@ -304,7 +330,7 @@ func (api *Client) Call(ctx context.Context, opts *Opts) (resp *http.Response, e } } if opts.NoResponse { - return resp, resp.Body.Close() + return resp, drainAndClose(resp.Body) } return resp, nil }