From 282044d45426dabc3cc69478c8184c4e4d1885b3 Mon Sep 17 00:00:00 2001 From: Bala FA Date: Fri, 8 Jul 2016 03:35:18 +0530 Subject: [PATCH] http: handle possible range requests properly. (#2122) Previously range string is not validated against various combination of values. This patch fixes the issue. Fixes #2098 --- httprange.go | 74 +++++++++++++++++++++++++++++++++-------------- httprange_test.go | 31 +++++++++++++++++--- 2 files changed, 79 insertions(+), 26 deletions(-) diff --git a/httprange.go b/httprange.go index 8fda6efb6..23c898554 100644 --- a/httprange.go +++ b/httprange.go @@ -19,6 +19,7 @@ package main import ( "errors" "fmt" + "regexp" "strconv" "strings" ) @@ -37,6 +38,9 @@ func (e InvalidRange) Error() string { return "The requested range is not satisfiable" } +// Valid byte position regexp +var validBytePos = regexp.MustCompile(`^[0-9]+$`) + // HttpRange specifies the byte range to be sent to the client. type httpRange struct { firstBytePos int64 @@ -63,52 +67,78 @@ func parseRequestRange(rangeString string, size int64) (hrange *httpRange, err e // Trim byte range prefix. byteRangeString := strings.TrimPrefix(rangeString, byteRangePrefix) - // Check if range string contains delimiter '-', else return error. + // Check if range string contains delimiter '-', else return error. eg. "bytes=8" sepIndex := strings.Index(byteRangeString, "-") if sepIndex == -1 { - return nil, fmt.Errorf("invalid range string '%s'", rangeString) + return nil, fmt.Errorf("'%s' does not have a valid range value", rangeString) } 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 !validBytePos.MatchString(firstBytePosString) { + return nil, fmt.Errorf("'%s' does not have a valid first byte position value", rangeString) + } + 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, fmt.Errorf("'%s' does not have a valid first byte position value", rangeString) } } + lastBytePosString := byteRangeString[sepIndex+1:] + lastBytePos := int64(-1) // Convert lastBytePosString only if its not empty. if len(lastBytePosString) > 0 { + if !validBytePos.MatchString(lastBytePosString) { + return nil, fmt.Errorf("'%s' does not have a valid last byte position value", rangeString) + } + if lastBytePos, err = strconv.ParseInt(lastBytePosString, 10, 64); err != nil { - return nil, fmt.Errorf("'%s' does not have valid last byte position value", rangeString) + return nil, fmt.Errorf("'%s' does not have a valid last byte position value", rangeString) } } - // 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) - } + // rangeString contains first and last byte positions. eg. "bytes=2-5" + if firstBytePos > -1 && lastBytePos > -1 { + if firstBytePos > lastBytePos { + // Last byte position is not greater than first byte position. eg. "bytes=5-2" + return nil, fmt.Errorf("'%s' does not have valid range value", rangeString) + } - if firstBytePos == -1 { - // Return error if lastBytePos is zero and firstBytePos is not given eg. "bytes=-0" - if lastBytePos == 0 { + // First and last byte positions should not be >= size. + if firstBytePos >= size { return nil, errInvalidRange } - firstBytePos = size - lastBytePos + + if lastBytePos >= size { + lastBytePos = size - 1 + } + } else if firstBytePos > -1 { + // rangeString contains only first byte position. eg. "bytes=8-" + if firstBytePos >= size { + // First byte position should not be >= size. + return nil, errInvalidRange + } + lastBytePos = size - 1 - } else if lastBytePos == -1 { + } else if lastBytePos > -1 { + // rangeString contains only last byte position. eg. "bytes=-3" + if lastBytePos == 0 { + // Last byte position should not be zero eg. "bytes=-0" + return nil, errInvalidRange + } + + if lastBytePos >= size { + firstBytePos = 0 + } else { + firstBytePos = size - lastBytePos + } + lastBytePos = size - 1 - } else if firstBytePos > lastBytePos { - // Return error if firstBytePos is greater than lastBytePos + } else { + // rangeString contains first and last byte positions missing. eg. "bytes=-" 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 &httpRange{firstBytePos, lastBytePos, size}, nil diff --git a/httprange_test.go b/httprange_test.go index d9952ebed..5144be8ef 100644 --- a/httprange_test.go +++ b/httprange_test.go @@ -29,8 +29,11 @@ func TestParseRequestRange(t *testing.T) { }{ {"bytes=2-5", 2, 5, 4}, {"bytes=2-20", 2, 9, 8}, + {"bytes=2-2", 2, 2, 1}, + {"bytes=0000-0006", 0, 6, 7}, {"bytes=2-", 2, 9, 8}, {"bytes=-4", 6, 9, 4}, + {"bytes=-20", 0, 9, 10}, } for _, successCase := range successCases { @@ -52,7 +55,20 @@ func TestParseRequestRange(t *testing.T) { } // 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"} + invalidRangeStrings := []string{ + "bytes=8", + "bytes=5-2", + "bytes=+2-5", + "bytes=2-+5", + "bytes=2--5", + "bytes=-", + "", + "2-5", + "bytes = 2-5", + "bytes=2 - 5", + "bytes=0-0,-1", + "bytes=2-5 ", + } for _, rangeString := range invalidRangeStrings { if _, err := parseRequestRange(rangeString, 10); err == nil { t.Fatalf("expected: an error, got: ") @@ -60,8 +76,15 @@ func TestParseRequestRange(t *testing.T) { } // Test error range strings. - errorRangeString := "bytes=-0" - if _, err := parseRequestRange(errorRangeString, 10); err != errInvalidRange { - t.Fatalf("expected: %s, got: %s", errInvalidRange, err) + errorRangeString := []string{ + "bytes=10-10", + "bytes=20-30", + "bytes=20-", + "bytes=-0", + } + for _, rangeString := range errorRangeString { + if _, err := parseRequestRange(rangeString, 10); err != errInvalidRange { + t.Fatalf("expected: %s, got: %s", errInvalidRange, err) + } } }