diff --git a/api-headers.go b/api-headers.go index 64a025817..f7fb43520 100644 --- a/api-headers.go +++ b/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 { - // override content-length - w.Header().Set("Content-Length", strconv.FormatInt(contentRange.length, 10)) - w.Header().Set("Content-Range", contentRange.String()) - w.WriteHeader(http.StatusPartialContent) - } + if contentRange != nil && contentRange.firstBytePos > -1 { + // override content-length + w.Header().Set("Content-Length", strconv.FormatInt(contentRange.getLength(), 10)) + w.Header().Set("Content-Range", contentRange.String()) + w.WriteHeader(http.StatusPartialContent) } } diff --git a/httprange.go b/httprange.go index 4bb23806e..8fda6efb6 100644 --- a/httprange.go +++ b/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{} - } - if i >= r.size { - i = r.size - 1 - } - r.length = i - r.start + 1 - } +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) } - 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") + // 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) } - if !strings.HasPrefix(s, b) { - return InvalidRange{} + + 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) + } } - ras := strings.Split(s[len(b):], ",") - if len(ras) == 0 { - return errors.New("invalid request") + // 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) + } } - // 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 } diff --git a/httprange_test.go b/httprange_test.go new file mode 100644 index 000000000..d9952ebed --- /dev/null +++ b/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: , 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: ") + } + } + + // Test error range strings. + errorRangeString := "bytes=-0" + if _, err := parseRequestRange(errorRangeString, 10); err != errInvalidRange { + t.Fatalf("expected: %s, got: %s", errInvalidRange, err) + } +} diff --git a/object-handlers.go b/object-handlers.go index faa42137a..97b2399b6 100644 --- a/object-handlers.go +++ b/object-handlers.go @@ -104,12 +104,18 @@ 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 { - writeErrorResponse(w, r, ErrInvalidRange, r.URL.Path) - return + 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. @@ -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.") diff --git a/server_test.go b/server_test.go index a520b4c28..ff6f64d6d 100644 --- a/server_test.go +++ b/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{}