Fix support resumable + fix incorrect handling of paths which start w…

…ith a / (#918) * update support * update support * update support * fix upload * fix routes order * Fix normalizeFileName * Fix escaping for mux vars * Fix escaping for mux vars * Fix escaping for mux vars * fix check and tests * move unescapeMuxVars to server * add new test cases * add new test cases * Update fakestorage/upload.go Co-authored-by: fsouza <[email protected]> * remove bucket escaping Co-authored-by: fsouza <[email protected]>
fsouza · Sep 19, 2022 · 071372e · 071372e
1 parent 6318cf7
commit 071372e
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 21 deletions.
diff --git a/fakestorage/bucket.go b/fakestorage/bucket.go
@@ -105,7 +105,7 @@ func (s *Server) listBuckets(r *http.Request) jsonResponse {
 }
 
 func (s *Server) getBucket(r *http.Request) jsonResponse {
-	bucketName := mux.Vars(r)["bucketName"]
+	bucketName := unescapeMuxVars(mux.Vars(r))["bucketName"]
 	bucket, err := s.backend.GetBucket(bucketName)
 	if err != nil {
 		return jsonResponse{status: http.StatusNotFound}
@@ -114,7 +114,7 @@ func (s *Server) getBucket(r *http.Request) jsonResponse {
 }
 
 func (s *Server) deleteBucket(r *http.Request) jsonResponse {
-	bucketName := mux.Vars(r)["bucketName"]
+	bucketName := unescapeMuxVars(mux.Vars(r))["bucketName"]
 	err := s.backend.DeleteBucket(bucketName)
 	if err == backend.BucketNotFound {
 		return jsonResponse{status: http.StatusNotFound}

diff --git a/fakestorage/object.go b/fakestorage/object.go
@@ -565,7 +565,7 @@ func (s *Server) objectWithGenerationOnValidGeneration(bucketName, objectName, g
 }
 
 func (s *Server) listObjects(r *http.Request) jsonResponse {
-	bucketName := mux.Vars(r)["bucketName"]
+	bucketName := unescapeMuxVars(mux.Vars(r))["bucketName"]
 	objs, prefixes, err := s.ListObjectsWithOptions(bucketName, ListOptions{
 		Prefix:                   r.URL.Query().Get("prefix"),
 		Delimiter:                r.URL.Query().Get("delimiter"),
@@ -587,7 +587,7 @@ func (s *Server) getObject(w http.ResponseWriter, r *http.Request) {
 	}
 
 	handler := jsonToHTTPHandler(func(r *http.Request) jsonResponse {
-		vars := mux.Vars(r)
+		vars := unescapeMuxVars(mux.Vars(r))
 
 		obj, err := s.objectWithGenerationOnValidGeneration(vars["bucketName"], vars["objectName"], r.FormValue("generation"))
 		// Calling Close before checking err is okay on objects, and the object
@@ -617,7 +617,7 @@ func (s *Server) getObject(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) deleteObject(r *http.Request) jsonResponse {
-	vars := mux.Vars(r)
+	vars := unescapeMuxVars(mux.Vars(r))
 	obj, err := s.GetObjectStreaming(vars["bucketName"], vars["objectName"])
 	// Calling Close before checking err is okay on objects, and the object
 	// may need to be closed whether or not there's an error.
@@ -639,7 +639,7 @@ func (s *Server) deleteObject(r *http.Request) jsonResponse {
 }
 
 func (s *Server) listObjectACL(r *http.Request) jsonResponse {
-	vars := mux.Vars(r)
+	vars := unescapeMuxVars(mux.Vars(r))
 
 	obj, err := s.GetObjectStreaming(vars["bucketName"], vars["objectName"])
 	if err != nil {
@@ -651,7 +651,7 @@ func (s *Server) listObjectACL(r *http.Request) jsonResponse {
 }
 
 func (s *Server) setObjectACL(r *http.Request) jsonResponse {
-	vars := mux.Vars(r)
+	vars := unescapeMuxVars(mux.Vars(r))
 
 	obj, err := s.GetObjectStreaming(vars["bucketName"], vars["objectName"])
 	if err != nil {
@@ -689,7 +689,7 @@ func (s *Server) setObjectACL(r *http.Request) jsonResponse {
 }
 
 func (s *Server) rewriteObject(r *http.Request) jsonResponse {
-	vars := mux.Vars(r)
+	vars := unescapeMuxVars(mux.Vars(r))
 	obj, err := s.objectWithGenerationOnValidGeneration(vars["sourceBucket"], vars["sourceObject"], r.FormValue("sourceGeneration"))
 	// Calling Close before checking err is okay on objects, and the object
 	// may need to be closed whether or not there's an error.
@@ -744,7 +744,7 @@ func (s *Server) rewriteObject(r *http.Request) jsonResponse {
 }
 
 func (s *Server) downloadObject(w http.ResponseWriter, r *http.Request) {
-	vars := mux.Vars(r)
+	vars := unescapeMuxVars(mux.Vars(r))
 	obj, err := s.objectWithGenerationOnValidGeneration(vars["bucketName"], vars["objectName"], r.FormValue("generation"))
 	// Calling Close before checking err is okay on objects, and the object
 	// may need to be closed whether or not there's an error.
@@ -901,7 +901,7 @@ func parseRange(rangeHeaderValue string, contentLength int64) (start int64, end
 }
 
 func (s *Server) patchObject(r *http.Request) jsonResponse {
-	vars := mux.Vars(r)
+	vars := unescapeMuxVars(mux.Vars(r))
 	bucketName := vars["bucketName"]
 	objectName := vars["objectName"]
 	var metadata struct {
@@ -928,7 +928,7 @@ func (s *Server) patchObject(r *http.Request) jsonResponse {
 }
 
 func (s *Server) updateObject(r *http.Request) jsonResponse {
-	vars := mux.Vars(r)
+	vars := unescapeMuxVars(mux.Vars(r))
 	bucketName := vars["bucketName"]
 	objectName := vars["objectName"]
 	var metadata struct {
@@ -955,7 +955,7 @@ func (s *Server) updateObject(r *http.Request) jsonResponse {
 }
 
 func (s *Server) composeObject(r *http.Request) jsonResponse {
-	vars := mux.Vars(r)
+	vars := unescapeMuxVars(mux.Vars(r))
 	bucketName := vars["bucketName"]
 	destinationObject := vars["destinationObject"]
 

diff --git a/fakestorage/server.go b/fakestorage/server.go
@@ -19,6 +19,7 @@ import (
 	"net/http/httptest"
 	"net/http/httputil"
 	"net/textproto"
+	"net/url"
 	"strings"
 	"sync"
 
@@ -211,9 +212,22 @@ func newServer(options Options) (*Server, error) {
 	return &s, nil
 }
 
+func unescapeMuxVars(vars map[string]string) map[string]string {
+	m := make(map[string]string)
+	for k, v := range vars {
+		r, err := url.PathUnescape(v)
+		if err == nil {
+			m[k] = r
+		} else {
+			m[k] = v
+		}
+	}
+	return m
+}
+
 func (s *Server) buildMuxer() {
 	const apiPrefix = "/storage/v1"
-	s.mux = mux.NewRouter()
+	s.mux = mux.NewRouter().SkipClean(true).UseEncodedPath()
 
 	// healthcheck
 	s.mux.Path("/_internal/healthcheck").Methods(http.MethodGet).HandlerFunc(s.healthcheck)
@@ -251,6 +265,7 @@ func (s *Server) buildMuxer() {
 	s.mux.Host(bucketHost).Path("/{objectName:.+}").Methods(http.MethodGet, http.MethodHead).HandlerFunc(s.downloadObject)
 	s.mux.Path("/download/storage/v1/b/{bucketName}/o/{objectName:.+}").Methods(http.MethodGet).HandlerFunc(s.downloadObject)
 	s.mux.Path("/upload/storage/v1/b/{bucketName}/o").Methods(http.MethodPost).HandlerFunc(jsonToHTTPHandler(s.insertObject))
+	s.mux.Path("/upload/storage/v1/b/{bucketName}/o").Methods(http.MethodPut).HandlerFunc(jsonToHTTPHandler(s.uploadFileContent))
 	s.mux.Path("/upload/resumable/{uploadId}").Methods(http.MethodPut, http.MethodPost).HandlerFunc(jsonToHTTPHandler(s.uploadFileContent))
 
 	// Batch endpoint

diff --git a/fakestorage/upload.go b/fakestorage/upload.go
@@ -14,6 +14,7 @@ import (
 	"mime"
 	"mime/multipart"
 	"net/http"
+	"net/url"
 	"strconv"
 	"strings"
 
@@ -46,7 +47,7 @@ type contentRange struct {
 }
 
 func (s *Server) insertObject(r *http.Request) jsonResponse {
-	bucketName := mux.Vars(r)["bucketName"]
+	bucketName := unescapeMuxVars(mux.Vars(r))["bucketName"]
 
 	if _, err := s.backend.GetBucket(bucketName); err != nil {
 		return jsonResponse{status: http.StatusNotFound}
@@ -78,7 +79,7 @@ func (s *Server) insertObject(r *http.Request) jsonResponse {
 }
 
 func (s *Server) insertFormObject(r *http.Request) xmlResponse {
-	bucketName := mux.Vars(r)["bucketName"]
+	bucketName := unescapeMuxVars(mux.Vars(r))["bucketName"]
 
 	if err := r.ParseMultipartForm(32 << 20); nil != err {
 		return xmlResponse{errorMessage: "invalid form", status: http.StatusBadRequest}
@@ -242,7 +243,7 @@ func (s notImplementedSeeker) Seek(offset int64, whence int) (int64, error) {
 
 func (s *Server) signedUpload(bucketName string, r *http.Request) jsonResponse {
 	defer r.Body.Close()
-	name := mux.Vars(r)["objectName"]
+	name := unescapeMuxVars(mux.Vars(r))["objectName"]
 	predefinedACL := r.URL.Query().Get("predefinedAcl")
 	contentEncoding := r.URL.Query().Get("contentEncoding")
 
@@ -362,6 +363,9 @@ func (s *Server) multipartUpload(bucketName string, r *http.Request) jsonRespons
 }
 
 func (s *Server) resumableUpload(bucketName string, r *http.Request) jsonResponse {
+	if r.URL.Query().Has("upload_id") {
+		return s.uploadFileContent(r)
+	}
 	predefinedACL := r.URL.Query().Get("predefinedAcl")
 	contentEncoding := r.URL.Query().Get("contentEncoding")
 	metadata := new(multipartMetadata)
@@ -391,9 +395,16 @@ func (s *Server) resumableUpload(bucketName string, r *http.Request) jsonRespons
 	}
 	s.uploads.Store(uploadID, obj)
 	header := make(http.Header)
-	header.Set("Location", s.URL()+"/upload/resumable/"+uploadID)
+	location := fmt.Sprintf(
+		"%s/upload/storage/v1/b/%s/o?uploadType=resumable&name=%s&upload_id=%s",
+		s.URL(),
+		bucketName,
+		url.PathEscape(objName),
+		uploadID,
+	)
+	header.Set("Location", location)
 	if r.Header.Get("X-Goog-Upload-Command") == "start" {
-		header.Set("X-Goog-Upload-URL", s.URL()+"/upload/resumable/"+uploadID)
+		header.Set("X-Goog-Upload-URL", location)
 		header.Set("X-Goog-Upload-Status", "active")
 	}
 	return jsonResponse{
@@ -438,7 +449,7 @@ func (s *Server) resumableUpload(bucketName string, r *http.Request) jsonRespons
 // then has a status of "200 OK", with a header "X-Http-Status-Code-Override"
 // set to "308".
 func (s *Server) uploadFileContent(r *http.Request) jsonResponse {
-	uploadID := mux.Vars(r)["uploadId"]
+	uploadID := r.URL.Query().Get("upload_id")
 	rawObj, ok := s.uploads.Load(uploadID)
 	if !ok {
 		return jsonResponse{status: http.StatusNotFound}

diff --git a/fakestorage/upload_test.go b/fakestorage/upload_test.go
@@ -48,6 +48,18 @@ func TestServerClientObjectWriter(t *testing.T) {
 				"other/interesting/object.txt",
 				googleapi.MinUploadChunkSize,
 			},
+			{
+				"file with backslash at beginning",
+				"other-bucket",
+				"/some/other/object.txt",
+				googleapi.DefaultUploadChunkSize,
+			},
+			{
+				"file with backslashes at name",
+				"other-bucket",
+				"//some//other//file.txt",
+				googleapi.MinUploadChunkSize,
+			},
 		}
 
 		for _, test := range tests {
@@ -793,8 +805,8 @@ func TestServerGzippedUpload(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			url := server.URL()
-			req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("%s/upload/storage/v1/b/%s/o?name=testobj&uploadType=media", url, bucketName), &buf)
+			serverUrl := server.URL()
+			req, err := http.NewRequest(http.MethodPost, fmt.Sprintf("%s/upload/storage/v1/b/%s/o?name=testobj&uploadType=media", serverUrl, bucketName), &buf)
 			if err != nil {
 				t.Fatal(err)
 			}