lib/http: return an error when parsing Content-Range

Instead of returning nil only, return the error to report the input
parameter back to the caller.

4 files changed, 54 insertions, 38 deletions

diff --git a/lib/http/range.go b/lib/http/range.go
index a52a545b..2a7d3b54 100644
--- a/lib/http/range.go
+++ b/lib/http/range.go
@@ -59,8 +59,9 @@ func ParseMultipartRange(body io.Reader, boundary string) (r *Range, err error)
 			continue
 		}
 
-		var pos = ParseContentRange(contentRange)
-		if pos == nil {
+		var pos *RangePosition
+		pos, err = ParseContentRange(contentRange)
+		if err != nil {
 			continue
 		}
 
diff --git a/lib/http/range_position.go b/lib/http/range_position.go
index a8835731..4f9624f0 100644
--- a/lib/http/range_position.go
+++ b/lib/http/range_position.go
@@ -35,49 +35,51 @@ type RangePosition struct {
 //	          end = 1*DIGIT
 //
 // It will return nil if the v is invalid.
-func ParseContentRange(v string) (pos *RangePosition) {
+func ParseContentRange(v string) (pos *RangePosition, err error) {
 	var (
-		p = libstrings.NewParser(v, ` `)
+		logp = `ParseContentRange`
+		p    = libstrings.NewParser(v, ` `)
 
 		tok   string
 		delim rune
-		err   error
 	)
 
 	pos = &RangePosition{}
 
 	tok, delim = p.ReadNoSpace()
 	if delim != ' ' {
-		return nil
+		return nil, fmt.Errorf(`%s: invalid Content-Range %q`, logp, v)
 	}
 
 	pos.unit = strings.ToLower(tok)
+	if !(pos.unit == AcceptRangesBytes || pos.unit == AcceptRangesNone) {
+		return nil, fmt.Errorf(`%s: invalid Content-Range %q`, logp, v)
+	}
 
 	p.SetDelimiters(`-/`)
 
 	tok, delim = p.ReadNoSpace()
 	if len(tok) == 0 {
-		return nil
+		return nil, fmt.Errorf(`%s: invalid Content-Range %q`, logp, v)
 	}
 	if tok == `*` {
 		if delim != '/' {
-			return nil
+			return nil, fmt.Errorf(`%s: invalid Content-Range %q`, logp, v)
 		}
 		tok, delim = p.ReadNoSpace()
 		if delim != 0 {
-			return nil
+			return nil, fmt.Errorf(`%s: invalid Content-Range %q`, logp, v)
 		}
 		if tok == `*` {
 			// "*/*": invalid range requested with unknown size.
-			pos = &RangePosition{}
-			return pos
+			return nil, fmt.Errorf(`%s: invalid Content-Range %q`, logp, v)
 		}
 
 		pos = &RangePosition{}
 		goto parselength
 	}
 	if delim != '-' {
-		return nil
+		return nil, fmt.Errorf(`%s: invalid Content-Range %q`, logp, v)
 	}
 
 	pos = &RangePosition{
@@ -88,39 +90,39 @@ func ParseContentRange(v string) (pos *RangePosition) {
 
 	*pos.start, err = strconv.ParseInt(tok, 10, 64)
 	if err != nil {
-		return nil
+		return nil, fmt.Errorf(`%s: invalid Content-Range %q: %w`, logp, v, err)
 	}
 
 	tok, delim = p.ReadNoSpace()
 	if delim != '/' {
-		return nil
+		return nil, fmt.Errorf(`%s: invalid Content-Range %q`, logp, v)
 	}
 	*pos.end, err = strconv.ParseInt(tok, 10, 64)
 	if err != nil {
-		return nil
+		return nil, fmt.Errorf(`%s: invalid Content-Range %q: %w`, logp, v, err)
 	}
 	if *pos.end < *pos.start {
-		return nil
+		return nil, fmt.Errorf(`%s: invalid Content-Range %q`, logp, v)
 	}
 
 	tok, delim = p.ReadNoSpace()
 	if delim != 0 {
-		return nil
+		return nil, fmt.Errorf(`%s: invalid Content-Range %q`, logp, v)
 	}
 	if tok == `*` {
 		// "x-y/*"
-		return pos
+		return pos, nil
 	}
 
 parselength:
 	*pos.length, err = strconv.ParseInt(tok, 10, 64)
 	if err != nil {
-		return nil
+		return nil, fmt.Errorf(`%s: invalid Content-Range %q: %w`, logp, v, err)
 	}
 	if *pos.length < 0 {
-		return nil
+		return nil, fmt.Errorf(`%s: invalid Content-Range %q`, logp, v)
 	}
-	return pos
+	return pos, nil
 }
 
 // Content return the range content body in multipart.
diff --git a/lib/http/range_position_example_test.go b/lib/http/range_position_example_test.go
index a0a7ed9f..8d8177c3 100644
--- a/lib/http/range_position_example_test.go
+++ b/lib/http/range_position_example_test.go
@@ -15,11 +15,11 @@ func ExampleParseContentRange() {
 	fmt.Println(libhttp.ParseContentRange(`bytes -10/x`))    // Invalid, invalid "size".
 	fmt.Println(libhttp.ParseContentRange(`bytes`))          // Invalid, missing position.
 	// Output:
-	// <nil>
-	// 10-19
-	// <nil>
-	// <nil>
-	// <nil>
-	// <nil>
-	// <nil>
+	// <nil> ParseContentRange: invalid Content-Range "bytes 10-/20": strconv.ParseInt: parsing "": invalid syntax
+	// 10-19 <nil>
+	// <nil> ParseContentRange: invalid Content-Range "bytes -10/20"
+	// <nil> ParseContentRange: invalid Content-Range "10-20/20"
+	// <nil> ParseContentRange: invalid Content-Range "bytes 10-"
+	// <nil> ParseContentRange: invalid Content-Range "bytes -10/x"
+	// <nil> ParseContentRange: invalid Content-Range "bytes"
 }
diff --git a/lib/http/range_position_test.go b/lib/http/range_position_test.go
index c2eae8f4..a4bc8429 100644
--- a/lib/http/range_position_test.go
+++ b/lib/http/range_position_test.go
@@ -8,32 +8,45 @@ import (
 
 func TestParseContentRange(t *testing.T) {
 	type testCase struct {
-		exp *RangePosition
-		v   string
+		exp      *RangePosition
+		expError string
+		v        string
 	}
 
 	var cases = []testCase{{
-		v: ``,
+		v:        ``,
+		expError: `ParseContentRange: invalid Content-Range ""`,
 	}, {
-		v: `bytes -1`,
+		v:        `bytes -1`,
+		expError: `ParseContentRange: invalid Content-Range "bytes -1"`,
 	}, {
-		v: `bytes -x/10`,
+		v:        `bytes -x/10`,
+		expError: `ParseContentRange: invalid Content-Range "bytes -x/10"`,
 	}, {
-		v: `bytes 1`,
+		v:        `bytes 1`,
+		expError: `ParseContentRange: invalid Content-Range "bytes 1"`,
 	}, {
-		v: `bytes x-10/10`,
+		v:        `bytes x-10/10`,
+		expError: `ParseContentRange: invalid Content-Range "bytes x-10/10": strconv.ParseInt: parsing "x": invalid syntax`,
 	}, {
-		v: `bytes 10-x/10`,
+		v:        `bytes 10-x/10`,
+		expError: `ParseContentRange: invalid Content-Range "bytes 10-x/10": strconv.ParseInt: parsing "x": invalid syntax`,
 	}, {
-		v: `bytes 10-20/20-`,
+		v:        `bytes 10-20/20-`,
+		expError: `ParseContentRange: invalid Content-Range "bytes 10-20/20-"`,
 	}}
 
 	var (
 		c   testCase
 		got *RangePosition
+		err error
 	)
 	for _, c = range cases {
-		got = ParseContentRange(c.v)
+		got, err = ParseContentRange(c.v)
+		if err != nil {
+			test.Assert(t, `error`, c.expError, err.Error())
+			continue
+		}
 		test.Assert(t, c.v, c.exp, got)
 	}
 }