Only set err if not set

putter.go

@@ -187,7 +187,9 @@ func (p *putter) retryPutPart(part *part) {
 		logger.debugPrintf("Error on attempt %d: Retrying part: %d, Error: %s", i, part.PartNumber, err)
 		time.Sleep(time.Duration(math.Exp2(float64(i))) * 100 * time.Millisecond) // exponential back-off
 	}
-	p.err = err
+	if p.err == nil {
+		p.err = err
+	}
 }
 
 // uploads a part, checking the etag against the calculated value

s3gof3r.go

@@ -16,6 +16,8 @@ import (
 	"time"
 )
 
+const versionParam = "versionId"
+
 var regionMatcher = regexp.MustCompile("s3-([a-z0-9-]+).amazonaws.com")
 
 // S3 contains the domain or endpoint of an S3-compatible service and
@@ -103,6 +105,8 @@ func (s *S3) Bucket(name string) *Bucket {
 // Header data from the downloaded object is also returned, useful for reading object metadata.
 // DefaultConfig is used if c is nil
 // Callers should call Close on r to ensure that all resources are released.
+//
+// To specify an object version in a versioned bucket, the version ID may be included in the path as a url parameter. See http://docs.aws.amazon.com/AmazonS3/latest/dev/RetrievingObjectVersions.html
 func (b *Bucket) GetReader(path string, c *Config) (r io.ReadCloser, h http.Header, err error) {
 	if path == "" {
 		return nil, nil, errors.New("empty path requested")
@@ -136,23 +140,36 @@ func (b *Bucket) PutWriter(path string, h http.Header, c *Config) (w io.WriteClo
 }
 
 // url returns a parsed url to the given path. c must not be nil
-// Note: Urls containing some special characters will fail due to net/http bug.
-// See https://code.google.com/p/go/issues/detail?id=5684
 func (b *Bucket) url(bPath string, c *Config) (*url.URL, error) {
 
+	// parse versionID parameter from path, if included
+	// See https://github.com/rlmcpherson/s3gof3r/issues/84 for rationale
+	purl, err := url.Parse(bPath)
+	if err != nil {
+		return nil, err
+	}
+	var vals url.Values
+	if v := purl.Query().Get(versionParam); v != "" {
+		vals = make(url.Values)
+		vals.Add(versionParam, v)
+		bPath = strings.Split(bPath, "?")[0] // remove versionID from path
+	}
+
 	// handling for bucket names containing periods / explicit PathStyle addressing
 	// http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html for details
 	if strings.Contains(b.Name, ".") || c.PathStyle {
 		return &url.URL{
-			Host:   b.S3.Domain,
-			Scheme: c.Scheme,
-			Path:   path.Clean(fmt.Sprintf("/%s/%s", b.Name, bPath)),
+			Host:     b.S3.Domain,
+			Scheme:   c.Scheme,
+			Path:     path.Clean(fmt.Sprintf("/%s/%s", b.Name, bPath)),
+			RawQuery: vals.Encode(),
 		}, nil
 	} else {
 		return &url.URL{
-			Scheme: c.Scheme,
-			Path:   path.Clean(fmt.Sprintf("/%s", bPath)),
-			Host:   path.Clean(fmt.Sprintf("%s.%s", b.Name, b.S3.Domain)),
+			Scheme:   c.Scheme,
+			Path:     path.Clean(fmt.Sprintf("/%s", bPath)),
+			Host:     path.Clean(fmt.Sprintf("%s.%s", b.Name, b.S3.Domain)),
+			RawQuery: vals.Encode(),
 		}, nil
 	}
 }

s3gof3r_test.go

@@ -565,6 +565,7 @@ func TestBucketURL(t *testing.T) {
 		{"bucket1", "#path ", DefaultConfig, `https://bucket1.s3.amazonaws.com/%23path%20`},
 		{"bucket.2", "path", DefaultConfig, "https://s3.amazonaws.com/bucket.2/path"},
 		{"bucket.2", "#path", DefaultConfig, `https://s3.amazonaws.com/bucket.2/%23path`},
+		{"bucket.2", "#path?versionId=seQK1YwRAy6Ex25YHb_yJHbo94jSDnpu", DefaultConfig, `https://s3.amazonaws.com/bucket.2/%23path%3FversionId=seQK1YwRAy6Ex25YHb_yJHbo94jSDnpu`}, // versionId-specific handling
 	}
 
 	for _, tt := range urlTests {