rest: fix and cleanup closing of http response body

If client.Do returns an error, then there's no body that has to be
closed. For requests for which we are not interested in the response
body, immediately drain and close the body to make sure it isn't
forgotten later on.

This change in particular adds the missing `Close()` call for the
`List()` command.
This commit is contained in:
Michael Eischer 2024-01-06 15:24:33 +01:00
parent 03e06d0797
commit b1a8fd1d03

View file

@ -58,6 +58,17 @@ func Open(_ context.Context, cfg Config, rt http.RoundTripper) (*Backend, error)
return be, nil
}
func drainAndClose(resp *http.Response) error {
_, err := io.Copy(io.Discard, resp.Body)
cerr := resp.Body.Close()
// return first error
if err != nil {
return errors.Errorf("drain: %w", err)
}
return cerr
}
// Create creates a new REST on server configured in config.
func Create(ctx context.Context, cfg Config, rt http.RoundTripper) (*Backend, error) {
be, err := Open(ctx, cfg, rt)
@ -80,20 +91,14 @@ func Create(ctx context.Context, cfg Config, rt http.RoundTripper) (*Backend, er
return nil, err
}
if err := drainAndClose(resp); err != nil {
return nil, err
}
if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("server response unexpected: %v (%v)", resp.Status, resp.StatusCode)
}
_, err = io.Copy(io.Discard, resp.Body)
if err != nil {
return nil, err
}
err = resp.Body.Close()
if err != nil {
return nil, err
}
return be, nil
}
@ -136,22 +141,19 @@ func (b *Backend) Save(ctx context.Context, h backend.Handle, rd backend.RewindR
req.ContentLength = rd.Length()
resp, err := b.client.Do(req)
var cerr error
if resp != nil {
_, _ = io.Copy(io.Discard, resp.Body)
cerr = resp.Body.Close()
}
if err != nil {
return errors.WithStack(err)
}
if err := drainAndClose(resp); err != nil {
return err
}
if resp.StatusCode != http.StatusOK {
return errors.Errorf("server response unexpected: %v (%v)", resp.Status, resp.StatusCode)
}
return errors.Wrap(cerr, "Close")
return nil
}
// notExistError is returned whenever the requested file does not exist on the
@ -215,22 +217,17 @@ func (b *Backend) openReader(ctx context.Context, h backend.Handle, length int,
req.Header.Set("Accept", ContentTypeV2)
resp, err := b.client.Do(req)
if err != nil {
if resp != nil {
_, _ = io.Copy(io.Discard, resp.Body)
_ = resp.Body.Close()
}
return nil, errors.Wrap(err, "client.Do")
}
if resp.StatusCode == http.StatusNotFound {
_ = resp.Body.Close()
_ = drainAndClose(resp)
return nil, &notExistError{h}
}
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusPartialContent {
_ = resp.Body.Close()
_ = drainAndClose(resp)
return nil, errors.Errorf("unexpected HTTP response (%v): %v", resp.StatusCode, resp.Status)
}
@ -250,13 +247,11 @@ func (b *Backend) Stat(ctx context.Context, h backend.Handle) (backend.FileInfo,
return backend.FileInfo{}, errors.WithStack(err)
}
_, _ = io.Copy(io.Discard, resp.Body)
if err = resp.Body.Close(); err != nil {
return backend.FileInfo{}, errors.Wrap(err, "Close")
if err = drainAndClose(resp); err != nil {
return backend.FileInfo{}, err
}
if resp.StatusCode == http.StatusNotFound {
_ = resp.Body.Close()
return backend.FileInfo{}, &notExistError{h}
}
@ -285,13 +280,15 @@ func (b *Backend) Remove(ctx context.Context, h backend.Handle) error {
req.Header.Set("Accept", ContentTypeV2)
resp, err := b.client.Do(req)
if err != nil {
return errors.Wrap(err, "client.Do")
}
if err = drainAndClose(resp); err != nil {
return err
}
if resp.StatusCode == http.StatusNotFound {
_ = resp.Body.Close()
return &notExistError{h}
}
@ -299,12 +296,7 @@ func (b *Backend) Remove(ctx context.Context, h backend.Handle) error {
return errors.Errorf("blob not removed, server response: %v (%v)", resp.Status, resp.StatusCode)
}
_, err = io.Copy(io.Discard, resp.Body)
if err != nil {
return errors.Wrap(err, "Copy")
}
return errors.Wrap(resp.Body.Close(), "Close")
return nil
}
// List runs fn for each file in the backend which has the type t. When an
@ -322,7 +314,6 @@ func (b *Backend) List(ctx context.Context, t backend.FileType, fn func(backend.
req.Header.Set("Accept", ContentTypeV2)
resp, err := b.client.Do(req)
if err != nil {
return errors.Wrap(err, "List")
}
@ -333,19 +324,25 @@ func (b *Backend) List(ctx context.Context, t backend.FileType, fn func(backend.
// already ignores missing directories, but misuses "not found" to
// report certain internal errors, see
// https://github.com/rclone/rclone/pull/7550 for details.
return nil
return drainAndClose(resp)
}
}
if resp.StatusCode != http.StatusOK {
_ = drainAndClose(resp)
return errors.Errorf("List failed, server response: %v (%v)", resp.Status, resp.StatusCode)
}
if resp.Header.Get("Content-Type") == ContentTypeV2 {
return b.listv2(ctx, resp, fn)
err = b.listv2(ctx, resp, fn)
} else {
err = b.listv1(ctx, t, resp, fn)
}
return b.listv1(ctx, t, resp, fn)
if cerr := drainAndClose(resp); cerr != nil && err == nil {
err = cerr
}
return err
}
// listv1 uses the REST protocol v1, where a list HTTP request (e.g. `GET