Skip to content

Commit f2418f2

Browse files
yannkYann Kerhervé
authored andcommitted
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 remove the quotes around the value if they exist.
1 parent c070c8f commit f2418f2

2 files changed

Lines changed: 48 additions & 36 deletions

File tree

s3util/uploader.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,17 @@ package s3util
33
import (
44
"bytes"
55
"encoding/xml"
6-
"github.com/kr/s3"
76
"fmt"
87
"io"
98
"net/http"
109
"net/url"
1110
"strconv"
11+
"strings"
1212
"sync"
1313
"syscall"
1414
"time"
15+
16+
"github.com/kr/s3"
1517
)
1618

1719
// defined by amazon
@@ -190,11 +192,14 @@ func (u *uploader) putPart(p *part) error {
190192
if resp.StatusCode != 200 {
191193
return newRespError(resp)
192194
}
193-
s := resp.Header.Get("etag") // includes quote chars for some reason
194-
if len(s) < 2 {
195-
return fmt.Errorf("received invalid etag %q", s)
195+
196+
s := resp.Header.Get("etag")
197+
s = strings.Trim(s, `"`) // includes quote chars for some reason
198+
if s == "" {
199+
return fmt.Errorf("received empty etag")
196200
}
197-
p.ETag = s[1 : len(s)-1]
201+
p.ETag = s
202+
198203
return nil
199204
}
200205

s3util/uploader_test.go

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -127,37 +127,44 @@ func TestEmptyEtag(t *testing.T) {
127127
t.Fatal("uploader panic", err)
128128
}
129129
}()
130-
body := readClose{
131-
strings.NewReader(`<UploadId>foo</UploadId>`),
132-
make(closeCounter, 100),
133-
}
134-
c := *DefaultConfig
135-
c.Client = &http.Client{
136-
Transport: RoundTripperFunc(func(req *http.Request) (*http.Response, error) {
137-
resp := &http.Response{
138-
StatusCode: 200,
139-
Body: body,
140-
Header: http.Header{
141-
"Etag": {""},
142-
},
143-
}
144-
return resp, nil
145-
}),
146-
}
147-
u, err := newUploader("https://s3.amazonaws.com/foo/bar", nil, &c)
148-
if err != nil {
149-
t.Fatal("unexpected err", err)
150-
}
151-
const size = minPartSize + minPartSize/3
152-
n, err := io.Copy(u, io.LimitReader(devZero, size))
153-
if err == nil || err.Error() != `received invalid etag ""` {
154-
t.Fatalf("expected err: %q", err)
155-
}
156-
if n != minPartSize {
157-
t.Fatalf("wrote %d bytes want %d", n, minPartSize)
130+
131+
testEmptyEtag := func(emptyEtag string) {
132+
body := readClose{
133+
strings.NewReader(`<UploadId>foo</UploadId>`),
134+
make(closeCounter, 100),
135+
}
136+
c := *DefaultConfig
137+
c.Client = &http.Client{
138+
Transport: RoundTripperFunc(func(req *http.Request) (*http.Response, error) {
139+
resp := &http.Response{
140+
StatusCode: 200,
141+
Body: body,
142+
Header: http.Header{
143+
"Etag": []string{emptyEtag},
144+
},
145+
}
146+
return resp, nil
147+
}),
148+
}
149+
u, err := newUploader("https://s3.amazonaws.com/foo/bar", nil, &c)
150+
if err != nil {
151+
t.Fatal("unexpected err", err)
152+
}
153+
const size = minPartSize + minPartSize/3
154+
n, err := io.Copy(u, io.LimitReader(devZero, size))
155+
if err == nil || err.Error() != `received empty etag` {
156+
t.Fatalf("expected err: %q", err)
157+
}
158+
if n != minPartSize {
159+
t.Fatalf("wrote %d bytes want %d", n, minPartSize)
160+
}
161+
err = u.Close()
162+
if err == nil || err.Error() != `received empty etag` {
163+
t.Fatalf("expected err: %q", err)
164+
}
158165
}
159-
err = u.Close()
160-
if err == nil || err.Error() != `received invalid etag ""` {
161-
t.Fatalf("expected err: %q", err)
166+
167+
for _, emptyEtag := range []string{"", `""`} {
168+
testEmptyEtag(emptyEtag)
162169
}
163170
}

0 commit comments

Comments
 (0)