diff --git a/cmd/acl-handlers.go b/cmd/acl-handlers.go index 9edfaa62f..9c21f1855 100644 --- a/cmd/acl-handlers.go +++ b/cmd/acl-handlers.go @@ -19,6 +19,7 @@ package cmd import ( "encoding/xml" "net/http" + "net/url" "github.com/gorilla/mux" "github.com/minio/minio/cmd/logger" @@ -110,7 +111,11 @@ func (api objectAPIHandlers) GetObjectACLHandler(w http.ResponseWriter, r *http. vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } objAPI := api.ObjectAPI() if objAPI == nil { @@ -126,7 +131,7 @@ func (api objectAPIHandlers) GetObjectACLHandler(w http.ResponseWriter, r *http. } // Before proceeding validate if object exists. - _, err := objAPI.GetObjectInfo(ctx, bucket, object, ObjectOptions{}) + _, err = objAPI.GetObjectInfo(ctx, bucket, object, ObjectOptions{}) if err != nil { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) return diff --git a/cmd/api-errors.go b/cmd/api-errors.go index 30b0bf5b0..9f6bce267 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -21,6 +21,7 @@ import ( "encoding/xml" "fmt" "net/http" + "net/url" "strings" "github.com/Azure/azure-storage-blob-go/azblob" @@ -1802,6 +1803,13 @@ func toAPIError(ctx context.Context, err error) APIError { // their internal error types. This code is only // useful with gateway implementations. switch e := err.(type) { + case url.EscapeError: + apiErr = APIError{ + Code: "XMinioInvalidObjectName", + Description: fmt.Sprintf("%s (%s)", errorCodes[ErrInvalidObjectName].Description, + e.Error()), + HTTPStatusCode: http.StatusBadRequest, + } case lifecycle.Error: apiErr = APIError{ Code: "InvalidRequest", diff --git a/cmd/fs-v1_test.go b/cmd/fs-v1_test.go index 168ba7ec8..18a764239 100644 --- a/cmd/fs-v1_test.go +++ b/cmd/fs-v1_test.go @@ -280,7 +280,7 @@ func TestFSDeleteObject(t *testing.T) { t.Fatal("Unexpected error: ", err) } // Test with invalid object name - if err := fs.DeleteObject(context.Background(), bucketName, "\\"); !isSameType(err, ObjectNameInvalid{}) { + if err := fs.DeleteObject(context.Background(), bucketName, "\\"); !isSameType(err, ObjectNotFound{}) { t.Fatal("Unexpected error: ", err) } // Test with object does not exist. diff --git a/cmd/gateway-main.go b/cmd/gateway-main.go index 63744d38c..117d57c5f 100644 --- a/cmd/gateway-main.go +++ b/cmd/gateway-main.go @@ -156,7 +156,10 @@ func StartGateway(ctx *cli.Context, gw Gateway) { globalServerConfig = srvCfg globalServerConfigMu.Unlock() - router := mux.NewRouter().SkipClean(true) + // Initialize router. `SkipClean(true)` stops gorilla/mux from + // normalizing URL path minio/minio#3256 + // avoid URL path encoding minio/minio#8950 + router := mux.NewRouter().SkipClean(true).UseEncodedPath() if globalEtcdClient != nil { // Enable STS router if etcd is enabled. diff --git a/cmd/logger/audit.go b/cmd/logger/audit.go index 13f1a7577..1a0638aa2 100644 --- a/cmd/logger/audit.go +++ b/cmd/logger/audit.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "net/http" + "net/url" "time" "github.com/gorilla/mux" @@ -143,7 +144,10 @@ func AuditLog(w http.ResponseWriter, r *http.Request, api string, reqClaims map[ vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + object = vars["object"] + } // Send audit logs only to http targets. for _, t := range AuditTargets { diff --git a/cmd/object-api-listobjects_test.go b/cmd/object-api-listobjects_test.go index de6e28248..e792e5256 100644 --- a/cmd/object-api-listobjects_test.go +++ b/cmd/object-api-listobjects_test.go @@ -552,7 +552,7 @@ func testListObjects(obj ObjectLayer, instanceType string, t1 TestErrHandler) { // Test with prefix and delimiter set to '/'. (60) {"test-bucket-list-object", SlashSeparator, "", SlashSeparator, 10, resultCases[30], nil, true}, // Test with invalid prefix (61) - {"test-bucket-list-object", "\\", "", SlashSeparator, 10, ListObjectsInfo{}, ObjectNameInvalid{Bucket: "test-bucket-list-object", Object: "\\"}, false}, + {"test-bucket-list-object", "\\", "", SlashSeparator, 10, ListObjectsInfo{}, nil, true}, // Test listing an empty directory in recursive mode (62) {"test-bucket-empty-dir", "", "", "", 10, resultCases[31], nil, true}, // Test listing an empty directory in a non recursive mode (63) diff --git a/cmd/object-api-multipart_test.go b/cmd/object-api-multipart_test.go index 0a3af1add..f93fee813 100644 --- a/cmd/object-api-multipart_test.go +++ b/cmd/object-api-multipart_test.go @@ -61,17 +61,12 @@ func testObjectNewMultipartUpload(obj ObjectLayer, instanceType string, t TestEr t.Fatalf("%s : %s", instanceType, err.Error()) } - _, err = obj.NewMultipartUpload(context.Background(), bucket, "\\", opts) - if err == nil { - t.Fatalf("%s: Expected to fail since object name is invalid.", instanceType) - } - - uploadID, err := obj.NewMultipartUpload(context.Background(), bucket, object, opts) + uploadID, err := obj.NewMultipartUpload(context.Background(), bucket, "\\", opts) if err != nil { t.Fatalf("%s : %s", instanceType, err.Error()) } - err = obj.AbortMultipartUpload(context.Background(), bucket, object, uploadID) + err = obj.AbortMultipartUpload(context.Background(), bucket, "\\", uploadID) if err != nil { switch err.(type) { case InvalidUploadID: @@ -112,9 +107,9 @@ func testObjectAbortMultipartUpload(obj ObjectLayer, instanceType string, t Test expectedErrType error }{ {"--", object, uploadID, BucketNotFound{}}, - {bucket, "\\", uploadID, ObjectNameInvalid{}}, {"foo", object, uploadID, BucketNotFound{}}, {bucket, object, "foo-foo", InvalidUploadID{}}, + {bucket, "\\", uploadID, InvalidUploadID{}}, {bucket, object, uploadID, nil}, } // Iterating over creatPartCases to generate multipart chunks. diff --git a/cmd/object-api-utils.go b/cmd/object-api-utils.go index 58b2ac555..f9225d8fa 100644 --- a/cmd/object-api-utils.go +++ b/cmd/object-api-utils.go @@ -165,10 +165,6 @@ func IsValidObjectPrefix(object string) bool { if !utf8.ValidString(object) { return false } - // Reject unsupported characters in object name. - if strings.ContainsAny(object, `\`) { - return false - } if strings.Contains(object, `//`) { return false } diff --git a/cmd/object-api-utils_test.go b/cmd/object-api-utils_test.go index 0d9f9e0a8..3ca33ce11 100644 --- a/cmd/object-api-utils_test.go +++ b/cmd/object-api-utils_test.go @@ -105,13 +105,24 @@ func TestIsValidObjectName(t *testing.T) { {"f*le", true}, {"contains-^-carret", true}, {"contains-|-pipe", true}, - {"contains-\"-quote", true}, {"contains-`-tick", true}, {"..test", true}, {".. test", true}, {". test", true}, {".test", true}, {"There are far too many object names, and far too few bucket names!", true}, + {"!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~/!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~)", true}, + {"!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~", true}, + {"␀␁␂␃␄␅␆␇␈␉␊␋␌␍␎␏␐␑␒␓␔␕␖␗␘␙␚␛␜␝␞␟␡", true}, + {"trailing VT␋/trailing VT␋", true}, + {"␋leading VT/␋leading VT", true}, + {"~leading tilde", true}, + {"\rleading CR", true}, + {"\nleading LF", true}, + {"\tleading HT", true}, + {"trailing CR\r", true}, + {"trailing LF\n", true}, + {"trailing HT\t", true}, // cases for which test should fail. // passing invalid object names. {"", false}, @@ -122,7 +133,6 @@ func TestIsValidObjectName(t *testing.T) { {" ../etc", false}, {"./././", false}, {"./etc", false}, - {`contains-\-backslash`, false}, {`contains//double/forwardslash`, false}, {`//contains/double-forwardslash-prefix`, false}, {string([]byte{0xff, 0xfe, 0xfd}), false}, diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index fae7a222f..40216a44e 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -109,7 +109,11 @@ func (api objectAPIHandlers) SelectObjectContentHandler(w http.ResponseWriter, r } vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } // get gateway encryption options opts, err := getOpts(ctx, r, bucket, object) @@ -273,7 +277,11 @@ func (api objectAPIHandlers) GetObjectHandler(w http.ResponseWriter, r *http.Req } vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } if vid := r.URL.Query().Get("versionId"); vid != "" && vid != "null" { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNoSuchVersion), r.URL, guessIsBrowserReq(r)) @@ -451,7 +459,11 @@ func (api objectAPIHandlers) HeadObjectHandler(w http.ResponseWriter, r *http.Re } vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } if vid := r.URL.Query().Get("versionId"); vid != "" && vid != "null" { writeErrorResponseHeadersOnly(w, errorCodes.ToAPIErr(ErrNoSuchVersion)) @@ -704,7 +716,11 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re } vars := mux.Vars(r) dstBucket := vars["bucket"] - dstObject := vars["object"] + dstObject, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } if s3Error := checkRequestAuthType(ctx, r, policy.PutObjectAction, dstBucket, dstObject); s3Error != ErrNone { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r)) @@ -774,7 +790,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re } var srcOpts, dstOpts ObjectOptions - srcOpts, err := copySrcOpts(ctx, r, srcBucket, srcObject) + srcOpts, err = copySrcOpts(ctx, r, srcBucket, srcObject) if err != nil { logger.LogIf(ctx, err) writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) @@ -1139,7 +1155,11 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req } vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } // To detect if the client has disconnected. r.Body = &detectDisconnect{r.Body, r.Context().Done()} @@ -1428,7 +1448,11 @@ func (api objectAPIHandlers) NewMultipartUploadHandler(w http.ResponseWriter, r } vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } if s3Error := checkRequestAuthType(ctx, r, policy.PutObjectAction, bucket, object); s3Error != ErrNone { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r)) @@ -1443,7 +1467,6 @@ func (api objectAPIHandlers) NewMultipartUploadHandler(w http.ResponseWriter, r // get gateway encryption options var opts ObjectOptions - var err error opts, err = putOpts(ctx, r, bucket, object, nil) if err != nil { @@ -1550,7 +1573,11 @@ func (api objectAPIHandlers) CopyObjectPartHandler(w http.ResponseWriter, r *htt vars := mux.Vars(r) dstBucket := vars["bucket"] - dstObject := vars["object"] + dstObject, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } if s3Error := checkRequestAuthType(ctx, r, policy.PutObjectAction, dstBucket, dstObject); s3Error != ErrNone { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r)) @@ -1866,7 +1893,11 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http } vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } // X-Amz-Copy-Source shouldn't be set for this call. if _, ok := r.Header[xhttp.AmzCopySource]; ok { @@ -2108,7 +2139,11 @@ func (api objectAPIHandlers) AbortMultipartUploadHandler(w http.ResponseWriter, vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } objectAPI := api.ObjectAPI() if objectAPI == nil { @@ -2143,7 +2178,11 @@ func (api objectAPIHandlers) ListObjectPartsHandler(w http.ResponseWriter, r *ht vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } objectAPI := api.ObjectAPI() if objectAPI == nil { @@ -2277,7 +2316,11 @@ func (api objectAPIHandlers) CompleteMultipartUploadHandler(w http.ResponseWrite vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } objectAPI := api.ObjectAPI() if objectAPI == nil { @@ -2474,7 +2517,11 @@ func (api objectAPIHandlers) DeleteObjectHandler(w http.ResponseWriter, r *http. vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } objectAPI := api.ObjectAPI() if objectAPI == nil { @@ -2532,7 +2579,11 @@ func (api objectAPIHandlers) PutObjectLegalHoldHandler(w http.ResponseWriter, r vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } if vid := r.URL.Query().Get("versionId"); vid != "" && vid != "null" { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNoSuchVersion), r.URL, guessIsBrowserReq(r)) @@ -2630,7 +2681,11 @@ func (api objectAPIHandlers) GetObjectLegalHoldHandler(w http.ResponseWriter, r vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } if vid := r.URL.Query().Get("versionId"); vid != "" && vid != "null" { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNoSuchVersion), r.URL, guessIsBrowserReq(r)) @@ -2687,7 +2742,11 @@ func (api objectAPIHandlers) PutObjectRetentionHandler(w http.ResponseWriter, r vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } if vid := r.URL.Query().Get("versionId"); vid != "" && vid != "null" { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNoSuchVersion), r.URL, guessIsBrowserReq(r)) @@ -2779,7 +2838,11 @@ func (api objectAPIHandlers) GetObjectRetentionHandler(w http.ResponseWriter, r vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } if vid := r.URL.Query().Get("versionId"); vid != "" && vid != "null" { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrNoSuchVersion), r.URL, guessIsBrowserReq(r)) @@ -2835,7 +2898,11 @@ func (api objectAPIHandlers) GetObjectTaggingHandler(w http.ResponseWriter, r *h vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } objAPI := api.ObjectAPI() if objAPI == nil { @@ -2866,7 +2933,11 @@ func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *h vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } objAPI := api.ObjectAPI() if objAPI == nil { @@ -2901,16 +2972,20 @@ func (api objectAPIHandlers) DeleteObjectTaggingHandler(w http.ResponseWriter, r ctx := newContext(r, w, "DeleteObjectTagging") defer logger.AuditLog(w, r, "DeleteObjectTagging", mustGetClaimsFromToken(r)) - vars := mux.Vars(r) - bucket := vars["bucket"] - object := vars["object"] - objAPI := api.ObjectAPI() if objAPI == nil { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL, guessIsBrowserReq(r)) return } + vars := mux.Vars(r) + bucket := vars["bucket"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return + } + // Allow deleteObjectTagging if policy action is set if s3Error := checkRequestAuthType(ctx, r, policy.DeleteObjectTaggingAction, bucket, object); s3Error != ErrNone { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL, guessIsBrowserReq(r)) @@ -2918,7 +2993,7 @@ func (api objectAPIHandlers) DeleteObjectTaggingHandler(w http.ResponseWriter, r } // Delete object tags - err := objAPI.DeleteObjectTag(ctx, bucket, object) + err = objAPI.DeleteObjectTag(ctx, bucket, object) if err != nil { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) return diff --git a/cmd/routers.go b/cmd/routers.go index 0f004a759..7e4237024 100644 --- a/cmd/routers.go +++ b/cmd/routers.go @@ -84,7 +84,7 @@ var globalHandlers = []HandlerFunc{ func configureServerHandler(endpointZones EndpointZones) (http.Handler, error) { // Initialize router. `SkipClean(true)` stops gorilla/mux from // normalizing URL path minio/minio#3256 - router := mux.NewRouter().SkipClean(true) + router := mux.NewRouter().SkipClean(true).UseEncodedPath() // Initialize distributed NS lock. if globalIsDistXL { diff --git a/cmd/utils.go b/cmd/utils.go index 35409c900..f598466f8 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -29,6 +29,7 @@ import ( "io/ioutil" "net" "net/http" + "net/url" "os" "path/filepath" "reflect" @@ -506,9 +507,14 @@ func ceilFrac(numerator, denominator int64) (ceil int64) { func newContext(r *http.Request, w http.ResponseWriter, api string) context.Context { vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] - prefix := vars["prefix"] - + object, err := url.PathUnescape(vars["object"]) + if err != nil { + object = vars["object"] + } + prefix, err := url.QueryUnescape(vars["prefix"]) + if err != nil { + prefix = vars["prefix"] + } if prefix != "" { object = prefix } diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index bb3a739be..73a9fd832 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -918,7 +918,11 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) { } vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeWebErrorResponse(w, err) + return + } retPerms := ErrAccessDenied holdPerms := ErrAccessDenied @@ -1135,7 +1139,11 @@ func (web *webAPIHandlers) Download(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) bucket := vars["bucket"] - object := vars["object"] + object, err := url.PathUnescape(vars["object"]) + if err != nil { + writeWebErrorResponse(w, err) + return + } token := r.URL.Query().Get("token") getRetPerms := ErrAccessDenied