Repository: fsouza/fake-gcs-server
Pull request #255 (2020-05-28 05:49:12)

Fix Range requests to correctly interpret last-byte-pos
I was having a problem with mocking `NewRangeReader`. My reads were giving me one-fewer bytes than I was expecting. I think I narrowed down why.

For Range request headers, per [RFC 7233](https://tools.ietf.org/html/rfc7233#section-4.2):

> `byte-range          = first-byte-pos "-" last-byte-pos`

In other words, the `byte-range` uses the `last-byte-pos`. This is subtly different than how Go slice indexing works [per the Go language spec](https://golang.org/ref/spec#Slice_expressions):

> The *indices* `low` and `high` select which elements of operand `a` appear in the result. The result has indices starting at 0 and length equal to `high - low`.

Therefore, we should take the `last-byte-pos` from the Range header and increment it by one to get the `high` that is expected by Go.

I created a test that illustrates the point. When I use the GCS `NewRangeReader` function, which has a `length` parameter, the old code would return `length-1` bytes.


Patch:
diff --git a/fakestorage/object.go b/fakestorage/object.go
index 7a3886a..f1851d3 100644
--- a/fakestorage/object.go
+++ b/fakestorage/object.go
@@ -395,9 +395,11 @@ func (s *Server) handleRange(obj Object, r *http.Request) (start, end int, conte
 			rangeParts := strings.SplitN(parts[1], "-", 2)
 			if len(rangeParts) == 2 {
 				start, _ = strconv.Atoi(rangeParts[0])
-				end, _ = strconv.Atoi(rangeParts[1])
-				if end < 1 {
+				var err error
+				if end, err = strconv.Atoi(rangeParts[1]); err != nil {
 					end = len(obj.Content)
+				} else {
+					end++
 				}
 				return start, end, obj.Content[start:end]
 			}


Test patch:
diff --git a/fakestorage/object_test.go b/fakestorage/object_test.go
index 4f2cd62..0d9308c 100644
--- a/fakestorage/object_test.go
+++ b/fakestorage/object_test.go
@@ -339,9 +339,9 @@ func TestServerClientObjectRangeReader(t *testing.T) {
 			t.Run(test.testCase, func(t *testing.T) {
 				length := test.length
 				if length == -1 {
-					length = int64(len(content)) - test.offset + 1
+					length = int64(len(content)) - test.offset
 				}
-				expectedData := content[test.offset : test.offset+length-1]
+				expectedData := content[test.offset : test.offset+length]
 				client := server.Client()
 				objHandle := client.Bucket(bucketName).Object(objectName)
 				reader, err := objHandle.NewRangeReader(context.TODO(), test.offset, test.length)
@@ -1260,3 +1260,51 @@ func checkObjectMetadata(actual, expected map[string]string, t *testing.T) {
 		}
 	}
 }
+
+func TestParseRangeRequest(t *testing.T) {
+	ctx := context.TODO()
+
+	in := []byte("this is a test object")
+
+	srv, _ := NewServerWithOptions(Options{
+		InitialObjects: []Object{
+			{
+				BucketName:  "test-bucket",
+				Name:        "test-object",
+				ContentType: "text/plain",
+				Content:     in,
+			},
+		},
+		NoListener: true,
+	})
+	obj := srv.Client().Bucket("test-bucket").Object("test-object")
+
+	var tests = []struct {
+		Start  int64
+		Length int64
+	}{
+		{4, 8},
+		{4, -1},
+		{0, 0},
+		{0, -1},
+		{0, 21},
+	}
+
+	for _, test := range tests {
+		start, length := test.Start, test.Length
+
+		rng, _ := obj.NewRangeReader(ctx, start, length)
+		out, _ := ioutil.ReadAll(rng)
+		rng.Close()
+
+		if length < 0 {
+			length = int64(len(in)) - start
+		}
+		if n := int64(len(out)); n != length {
+			t.Fatalf("expected %d bytes, RangeReader returned %d bytes", length, n)
+		}
+		if expected := in[start : start+length]; !bytes.Equal(expected, out) {
+			t.Fatalf("expected %q, RangeReader returned %q", expected, out)
+		}
+	}
+}