http: handle request range like Amazon S3. (#2112)

Fixes #2098

api-headers.go

@@ -74,12 +74,10 @@ func setObjectHeaders(w http.ResponseWriter, objInfo ObjectInfo, contentRange *h
 	w.Header().Set("Content-Length", strconv.FormatInt(objInfo.Size, 10))
 
 	// for providing ranged content
-	if contentRange != nil {
-		if contentRange.start > 0 || contentRange.length > 0 {
+	if contentRange != nil && contentRange.firstBytePos > -1 {
 		// override content-length
-			w.Header().Set("Content-Length", strconv.FormatInt(contentRange.length, 10))
+		w.Header().Set("Content-Length", strconv.FormatInt(contentRange.getLength(), 10))
 		w.Header().Set("Content-Range", contentRange.String())
 		w.WriteHeader(http.StatusPartialContent)
 	}
-	}
 }

httprange.go

@@ -1,5 +1,5 @@
 /*
- * Minio Cloud Storage, (C) 2015 Minio, Inc.
+ * Minio Cloud Storage, (C) 2015, 2016 Minio, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,10 +24,13 @@ import (
 )
 
 const (
-	b = "bytes="
+	byteRangePrefix = "bytes="
 )
 
-// InvalidRange - invalid range
+// errInvalidRange - returned when given range value is not valid.
+var errInvalidRange = errors.New("Invalid range")
+
+// InvalidRange - invalid range typed error.
 type InvalidRange struct{}
 
 func (e InvalidRange) Error() string {
@@ -36,93 +39,77 @@ func (e InvalidRange) Error() string {
 
 // HttpRange specifies the byte range to be sent to the client.
 type httpRange struct {
-	start, length, size int64
+	firstBytePos int64
+	lastBytePos  int64
+	size         int64
 }
 
 // String populate range stringer interface
-func (r *httpRange) String() string {
-	return fmt.Sprintf("bytes %d-%d/%d", r.start, r.start+r.length-1, r.size)
+func (hrange httpRange) String() string {
+	return fmt.Sprintf("bytes %d-%d/%d", hrange.firstBytePos, hrange.lastBytePos, hrange.size)
 }
 
-// Grab new range from request header
-func getRequestedRange(hrange string, size int64) (*httpRange, error) {
-	r := &httpRange{
-		start:  0,
-		length: 0,
-		size:   0,
-	}
-	r.size = size
-	if hrange != "" {
-		err := r.parseRange(hrange)
-		if err != nil {
-			return nil, err
-		}
-	}
-	return r, nil
+// getLength - get length from the range.
+func (hrange httpRange) getLength() int64 {
+	return 1 + hrange.lastBytePos - hrange.firstBytePos
 }
 
-func (r *httpRange) parse(ra string) error {
-	i := strings.Index(ra, "-")
-	if i < 0 {
-		return InvalidRange{}
-	}
-	start, end := strings.TrimSpace(ra[:i]), strings.TrimSpace(ra[i+1:])
-	if start == "" {
-		// If no start is specified, end specifies the
-		// range start relative to the end of the file.
-		i, err := strconv.ParseInt(end, 10, 64)
-		if err != nil {
-			return InvalidRange{}
-		}
-		if i > r.size {
-			i = r.size
-		}
-		r.start = r.size - i
-		r.length = r.size - r.start
-	} else {
-		i, err := strconv.ParseInt(start, 10, 64)
-		if err != nil || i > r.size || i < 0 {
-			return InvalidRange{}
-		}
-		r.start = i
-		if end == "" {
-			// If no end is specified, range extends to end of the file.
-			r.length = r.size - r.start
-		} else {
-			i, err := strconv.ParseInt(end, 10, 64)
-			if err != nil || r.start > i {
-				return InvalidRange{}
+func parseRequestRange(rangeString string, size int64) (hrange *httpRange, err error) {
+	// Return error if given range string doesn't start with byte range prefix.
+	if !strings.HasPrefix(rangeString, byteRangePrefix) {
+		return nil, fmt.Errorf("'%s' does not start with '%s'", rangeString, byteRangePrefix)
 	}
-			if i >= r.size {
-				i = r.size - 1
+
+	// Trim byte range prefix.
+	byteRangeString := strings.TrimPrefix(rangeString, byteRangePrefix)
+
+	// Check if range string contains delimiter '-', else return error.
+	sepIndex := strings.Index(byteRangeString, "-")
+	if sepIndex == -1 {
+		return nil, fmt.Errorf("invalid range string '%s'", rangeString)
 	}
-			r.length = i - r.start + 1
+
+	firstBytePosString := byteRangeString[:sepIndex]
+	lastBytePosString := byteRangeString[sepIndex+1:]
+
+	firstBytePos := int64(-1)
+	lastBytePos := int64(-1)
+
+	// Convert firstBytePosString only if its not empty.
+	if len(firstBytePosString) > 0 {
+		if firstBytePos, err = strconv.ParseInt(firstBytePosString, 10, 64); err != nil {
+			return nil, fmt.Errorf("'%s' does not have valid first byte position value", rangeString)
 		}
 	}
-	return nil
-}
 
-// parseRange parses a Range header string as per RFC 2616.
-func (r *httpRange) parseRange(s string) error {
-	if s == "" {
-		return errors.New("header not present")
+	// Convert lastBytePosString only if its not empty.
+	if len(lastBytePosString) > 0 {
+		if lastBytePos, err = strconv.ParseInt(lastBytePosString, 10, 64); err != nil {
+			return nil, fmt.Errorf("'%s' does not have valid last byte position value", rangeString)
 		}
-	if !strings.HasPrefix(s, b) {
-		return InvalidRange{}
 	}
 
-	ras := strings.Split(s[len(b):], ",")
-	if len(ras) == 0 {
-		return errors.New("invalid request")
-	}
-	// Just pick the first one and ignore the rest, we only support one range per object
-	if len(ras) > 1 {
-		return errors.New("multiple ranges specified")
+	// Return error if firstBytePosString and lastBytePosString are empty.
+	if firstBytePos == -1 && lastBytePos == -1 {
+		return nil, fmt.Errorf("'%s' does not have valid range value", rangeString)
 	}
 
-	ra := strings.TrimSpace(ras[0])
-	if ra == "" {
-		return InvalidRange{}
+	if firstBytePos == -1 {
+		// Return error if lastBytePos is zero and firstBytePos is not given eg. "bytes=-0"
+		if lastBytePos == 0 {
+			return nil, errInvalidRange
+		}
+		firstBytePos = size - lastBytePos
+		lastBytePos = size - 1
+	} else if lastBytePos == -1 {
+		lastBytePos = size - 1
+	} else if firstBytePos > lastBytePos {
+		// Return error if firstBytePos is greater than lastBytePos
+		return nil, fmt.Errorf("'%s' does not have valid range value", rangeString)
+	} else if lastBytePos >= size {
+		// Set lastBytePos is out of size range.
+		lastBytePos = size - 1
 	}
-	return r.parse(ra)
+
+	return &httpRange{firstBytePos, lastBytePos, size}, nil
 }

httprange_test.go

@@ -0,0 +1,67 @@
+/*
+ * Minio Cloud Storage, (C) 2016 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package main
+
+import "testing"
+
+// Test parseRequestRange()
+func TestParseRequestRange(t *testing.T) {
+	// Test success cases.
+	successCases := []struct {
+		rangeString  string
+		firstBytePos int64
+		lastBytePos  int64
+		length       int64
+	}{
+		{"bytes=2-5", 2, 5, 4},
+		{"bytes=2-20", 2, 9, 8},
+		{"bytes=2-", 2, 9, 8},
+		{"bytes=-4", 6, 9, 4},
+	}
+
+	for _, successCase := range successCases {
+		hrange, err := parseRequestRange(successCase.rangeString, 10)
+		if err != nil {
+			t.Fatalf("expected: <nil>, got: %s", err)
+		}
+
+		if hrange.firstBytePos != successCase.firstBytePos {
+			t.Fatalf("expected: %d, got: %d", successCase.firstBytePos, hrange.firstBytePos)
+		}
+
+		if hrange.lastBytePos != successCase.lastBytePos {
+			t.Fatalf("expected: %d, got: %d", successCase.lastBytePos, hrange.lastBytePos)
+		}
+		if hrange.getLength() != successCase.length {
+			t.Fatalf("expected: %d, got: %d", successCase.length, hrange.getLength())
+		}
+	}
+
+	// Test invalid range strings.
+	invalidRangeStrings := []string{"", "2-5", "bytes= 2-5", "bytes=2 - 5", "bytes=0-0,-1", "bytes=5-2", "bytes=2-5 ", "bytes=2--5"}
+	for _, rangeString := range invalidRangeStrings {
+		if _, err := parseRequestRange(rangeString, 10); err == nil {
+			t.Fatalf("expected: an error, got: <nil>")
+		}
+	}
+
+	// Test error range strings.
+	errorRangeString := "bytes=-0"
+	if _, err := parseRequestRange(errorRangeString, 10); err != errInvalidRange {
+		t.Fatalf("expected: %s, got: %s", errInvalidRange, err)
+	}
+}

object-handlers.go

@@ -104,14 +104,20 @@ func (api objectAPIHandlers) GetObjectHandler(w http.ResponseWriter, r *http.Req
 		return
 	}
 
-	// Caculate the http Range.
+	// Get request range.
 	var hrange *httpRange
-	hrange, err = getRequestedRange(r.Header.Get("Range"), objInfo.Size)
-	if err != nil {
+	if hrange, err = parseRequestRange(r.Header.Get("Range"), objInfo.Size); err != nil {
+		// Handle only errInvalidRange
+		// Ignore other parse error and treat it as regular Get request like Amazon S3.
+		if err == errInvalidRange {
 			writeErrorResponse(w, r, ErrInvalidRange, r.URL.Path)
 			return
 		}
 
+		// log the error.
+		errorIf(err, "Invalid request range")
+	}
+
 	// Set standard object headers.
 	setObjectHeaders(w, objInfo, hrange)
 
@@ -129,12 +135,12 @@ func (api objectAPIHandlers) GetObjectHandler(w http.ResponseWriter, r *http.Req
 	}
 
 	// Get the object.
-	startOffset := hrange.start
-	length := hrange.length
-	if length == 0 {
-		length = objInfo.Size - startOffset
+	startOffset := int64(0)
+	length := objInfo.Size
+	if hrange != nil {
+		startOffset = hrange.firstBytePos
+		length = hrange.getLength()
 	}
-
 	// Reads the object at startOffset and writes to mw.
 	if err := api.ObjectAPI.GetObject(bucket, object, startOffset, length, w); err != nil {
 		errorIf(err, "Unable to write to client.")

server_test.go

@@ -1646,7 +1646,7 @@ func (s *TestSuiteCommon) TestGetObjectRangeErrors(c *C) {
 	request, err = newTestRequest("GET", getGetObjectURL(s.endPoint, bucketName, objectName),
 		0, nil, s.accessKey, s.secretKey)
 	// Invalid byte range set.
-	request.Header.Add("Range", "bytes=13-")
+	request.Header.Add("Range", "bytes=-0")
 	c.Assert(err, IsNil)
 
 	client = http.Client{}