Migrated v4 signing process to streaming. #37

Merged
merged 1 commit into from Mar 12, 2015
Jump to file or symbol
Failed to load files and symbols.
+48 −25
Split
View
@@ -29,7 +29,7 @@ type Signer func(*http.Request, Auth) error
var _ Signer = SignV2
var _ Signer = SignV4Factory("", "")
-type hasher func([]byte) string
+type hasher func(io.Reader) (string, error)
const (
ISO8601BasicFormat = "20060102T150405Z"
@@ -216,7 +216,9 @@ func canonicalRequest(
canReq = c.String()
debug.Printf("canReq:\n\"\"\"\n%s\n\"\"\"", canReq)
- return canReq, hasher([]byte(canReq)), sortedHdrNames, nil
+ canReqHash, err = hasher(bytes.NewBuffer([]byte(canReq)))
+
+ return canReq, canReqHash, sortedHdrNames, err
}
// Task 2: Create a string to Sign
@@ -309,16 +311,10 @@ func canonicalHeaders(sortedHeaderNames []string, host string, hdr http.Header)
// lowercase hexadecimal string.
func payloadHash(req *http.Request, hasher hasher) (string, error) {
@axw

axw Mar 12, 2015

Member

Probably should add a comment to this saying that it'll eat the request body.

@axw

axw Mar 12, 2015

Member

On second thoughts, I think a much simpler and less error prone approach would be to check in payloadHash if req.Body is an io.ReadSeeker. If it is, pass to hasher as-is and then seek back to the original pos, if it's not, use the old approach of reading into memory and then replacing req.Body.

@kat-co

kat-co Mar 12, 2015

Member

As discussed in IRC, http.NewRequest wraps all body parameters in an ioutil.NopCloser, so this type assertion won't work :(

if req.Body == nil {
- return hasher([]byte("")), nil
- }
-
- b, err := ioutil.ReadAll(req.Body)
- if err != nil {
- return "", err
+ return hasher(bytes.NewBuffer(nil))
}
- req.Body = ioutil.NopCloser(bytes.NewBuffer(b))
- return hasher(b), nil
+ return hasher(req.Body)
}
// Retrieve the header names, lower-case them, and sort them.
@@ -340,8 +336,11 @@ func hmacHasher(key []byte, value string) []byte {
return h.Sum(nil)
}
-func sha256Hasher(payload []byte) string {
- return fmt.Sprintf("%x", sha256.Sum256(payload))
+func sha256Hasher(payloadReader io.Reader) (string, error) {
+ hasher := sha256.New()
+ _, err := io.Copy(hasher, payloadReader)
+
+ return fmt.Sprintf("%x", hasher.Sum(nil)), err
}
func credentialScope(t time.Time, regionName, svcName string) string {
View
@@ -211,6 +211,11 @@ func (m *Multi) putPart(n int, r io.ReadSeeker, partSize int64, md5b64 string) (
if err := m.Bucket.S3.Sign(req, m.Bucket.Auth); err != nil {
return Part{}, err
}
+ // Signing may read the request body.
+ if _, err := r.Seek(0, 0); err != nil {
+ return Part{}, err
+ }
+
resp, err := requestRetryLoop(req, attempts)
defer resp.Body.Close()
@@ -421,11 +426,12 @@ func (m *Multi) Complete(parts []Part) error {
if err != nil {
return err
}
+ body := bytes.NewReader(data)
req, err := http.NewRequest(
"POST",
m.Bucket.Region.ResolveS3BucketEndpoint(m.Bucket.Name),
- bytes.NewReader(data),
+ body,
)
if err != nil {
return err
@@ -443,6 +449,11 @@ func (m *Multi) Complete(parts []Part) error {
if err := m.Bucket.S3.Sign(req, m.Bucket.Auth); err != nil {
return err
}
+ // Signing may read the request body.
+ if _, err := body.Seek(0, 0); err != nil {
+ return err
+ }
+
resp, err := requestRetryLoop(req, attempts)
if err != nil {
return err
View
@@ -87,11 +87,11 @@ var createBucketConfiguration = `<CreateBucketConfiguration xmlns="http://s3.ama
<LocationConstraint>%s</LocationConstraint>
</CreateBucketConfiguration>`
-// locationConstraint returns an io.Reader specifying a LocationConstraint if
-// required for the region.
+// locationConstraint returns a *strings.Reader specifying a
+// LocationConstraint if required for the region.
//
// See http://goo.gl/bh9Kq for details.
-func (s3 *S3) locationConstraint() io.Reader {
+func (s3 *S3) locationConstraint() *strings.Reader {
constraint := ""
if s3.Region.S3LocationConstraint {
constraint = fmt.Sprintf(createBucketConfiguration, s3.Region.Name)
@@ -114,16 +114,16 @@ const (
//
// See http://goo.gl/FEBPD for details.
func (b *Bucket) Put(path string, data []byte, contType string, perm ACL) error {
- body := bytes.NewBuffer(data)
+ body := bytes.NewReader(data)
return b.PutReader(path, body, int64(len(data)), contType, perm)
}
// PutBucket creates a new bucket.
//
// See http://goo.gl/ndjnR for details.
func (b *Bucket) PutBucket(perm ACL) error {
-
- req, err := http.NewRequest("PUT", b.ResolveS3BucketEndpoint(b.Name), b.locationConstraint())
+ body := b.locationConstraint()
+ req, err := http.NewRequest("PUT", b.ResolveS3BucketEndpoint(b.Name), body)
if err != nil {
return err
}
@@ -135,6 +135,11 @@ func (b *Bucket) PutBucket(perm ACL) error {
if err := b.S3.Sign(req, b.Auth); err != nil {
return err
}
+ // Signing may read the request body.
+ if _, err := body.Seek(0, 0); err != nil {
+ return err
+ }
+
_, err = http.DefaultClient.Do(req)
return err
}
@@ -201,14 +206,18 @@ func (b *Bucket) GetReader(path string) (rc io.ReadCloser, err error) {
// PutReader inserts an object into the S3 bucket by consuming data
// from r until EOF.
-func (b *Bucket) PutReader(path string, r io.Reader, length int64, contType string, perm ACL) error {
+//
+// The signature of this method was modified to take in an
+// io.ReadSeeker to help address
+// https://github.com/go-amz/amz/issues/35.
+func (b *Bucket) PutReader(path string, r io.ReadSeeker, length int64, contType string, perm ACL) error {
return b.PutReaderWithHeader(path, r, length, contType, perm, http.Header{})
}
// PutReaderWithHeader inserts an object into the S3 bucket by
// consuming data from r until EOF. It also adds the headers provided
// to the request.
-func (b *Bucket) PutReaderWithHeader(path string, r io.Reader, length int64, contType string, perm ACL, hdrs http.Header) error {
+func (b *Bucket) PutReaderWithHeader(path string, r io.ReadSeeker, length int64, contType string, perm ACL, hdrs http.Header) error {
req, err := http.NewRequest("PUT", b.Region.ResolveS3BucketEndpoint(b.Name), r)
if err != nil {
@@ -226,6 +235,10 @@ func (b *Bucket) PutReaderWithHeader(path string, r io.Reader, length int64, con
if err := b.S3.Sign(req, b.Auth); err != nil {
return err
}
+ // Signing may read the request body.
+ if _, err := r.Seek(0, 0); err != nil {
+ return err
+ }
resp, err := http.DefaultClient.Do(req)
if err != nil {
@@ -511,7 +524,7 @@ func requestRetryLoop(req *http.Request, retryStrat aws.AttemptStrategy) (*http.
if shouldRetry(err) && attempt.HasNext() {
continue
}
- return nil, err
+ return nil, fmt.Errorf("making request: %v", err)
}
if debug {
View
@@ -191,7 +191,7 @@ func (s *S) TestPutReader(c *C) {
b, err := s.s3.Bucket("bucket")
c.Assert(err, IsNil)
- buf := bytes.NewBufferString("content")
+ buf := bytes.NewReader([]byte("content"))
err = b.PutReader("name", buf, int64(buf.Len()), "content-type", s3.Private)
c.Assert(err, IsNil)
@@ -210,7 +210,7 @@ func (s *S) TestPutReaderWithHeader(c *C) {
b, err := s.s3.Bucket("bucket")
c.Assert(err, IsNil)
- buf := bytes.NewBufferString("content")
+ buf := bytes.NewReader([]byte("content"))
err = b.PutReaderWithHeader("name", buf, int64(buf.Len()), "content-type", s3.Private, http.Header{
"Cache-Control": []string{"max-age=5"},
})
View
@@ -198,7 +198,7 @@ func (s *ClientTests) TestBasicFunctionality(c *C) {
c.Assert(err, IsNil)
c.Assert(string(data), Equals, "yo!")
- buf := bytes.NewBufferString("hey!")
+ buf := bytes.NewReader([]byte("hey!"))
err = b.PutReader("name2", buf, int64(buf.Len()), "text/plain", s3.Private)
c.Assert(err, IsNil)
defer b.Del("name2")