Repository: fsouza/fake-gcs-server
Pull request #39 (2019-06-11 17:16:09)

Update uploadFileContent for Python client
This updates ``uploadFileContent`` to handle the needs of both the Go and Python clients when uploading over multiple requests. It rewrites ``parseRange`` as ``parseContentRange``, to parse the header content and leave interpretation and setting response headers to ``uploadFileContent``.

The Go client always sends content as one or more chunks without a total size, and, when the stream is exhausted, sends a final request with no content but the final size.

The Go client also sends the header ``X-Guploader-No-308``. This specifies that the response to a "mid-stream" request should be sent with a status code ``200 OK`` plus the header ``X-Http-Status-Code-Override: 308``.

The Python client sends chunks with a total size when known, such as when uploading a file or an in-memory string. Unlike a streaming response, the final request contains content. and the final request contains data. It expects a mid-stream response to have a status code ``308 Permanent Redirect``.

The Python client is pickier about some response details, and ``uploadFileContent``'s responses are changed to work with both the Python and (more flexible) Go clients:

 * The Python client requires the ``Range`` response header to start counting at 0, such as ``bytes 0-999``, while the Go client appears to ignore the start byte.
 * The Python client requires that the final response is a ``200 OK``, while the Go client accepts this or a ``201 Created``.


Patch:
diff --git a/fakestorage/upload.go b/fakestorage/upload.go
index 0c08ad2..ac2dcc0 100644
--- a/fakestorage/upload.go
+++ b/fakestorage/upload.go
@@ -27,6 +27,14 @@ type multipartMetadata struct {
 	Name string `json:"name"`
 }
 
+type contentRange struct {
+	KnownRange bool // Is the range known, or "*"?
+	KnownTotal bool // Is the total known, or "*"?
+	Start      int  // Start of the range, -1 if unknown
+	End        int  // End of the range, -1 if unknown
+	Total      int  // Total bytes expected, -1 if unknown
+}
+
 func (s *Server) insertObject(w http.ResponseWriter, r *http.Request) {
 	bucketName := mux.Vars(r)["bucketName"]
 	if err := s.backend.GetBucket(bucketName); err != nil {
@@ -160,6 +168,41 @@ func (s *Server) resumableUpload(bucketName string, w http.ResponseWriter, r *ht
 	json.NewEncoder(w).Encode(obj)
 }
 
+// uploadFileContent accepts a chunk of a resumable upload
+//
+// A resumable upload is sent in one or more chunks. The request's
+// "Content-Range" header is used to determine if more data is expected.
+//
+// When sending streaming content, the total size is unknown until the stream
+// is exhausted. The Go client always sends streaming content. The sequence of
+// "Content-Range" headers for 2600-byte content sent in 1000-byte chunks are:
+//
+//   Content-Range: bytes 0-999/*
+//   Content-Range: bytes 1000-1999/*
+//   Content-Range: bytes 2000-2599/*
+//   Content-Range: bytes */2600
+//
+// When sending chunked content of a known size, the total size is sent as
+// well. The Python client uses this method to upload files and in-memory
+// content. The sequence of "Content-Range" headers for the 2600-byte content
+// sent in 1000-byte chunks are:
+//
+//   Content-Range: bytes 0-999/2600
+//   Content-Range: bytes 1000-1999/2600
+//   Content-Range: bytes 2000-2599/2600
+//
+// The server collects the content, analyzes the "Content-Range", and returns a
+// "308 Permanent Redirect" response if more chunks are expected, and a
+// "200 OK" response if the upload is complete (the Go client also accepts a
+// "201 Created" response). The "Range" header in the response should be set to
+// the size of the content received so far, such as:
+//
+//   Range: bytes 0-2000
+//
+// The client (such as the Go client) can send a header "X-Guploader-No-308" if
+// it can't process a native "308 Permanent Redirect". The in-process response
+// then has a status of "200 OK", with a header "X-Http-Status-Code-Override"
+// set to "308".
 func (s *Server) uploadFileContent(w http.ResponseWriter, r *http.Request) {
 	uploadID := mux.Vars(r)["uploadId"]
 	rawObj, ok := s.uploads.Load(uploadID)
@@ -174,17 +217,25 @@ func (s *Server) uploadFileContent(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	commit := true
-	status := http.StatusCreated
-	objLength := len(obj.Content)
+	status := http.StatusOK
 	obj.Content = append(obj.Content, content...)
 	obj.Crc32c = encodedCrc32cChecksum(obj.Content)
 	obj.Md5Hash = encodedMd5Hash(obj.Content)
 	if contentRange := r.Header.Get("Content-Range"); contentRange != "" {
-		commit, err = parseRange(contentRange, objLength, len(content), w)
+		parsed, err := parseContentRange(contentRange)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
+		if parsed.KnownRange {
+			// Middle of streaming request, or any part of chunked request
+			w.Header().Set("Range", fmt.Sprintf("bytes=0-%d", parsed.End))
+			// Complete if the range covers the known total
+			commit = parsed.KnownTotal && (parsed.End+1 >= parsed.Total)
+		} else {
+			// End of a streaming request
+			w.Header().Set("Range", fmt.Sprintf("bytes=0-%d", len(obj.Content)))
+		}
 	}
 	if commit {
 		s.uploads.Delete(uploadID)
@@ -194,8 +245,13 @@ func (s *Server) uploadFileContent(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 	} else {
-		status = http.StatusOK
-		w.Header().Set("X-Http-Status-Code-Override", "308")
+		if _, no308 := r.Header["X-Guploader-No-308"]; no308 {
+			// Go client
+			w.Header().Set("X-Http-Status-Code-Override", "308")
+		} else {
+			// Python client
+			status = http.StatusPermanentRedirect
+		}
 		s.uploads.Store(uploadID, obj)
 	}
 	data, _ := json.Marshal(obj)
@@ -205,42 +261,62 @@ func (s *Server) uploadFileContent(w http.ResponseWriter, r *http.Request) {
 	w.Write(data)
 }
 
-func parseRange(r string, objLength, bodyLength int, w http.ResponseWriter) (finished bool, err error) {
+// Parse a Content-Range header
+// Some possible valid header values:
+//   bytes 0-1023/4096 (first 1024 bytes of a 4096-byte document)
+//   bytes 1024-2047/* (second 1024 bytes of a streaming document)
+//   bytes */4096      (The end of 4096 byte streaming document)
+func parseContentRange(r string) (parsed contentRange, err error) {
 	invalidErr := fmt.Errorf("invalid Content-Range: %v", r)
+
+	// Require that units == "bytes"
 	const bytesPrefix = "bytes "
-	var contentLength int
 	if !strings.HasPrefix(r, bytesPrefix) {
-		return false, invalidErr
+		return parsed, invalidErr
 	}
+
+	// Split range from total length
 	parts := strings.SplitN(r[len(bytesPrefix):], "/", 2)
 	if len(parts) != 2 {
-		return false, invalidErr
+		return parsed, invalidErr
 	}
-	var rangeStart, rangeEnd int
 
+	// Process range
 	if parts[0] == "*" {
-		rangeStart = objLength
-		rangeEnd = objLength + bodyLength
+		parsed.Start = -1
+		parsed.End = -1
 	} else {
 		rangeParts := strings.SplitN(parts[0], "-", 2)
 		if len(rangeParts) != 2 {
-			return false, invalidErr
+			return parsed, invalidErr
 		}
-		rangeStart, err = strconv.Atoi(rangeParts[0])
+		parsed.KnownRange = true
+		parsed.Start, err = strconv.Atoi(rangeParts[0])
 		if err != nil {
-			return false, invalidErr
+			return parsed, invalidErr
 		}
-		rangeEnd, err = strconv.Atoi(rangeParts[1])
+		parsed.End, err = strconv.Atoi(rangeParts[1])
 		if err != nil {
-			return false, invalidErr
+			return parsed, invalidErr
 		}
 	}
 
-	contentLength = objLength + bodyLength
-	finished = rangeEnd == contentLength
-	w.Header().Set("Range", fmt.Sprintf("bytes=%d-%d", rangeStart, rangeEnd))
+	// Process total length
+	if parts[1] == "*" {
+		parsed.Total = -1
+		if !parsed.KnownRange {
+			// Must know either range or total
+			return parsed, invalidErr
+		}
+	} else {
+		parsed.KnownTotal = true
+		parsed.Total, err = strconv.Atoi(parts[1])
+		if err != nil {
+			return parsed, invalidErr
+		}
+	}
 
-	return finished, nil
+	return parsed, nil
 }
 
 func loadMetadata(rc io.ReadCloser) (*multipartMetadata, error) {


Test patch:
diff --git a/fakestorage/upload_test.go b/fakestorage/upload_test.go
index 6cfca7d..3178fd0 100644
--- a/fakestorage/upload_test.go
+++ b/fakestorage/upload_test.go
@@ -219,3 +219,58 @@ func TestServerInvalidUploadType(t *testing.T) {
 		t.Errorf("wrong status returned\nwant %d\ngot  %d", expectedStatus, resp.StatusCode)
 	}
 }
+
+func TestParseContentRange(t *testing.T) {
+	t.Parallel()
+	var goodHeaderTests = []struct {
+		header string
+		output contentRange
+	}{
+		{
+			"bytes */1024", // End of a streaming request, total is now known
+			contentRange{KnownTotal: true, Start: -1, End: -1, Total: 1024},
+		},
+		{
+			"bytes 1024-2047/4096", // Range with a known total
+			contentRange{KnownRange: true, KnownTotal: true, Start: 1024, End: 2047, Total: 4096},
+		},
+		{
+			"bytes 0-1024/*", // A streaming request, unknown total
+			contentRange{KnownRange: true, Start: 0, End: 1024, Total: -1},
+		},
+	}
+
+	for _, test := range goodHeaderTests {
+		test := test
+		t.Run(test.header, func(t *testing.T) {
+			t.Parallel()
+			output, err := parseContentRange(test.header)
+			if output != test.output {
+				t.Fatalf("output is different.\nexpected: %+v\n  actual: %+v\n", test.output, output)
+			}
+			if err != nil {
+				t.Fatal(err)
+			}
+		})
+	}
+
+	var badHeaderTests = []string{
+		"none",                // Unsupported unit "none"
+		"bytes 20",            // No slash to split range from size
+		"bytes 1/4",           // Single-field range
+		"bytes start-20/100",  // Non-integer range start
+		"bytes 20-end/100",    // Non-integer range end
+		"bytes 100-200/total", // Non-integer size
+		"bytes */*",           // Unknown range or size
+	}
+	for _, test := range badHeaderTests {
+		test := test
+		t.Run(test, func(t *testing.T) {
+			t.Parallel()
+			_, err := parseContentRange(test)
+			if err == nil {
+				t.Fatalf("Expected err!=<nil>, but was %v", err)
+			}
+		})
+	}
+}