From 1d079524056eeaf5c59b98dad1c57aba03055f80 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Yann=20Kerherv=C3=A9?= <yann.kerherve@gmail.com>
Date: Wed, 19 Aug 2015 17:38:36 -0700
Subject: [PATCH] Fix uploader when ETag value is not quoted

Previous version of the code was blindly removing the quotes assuming
they were always present, in the case where they are not (for instance
when using it against compatible-ish backends) then the ETag was making
the request fail.

This version only removes the quotes around the value if they exist.
---
 s3util/uploader.go      | 15 ++++++---
 s3util/uploader_test.go | 69 +++++++++++++++++++++++------------------
 2 files changed, 48 insertions(+), 36 deletions(-)

diff --git a/s3util/uploader.go b/s3util/uploader.go
index 1a21511..49fa918 100644
--- a/s3util/uploader.go
+++ b/s3util/uploader.go
@@ -3,15 +3,17 @@ package s3util
 import (
 	"bytes"
 	"encoding/xml"
-	"github.com/kr/s3"
 	"fmt"
 	"io"
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 	"sync"
 	"syscall"
 	"time"
+
+	"github.com/kr/s3"
 )
 
 // defined by amazon
@@ -190,11 +192,14 @@ func (u *uploader) putPart(p *part) error {
 	if resp.StatusCode != 200 {
 		return newRespError(resp)
 	}
-	s := resp.Header.Get("etag") // includes quote chars for some reason
-	if len(s) < 2 {
-		return fmt.Errorf("received invalid etag %q", s)
+
+	s := resp.Header.Get("etag")
+	s = strings.Trim(s, `"`) // includes quote chars for some reason
+	if s == "" {
+		return fmt.Errorf("received empty etag")
 	}
-	p.ETag = s[1 : len(s)-1]
+	p.ETag = s
+
 	return nil
 }
 
diff --git a/s3util/uploader_test.go b/s3util/uploader_test.go
index 77adbe8..ff32dd4 100644
--- a/s3util/uploader_test.go
+++ b/s3util/uploader_test.go
@@ -127,37 +127,44 @@ func TestEmptyEtag(t *testing.T) {
 			t.Fatal("uploader panic", err)
 		}
 	}()
-	body := readClose{
-		strings.NewReader(`<UploadId>foo</UploadId>`),
-		make(closeCounter, 100),
-	}
-	c := *DefaultConfig
-	c.Client = &http.Client{
-		Transport: RoundTripperFunc(func(req *http.Request) (*http.Response, error) {
-			resp := &http.Response{
-				StatusCode: 200,
-				Body: body,
-				Header: http.Header{
-					"Etag": {""},
-				},
-			}
-			return resp, nil
-		}),
-	}
-	u, err := newUploader("https://s3.amazonaws.com/foo/bar", nil, &c)
-	if err != nil {
-		t.Fatal("unexpected err", err)
-	}
-	const size = minPartSize + minPartSize/3
-	n, err := io.Copy(u, io.LimitReader(devZero, size))
-	if err == nil || err.Error() != `received invalid etag ""` {
-		t.Fatalf("expected err: %q", err)
-	}
-	if n != minPartSize {
-		t.Fatalf("wrote %d bytes want %d", n, minPartSize)
+
+	testEmptyEtag := func(emptyEtag string) {
+		body := readClose{
+			strings.NewReader(`<UploadId>foo</UploadId>`),
+			make(closeCounter, 100),
+		}
+		c := *DefaultConfig
+		c.Client = &http.Client{
+			Transport: RoundTripperFunc(func(req *http.Request) (*http.Response, error) {
+				resp := &http.Response{
+					StatusCode: 200,
+					Body:       body,
+					Header: http.Header{
+						"Etag": []string{emptyEtag},
+					},
+				}
+				return resp, nil
+			}),
+		}
+		u, err := newUploader("https://s3.amazonaws.com/foo/bar", nil, &c)
+		if err != nil {
+			t.Fatal("unexpected err", err)
+		}
+		const size = minPartSize + minPartSize/3
+		n, err := io.Copy(u, io.LimitReader(devZero, size))
+		if err == nil || err.Error() != `received empty etag` {
+			t.Fatalf("expected err: %q", err)
+		}
+		if n != minPartSize {
+			t.Fatalf("wrote %d bytes want %d", n, minPartSize)
+		}
+		err = u.Close()
+		if err == nil || err.Error() != `received empty etag` {
+			t.Fatalf("expected err: %q", err)
+		}
 	}
-	err = u.Close()
-	if err == nil || err.Error() != `received invalid etag ""` {
-		t.Fatalf("expected err: %q", err)
+
+	for _, emptyEtag := range []string{"", `""`} {
+		testEmptyEtag(emptyEtag)
 	}
 }