From 0edbc9578dad74022b8886f6fc970d5f320b01b1 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Wed, 28 Aug 2019 11:21:38 +0100
Subject: [PATCH] googlephotos,onedrive: fix crash on error response - fixes
 #3491

This fixes a crash on the google photos backend when an error is
returned from the rest.Call function.

This turned out to be a mis-understanding of the rest docs so
- improved rest.Call docs
- fixed mis-understanding in google photos backend
- fixed similar mis-understading in onedrive backend
---
 backend/googlephotos/googlephotos.go |  1 -
 backend/onedrive/onedrive.go         | 24 +++++++++++++-----------
 lib/rest/rest.go                     |  6 ++++--
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/backend/googlephotos/googlephotos.go b/backend/googlephotos/googlephotos.go
index c7c3c4337..0bb996253 100644
--- a/backend/googlephotos/googlephotos.go
+++ b/backend/googlephotos/googlephotos.go
@@ -956,7 +956,6 @@ func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, op
 	err = o.fs.pacer.CallNoRetry(func() (bool, error) {
 		resp, err = o.fs.srv.Call(&opts)
 		if err != nil {
-			_ = resp.Body.Close()
 			return shouldRetry(resp, err)
 		}
 		token, err = rest.ReadBody(resp)
diff --git a/backend/onedrive/onedrive.go b/backend/onedrive/onedrive.go
index c3edc00c3..82b9bba5f 100644
--- a/backend/onedrive/onedrive.go
+++ b/backend/onedrive/onedrive.go
@@ -1464,22 +1464,24 @@ func (o *Object) uploadFragment(url string, start int64, totalSize int64, chunk
 	}
 	//	var response api.UploadFragmentResponse
 	var resp *http.Response
+	var body []byte
 	err = o.fs.pacer.Call(func() (bool, error) {
 		_, _ = chunk.Seek(0, io.SeekStart)
 		resp, err = o.fs.srv.Call(&opts)
-		if resp != nil {
-			defer fs.CheckClose(resp.Body, &err)
+		if err != nil {
+			return shouldRetry(resp, err)
 		}
-		retry, err := shouldRetry(resp, err)
-		if !retry && resp != nil {
-			if resp.StatusCode == 200 || resp.StatusCode == 201 {
-				// we are done :)
-				// read the item
-				info = &api.Item{}
-				return false, json.NewDecoder(resp.Body).Decode(info)
-			}
+		body, err = rest.ReadBody(resp)
+		if err != nil {
+			return shouldRetry(resp, err)
 		}
-		return retry, err
+		if resp.StatusCode == 200 || resp.StatusCode == 201 {
+			// we are done :)
+			// read the item
+			info = &api.Item{}
+			return false, json.Unmarshal(body, info)
+		}
+		return false, nil
 	})
 	return info, err
 }
diff --git a/lib/rest/rest.go b/lib/rest/rest.go
index 0796c3e43..39dbe1807 100644
--- a/lib/rest/rest.go
+++ b/lib/rest/rest.go
@@ -46,7 +46,7 @@ func ReadBody(resp *http.Response) (result []byte, err error) {
 }
 
 // defaultErrorHandler doesn't attempt to parse the http body, just
-// returns it in the error message
+// returns it in the error message closing resp.Body
 func defaultErrorHandler(resp *http.Response) (err error) {
 	body, err := ReadBody(resp)
 	if err != nil {
@@ -178,9 +178,11 @@ func ClientWithNoRedirects(c *http.Client) *http.Client {
 
 // Call makes the call and returns the http.Response
 //
-// if err != nil then resp.Body will need to be closed unless
+// if err == nil then resp.Body will need to be closed unless
 // opt.NoResponse is set
 //
+// if err != nil then resp.Body will have been closed
+//
 // it will return resp if at all possible, even if err is set
 func (api *Client) Call(opts *Opts) (resp *http.Response, err error) {
 	api.mu.RLock()