From c0152e6be1042d80400590cb13f4801fc30da2ef Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 21 Oct 2026 20:02:04 -0700 Subject: [PATCH] s3: keep '\' a literal object-key character behind -s3.allowBackslashInKey (#11911) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * s3: keep '\' a literal object-key character when AllowBackslashInKey is set NormalizeObjectKey rewrites every S3 object key: '\' becomes '/' and duplicate slashes collapse. On AWS S3 both are ordinary key characters, so keys that are distinct there alias one object here — writing probe/back/slash.txt silently overwrites probe/back\slash.txt. The filer only treats '/' as a path separator, so a verbatim '\' stays inside one entry name. Gate the backslash rewrite and the matching '\' handling in IsValidObjectKey behind a process-wide mode the S3 server sets from the new S3ApiServerOption. Duplicate-slash collapsing stays: an entry name cannot contain an empty segment. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * command: add -s3.allowBackslashInKey flag for AWS-compatible key handling Registers the flag on all four S3 entry points (s3, server -s3, filer -s3, mini) and passes it through S3Options into S3ApiServerOption. Default off, so existing normalization is unchanged. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: correct IsValidObjectKey/NormalizeObjectKey docs under allowBackslashInKey Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: normalize DeleteObjects body keys the same as path keys DeleteObjects keys arrive in the XML body, so they bypassed the normalization applied to route-var keys. The lock, the authorization checks, the table-bucket layout check and the filer delete itself all addressed the raw key while the conditional check resolved the normalized one, so a key spelled with '\' or '//' could be reported deleted while the stored entry survived. Echo the client-sent key in the result while operating on the normalized form. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: report a normalized-empty DeleteObjects key as a delete no-op A multi-delete key of only slashes normalizes to the empty string. Left to reach the filer delete, the empty name resolves to the bucket entry itself, so an unauthorized caller could remove a bucket through the body-key path. Such a key can never name a stored entry, so like a missing key the delete is an idempotent no-op reported before any permission check or filer call. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: drop unused AllowBackslashInObjectKey getter * s3: answer conditional delete of a normalized-empty key with NoSuchKey * s3: normalize the probed key in authorizeAnonymousBucketList The masked-miss probe read the raw mux var, so a spelling like folder// or folder\ was authorized against a key that can never exist while the request itself targets the normalized folder/. Probe the normalized key so the answer matches what the read path authorizes. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: cover anonymous authz of a literal-backslash key under allowBackslashInKey A public grant on the slash key must not authorize an anonymous read of the distinct backslash object when the flag keeps '\' literal; grant the backslash key's own ACL and it reads back. Default mode still folds the spelling onto the covered slash key. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: publish the key mode only after the S3 server is constructed A failed NewS3ApiServerWithStore could previously flip the process-wide key mode out from under an already-running gateway. * s3: normalize the copy-source key before validation and authorization pathToBucketObjectAndVersion returned the raw header spelling, so an equivalent spelling (a//b, a\b) was authorized and compared against the normalized destination while resolveCopySourceEntry looked up the normalized key — a deny on the canonical key could be bypassed and self-copy detection missed. Normalize once at parse so the CopyObject and UploadPartCopy paths agree. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/command/filer.go | 6 +++ weed/command/mini.go | 1 + weed/command/s3.go | 3 ++ weed/command/server.go | 1 + weed/s3api/s3_constants/header.go | 30 +++++++++--- weed/s3api/s3_constants/header_test.go | 48 +++++++++++++++++++ .../s3api_object_anonymous_cache_test.go | 43 ++++++++++++++++- weed/s3api/s3api_object_anonymous_read.go | 2 +- weed/s3api/s3api_object_handlers_copy.go | 4 +- weed/s3api/s3api_object_handlers_delete.go | 43 +++++++++++------ weed/s3api/s3api_server.go | 7 +++ 11 files changed, 162 insertions(+), 26 deletions(-) diff --git a/weed/command/filer.go b/weed/command/filer.go index 90eb04bee9..d1385f918a 100644 --- a/weed/command/filer.go +++ b/weed/command/filer.go @@ -51,6 +51,11 @@ var ( // S3 gateway, whose remote-mount read paths dial the mounted endpoint directly. const allowUntrustedRemoteEndpointsUsage = "if true, a read of a remote-only entry accepts arbitrary remote S3 endpoints including loopback / link-local hosts. Default rejects internal / metadata endpoints." +// allowBackslashInKeyUsage documents the S3 flag keeping '\' a literal +// object-key character instead of normalizing it to '/'. Repeated '//' in a +// key still collapses: a filer entry name cannot contain an empty segment. +const allowBackslashInKeyUsage = "treat backslash as a regular object-key character instead of converting it to '/' (aws-compatible; '//' still collapses)" + type FilerOptions struct { masters *pb.ServerDiscovery mastersString *string @@ -173,6 +178,7 @@ func init() { filerS3Options.cacheSizeMB = cmdFiler.Flag.Int64("s3.cacheCapacityMB", 0, "in-memory chunk cache capacity in MB for S3 GETs shared across requests (0 disables)") filerS3Options.readerCacheSizeMB = cmdFiler.Flag.Int64("s3.readerCacheSizeMB", 0, "memory budget in MiB for downloaded and in-flight reader buffers across all S3 GETs (0 means unlimited)") filerS3Options.allowUntrustedRemoteEndpoints = cmdFiler.Flag.Bool("s3.allowUntrustedRemoteEndpoints", false, allowUntrustedRemoteEndpointsUsage) + filerS3Options.allowBackslashInKey = cmdFiler.Flag.Bool("s3.allowBackslashInKey", false, allowBackslashInKeyUsage) // start webdav on filer filerStartWebDav = cmdFiler.Flag.Bool("webdav", false, "whether to start webdav gateway") diff --git a/weed/command/mini.go b/weed/command/mini.go index 1ded2f936c..9d4eb630cb 100644 --- a/weed/command/mini.go +++ b/weed/command/mini.go @@ -556,6 +556,7 @@ func initMiniS3Flags() { miniS3Options.cacheSizeMB = cmdMini.Flag.Int64("s3.cacheCapacityMB", 0, "in-memory chunk cache capacity in MB for S3 GETs shared across requests (0 disables)") miniS3Options.readerCacheSizeMB = cmdMini.Flag.Int64("s3.readerCacheSizeMB", 0, "memory budget in MiB for downloaded and in-flight reader buffers across all S3 GETs (0 means unlimited)") miniS3Options.allowUntrustedRemoteEndpoints = cmdMini.Flag.Bool("s3.allowUntrustedRemoteEndpoints", false, allowUntrustedRemoteEndpointsUsage) + miniS3Options.allowBackslashInKey = cmdMini.Flag.Bool("s3.allowBackslashInKey", false, allowBackslashInKeyUsage) // In mini mode, S3 uses the shared debug server started at line 681, not its own separate debug server miniS3Options.debug = new(bool) // explicitly false miniS3Options.debugPort = cmdMini.Flag.Int("s3.debug.port", 6060, "http port for debugging (unused in mini mode)") diff --git a/weed/command/s3.go b/weed/command/s3.go index 036e3813eb..98b0bbcc3c 100644 --- a/weed/command/s3.go +++ b/weed/command/s3.go @@ -84,6 +84,7 @@ type S3Options struct { readerCacheSizeMB *int64 allowUntrustedRemoteEndpoints *bool + allowBackslashInKey *bool // shutdownCtx, when non-nil, tells startS3Server to gracefully shut down // its HTTP/gRPC servers once the ctx is cancelled, in addition to on // interrupt. Used by weed mini to orchestrate an ordered shutdown. @@ -140,6 +141,7 @@ func init() { s3StandaloneOptions.cacheSizeMB = cmdS3.Flag.Int64("cacheCapacityMB", 0, "in-memory chunk cache capacity in MB for S3 GETs shared across requests (0 disables)") s3StandaloneOptions.readerCacheSizeMB = cmdS3.Flag.Int64("readerCacheSizeMB", 0, "memory budget in MiB for downloaded and in-flight reader buffers across all S3 GETs (0 means unlimited)") s3StandaloneOptions.allowUntrustedRemoteEndpoints = cmdS3.Flag.Bool("allowUntrustedRemoteEndpoints", false, allowUntrustedRemoteEndpointsUsage) + s3StandaloneOptions.allowBackslashInKey = cmdS3.Flag.Bool("allowBackslashInKey", false, allowBackslashInKeyUsage) } var cmdS3 = &Command{ @@ -399,6 +401,7 @@ func (s3opt *S3Options) startS3Server() bool { MaxMB: filerMaxMB, AllowUntrustedRemoteEndpoints: *s3opt.allowUntrustedRemoteEndpoints, + AllowBackslashInKey: *s3opt.allowBackslashInKey, }) if s3ApiServer_err != nil { glog.Fatalf("S3 API Server startup error: %v", s3ApiServer_err) diff --git a/weed/command/server.go b/weed/command/server.go index 0e4ac169a3..3fd367fd7f 100644 --- a/weed/command/server.go +++ b/weed/command/server.go @@ -195,6 +195,7 @@ func init() { s3Options.iamReadOnly = cmdServer.Flag.Bool("s3.iam.readOnly", true, "disable IAM write operations on this server") s3Options.cipher = cmdServer.Flag.Bool("s3.encryptVolumeData", false, "encrypt data on volume servers for S3 uploads") s3Options.externalUrl = cmdServer.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Advertised to Iceberg and Lance clients, and tried first when verifying S3 signatures behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + s3Options.allowBackslashInKey = cmdServer.Flag.Bool("s3.allowBackslashInKey", false, allowBackslashInKeyUsage) s3Options.defaultFileMode = cmdServer.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") s3Options.cacheSizeMB = cmdServer.Flag.Int64("s3.cacheCapacityMB", 0, "in-memory chunk cache capacity in MB for S3 GETs shared across requests (0 disables)") s3Options.readerCacheSizeMB = cmdServer.Flag.Int64("s3.readerCacheSizeMB", 0, "memory budget in MiB for downloaded and in-flight reader buffers across all S3 GETs (0 means unlimited)") diff --git a/weed/s3api/s3_constants/header.go b/weed/s3api/s3_constants/header.go index 28a2d1baf0..6fcedfdedc 100644 --- a/weed/s3api/s3_constants/header.go +++ b/weed/s3api/s3_constants/header.go @@ -216,10 +216,21 @@ func GetBucketAndObject(r *http.Request) (bucket, object string) { return } -// IsValidObjectKey rejects S3 object keys that — after normalization -// (backslash→slash, slash collapse) — contain a `.` or `..` path segment, or -// embed a NUL byte. Such keys are collapsed by filepath.Join inside the filer -// and would escape the bucket directory, so they must not reach the gRPC layer. +// allowBackslashInObjectKey keeps '\' a literal object-key character instead +// of normalizing it to '/'. It is a process-wide mode set once at S3 server +// startup; the filer only treats '/' as a path separator, so a verbatim '\' +// stays inside one filer entry name. +var allowBackslashInObjectKey atomic.Bool + +func SetAllowBackslashInObjectKey(allow bool) { + allowBackslashInObjectKey.Store(allow) +} + +// IsValidObjectKey rejects S3 object keys that — after the mode-dependent +// normalization NormalizeObjectKey applies — contain a `.` or `..` path +// segment, or embed a NUL byte. Such keys are collapsed by filepath.Join +// inside the filer and would escape the bucket directory, so they must not +// reach the gRPC layer. // Gorilla mux URL-decodes captured vars before this runs, so `%2e%2e` is // already `..` here. func IsValidObjectKey(object string) bool { @@ -229,7 +240,9 @@ func IsValidObjectKey(object string) bool { if strings.ContainsRune(object, '\x00') { return false } - object = strings.ReplaceAll(object, "\\", "/") + if !allowBackslashInObjectKey.Load() { + object = strings.ReplaceAll(object, "\\", "/") + } for _, seg := range strings.Split(object, "/") { if seg == "." || seg == ".." { return false @@ -263,7 +276,8 @@ func IsValidPathSegment(value string) bool { // NormalizeObjectKey normalizes object keys by removing duplicate slashes and converting backslashes. // This normalizes keys from various sources (URL path, form values, etc.) to a consistent format. -// It also converts Windows-style backslashes to forward slashes for cross-platform compatibility. +// It also converts Windows-style backslashes to forward slashes for cross-platform compatibility, +// unless allowBackslashInObjectKey is set. // Returns keys WITHOUT leading slash to match S3 API format (e.g., "foo/bar" not "/foo/bar"). // Preserves trailing slash if present (e.g., "foo/" stays "foo/"). func NormalizeObjectKey(object string) string { @@ -271,7 +285,9 @@ func NormalizeObjectKey(object string) string { hasTrailingSlash := strings.HasSuffix(object, "/") // Convert Windows-style backslashes to forward slashes - object = strings.ReplaceAll(object, "\\", "/") + if !allowBackslashInObjectKey.Load() { + object = strings.ReplaceAll(object, "\\", "/") + } object = removeDuplicateSlashes(object) // Remove leading slash to match S3 API format object = strings.TrimPrefix(object, "/") diff --git a/weed/s3api/s3_constants/header_test.go b/weed/s3api/s3_constants/header_test.go index 7e45882710..de780d0d51 100644 --- a/weed/s3api/s3_constants/header_test.go +++ b/weed/s3api/s3_constants/header_test.go @@ -123,6 +123,54 @@ func TestIsValidObjectKey(t *testing.T) { } } +func TestNormalizeObjectKeyAllowBackslash(t *testing.T) { + SetAllowBackslashInObjectKey(true) + defer SetAllowBackslashInObjectKey(false) + + normalizeTests := []struct { + name string + input string + expected string + }{ + {"backslash kept verbatim", "folder\\file.txt", "folder\\file.txt"}, + {"nested backslash kept", "folder\\sub\\file.txt", "folder\\sub\\file.txt"}, + {"trailing backslash kept", "folder\\", "folder\\"}, + {"lone backslash kept", "\\", "\\"}, + {"double backslash kept", "a\\\\b", "a\\\\b"}, + {"mixed separators", "folder\\sub/another\\file.txt", "folder\\sub/another\\file.txt"}, + {"duplicate slashes still collapse", "folder//sub///file.txt", "folder/sub/file.txt"}, + {"leading slash still trimmed", "/folder\\file.txt", "folder\\file.txt"}, + {"trailing slash preserved", "folder/", "folder/"}, + {"windows path style", "C:\\Users\\test\\file.txt", "C:\\Users\\test\\file.txt"}, + } + for _, tt := range normalizeTests { + t.Run(tt.name, func(t *testing.T) { + if got := NormalizeObjectKey(tt.input); got != tt.expected { + t.Errorf("NormalizeObjectKey(%q) = %q, want %q", tt.input, got, tt.expected) + } + }) + } + + validTests := []struct { + name string + input string + want bool + }{ + {"backslash dotdot is one segment", "..\\evil\\test.txt", true}, + {"backslash dotdot inside", "good\\..\\evil/test.txt", true}, + {"nested dotdot still rejected", "good/../evil/test.txt", false}, + {"leading dotdot still rejected", "../evil/test.txt", false}, + {"plain backslash key", "folder\\file.txt", true}, + } + for _, tt := range validTests { + t.Run(tt.name, func(t *testing.T) { + if got := IsValidObjectKey(tt.input); got != tt.want { + t.Errorf("IsValidObjectKey(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} + func TestIsValidBucketName(t *testing.T) { tests := []struct { name string diff --git a/weed/s3api/s3api_object_anonymous_cache_test.go b/weed/s3api/s3api_object_anonymous_cache_test.go index d5a0f2f593..a349585727 100644 --- a/weed/s3api/s3api_object_anonymous_cache_test.go +++ b/weed/s3api/s3api_object_anonymous_cache_test.go @@ -238,10 +238,12 @@ func TestAnonymousObjectACLVersionedDirectoryIdentity(t *testing.T) { f.entries["/buckets/b/folder/.versions"] = &filer_pb.Entry{IsDirectory: true, Extended: map[string][]byte{s3_constants.ExtLatestVersionIdKey: []byte("")}} } // Equivalent filer-path spellings must not recover a different object's data. - assertMissing(serveAnonymousRead(s3a, method, "folder/", route.query, headers)) + for _, object := range []string{"folder/", "folder//", "folder\\"} { + assertMissing(serveAnonymousRead(s3a, method, object, route.query, headers)) + } // Spellings the policy's folder/ grant does not cover are // masked: a miss must be indistinguishable from a denied read. - for _, object := range []string{"folder//", "folder\\"} { + for _, object := range []string{"folder//x", "folder\\x"} { rr := serveAnonymousRead(s3a, method, object, route.query, headers) require.Equal(t, http.StatusForbidden, rr.Code, rr.Body.String()) } @@ -281,3 +283,40 @@ func TestAnonymousObjectACLVersionedDirectoryIdentity(t *testing.T) { } } } + +// A literal-backslash key is a different object than its slash spelling only +// when allowBackslashInObjectKey is set. A public grant on b/folder/ must not +// authorize an anonymous read of the separate folder\ object under that mode. +func TestAnonymousObjectACLBackslashKeyIdentity(t *testing.T) { + for _, allowBackslash := range []bool{false, true} { + t.Run(fmt.Sprintf("allowBackslashInKey=%v", allowBackslash), func(t *testing.T) { + s3a, f := newAnonymousReadTestServer(t) + s3_constants.SetAllowBackslashInObjectKey(allowBackslash) + t.Cleanup(func() { s3_constants.SetAllowBackslashInObjectKey(false) }) + require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy("b", `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"*","Action":"s3:GetObject*","Resource":"arn:aws:s3:::b/folder/"}]}`)) + slash := anonymousReadEntry([]byte(`[]`)) + slash.Name = "folder" + slash.MarkPrefixObject() + f.entries["/buckets/b/folder"] = slash + backslash := anonymousReadEntry([]byte(`[]`)) + backslash.Name = `folder\` + f.entries[`/buckets/b/folder\`] = backslash + + rr := serveAnonymousRead(s3a, http.MethodGet, "folder/", "", nil) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, "hello world", rr.Body.String()) + + rr = serveAnonymousRead(s3a, http.MethodGet, `folder\`, "", nil) + if !allowBackslash { + // Default mode folds the spelling onto the slash key the policy covers. + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + } else { + require.Equal(t, http.StatusForbidden, rr.Code, rr.Body.String()) + backslash.Extended[s3_constants.ExtAmzAclKey] = anonymousReadGrant("READ") + rr = serveAnonymousRead(s3a, http.MethodGet, `folder\`, "", nil) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, "hello world", rr.Body.String()) + } + }) + } +} diff --git a/weed/s3api/s3api_object_anonymous_read.go b/weed/s3api/s3api_object_anonymous_read.go index e5d1c21c28..6fd86001aa 100644 --- a/weed/s3api/s3api_object_anonymous_read.go +++ b/weed/s3api/s3api_object_anonymous_read.go @@ -118,7 +118,7 @@ func (s3a *S3ApiServer) authorizeAnonymousObjectRead(r *http.Request, bucket, ob // public-read bucket. Anything else masks the miss as AccessDenied. func (s3a *S3ApiServer) authorizeAnonymousBucketList(r *http.Request, bucket string) s3err.ErrorCode { identity, _ := s3_constants.GetIdentityFromContext(r).(*Identity) - object := mux.Vars(r)["object"] + object := s3_constants.NormalizeObjectKey(mux.Vars(r)["object"]) // The caller's query parameters would otherwise retarget these checks // (?policy resolves to GetBucketPolicy) or satisfy a prefix condition on an // unrelated subtree, so each runs on a request with a fixed shape: a diff --git a/weed/s3api/s3api_object_handlers_copy.go b/weed/s3api/s3api_object_handlers_copy.go index fbbf2b2fd5..2473ed77cc 100644 --- a/weed/s3api/s3api_object_handlers_copy.go +++ b/weed/s3api/s3api_object_handlers_copy.go @@ -900,13 +900,13 @@ func pathToBucketObjectAndVersion(rawPath, decodedPath string) (bucket, object, } bucket, object = pathToBucketAndObject(pathForBucket) - return bucket, object, versionId + return bucket, s3_constants.NormalizeObjectKey(object), versionId } } } bucket, object = pathToBucketAndObject(pathForBucket) - return bucket, object, versionId + return bucket, s3_constants.NormalizeObjectKey(object), versionId } type CopyPartResult struct { diff --git a/weed/s3api/s3api_object_handlers_delete.go b/weed/s3api/s3api_object_handlers_delete.go index 3817a28adc..caa0069624 100644 --- a/weed/s3api/s3api_object_handlers_delete.go +++ b/weed/s3api/s3api_object_handlers_delete.go @@ -479,29 +479,44 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h deleteErrors = append(deleteErrors, deleteErrorFromCode(validationCode, object.Key, object.VersionId)) continue } - if _, err := s3a.validateTableBucketObjectPathOrVerify(bucket, object.Key); err != nil { + key := s3_constants.NormalizeObjectKey(object.Key) + if key == "" { + // A key of only slashes can never name a stored entry; like a + // missing key the delete is an idempotent no-op, and a + // conditional delete has no target to match. + if object.ETag != "" { + deleteErrors = append(deleteErrors, deleteErrorFromCode(s3err.ErrNoSuchKey, object.Key, object.VersionId)) + continue + } + deletedCount++ + if !deleteObjects.Quiet { + deletedObjects = append(deletedObjects, ObjectIdentifier{Key: object.Key, VersionId: object.VersionId}) + } + continue + } + if _, err := s3a.validateTableBucketObjectPathOrVerify(bucket, key); err != nil { deleteErrors = append(deleteErrors, deleteErrorFromCode(s3err.ErrAccessDenied, object.Key, object.VersionId)) continue } - if authErr := s3a.iam.AuthorizeObjectDelete(r, identity, bucket, object.Key, object.VersionId); authErr != s3err.ErrNone { + if authErr := s3a.iam.AuthorizeObjectDelete(r, identity, bucket, key, object.VersionId); authErr != s3err.ErrNone { // Defer to the recheck, mirroring the copy-source fallback: a // tag-conditioned Allow may still grant what phase one denied. - if authErr != s3err.ErrAccessDenied || s3a.recheckPolicyForKeyWithLookup(r, http.MethodDelete, bucket, object.Key, object.VersionId, string(s3_constants.ACTION_WRITE), "DeleteMultipleObjectsHandler") != s3err.ErrNone { + if authErr != s3err.ErrAccessDenied || s3a.recheckPolicyForKeyWithLookup(r, http.MethodDelete, bucket, key, object.VersionId, string(s3_constants.ACTION_WRITE), "DeleteMultipleObjectsHandler") != s3err.ErrNone { deleteErrors = append(deleteErrors, deleteErrorFromCode(authErr, object.Key, object.VersionId)) continue } } - if errCode := s3a.recheckTagConditionedPolicyWithLookup(objectKeyPolicyRequest(r, http.MethodDelete, bucket, object.Key, object.VersionId), bucket, object.Key, object.VersionId, string(s3_constants.ACTION_WRITE), "DeleteMultipleObjectsHandler"); errCode != s3err.ErrNone { + if errCode := s3a.recheckTagConditionedPolicyWithLookup(objectKeyPolicyRequest(r, http.MethodDelete, bucket, key, object.VersionId), bucket, key, object.VersionId, string(s3_constants.ACTION_WRITE), "DeleteMultipleObjectsHandler"); errCode != s3err.ErrNone { deleteErrors = append(deleteErrors, deleteErrorFromCode(errCode, object.Key, object.VersionId)) continue } var deleteResult deleteMutationResult - deleteCode := s3a.withObjectWriteLock(bucket, object.Key, func() s3err.ErrorCode { - return s3a.checkDeleteIfMatch(bucket, object.Key, object.VersionId, versioningState, object.ETag, s3err.ErrNoSuchKey) + deleteCode := s3a.withObjectWriteLock(bucket, key, func() s3err.ErrorCode { + return s3a.checkDeleteIfMatch(bucket, key, object.VersionId, versioningState, object.ETag, s3err.ErrNoSuchKey) }, func() s3err.ErrorCode { if versioningConfigured { - result, errCode := s3a.deleteVersionedObject(r, bucket, object.Key, object.VersionId, versioningState) + result, errCode := s3a.deleteVersionedObject(r, bucket, key, object.VersionId, versioningState) if errCode != s3err.ErrNone { return errCode } @@ -509,18 +524,18 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h return s3err.ErrNone } - governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object.Key) - if err := s3a.enforceObjectLockProtections(r, bucket, object.Key, "", governanceBypassAllowed); err != nil { - glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s: %v", bucket, object.Key, err) + governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, key) + if err := s3a.enforceObjectLockProtections(r, bucket, key, "", governanceBypassAllowed); err != nil { + glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s: %v", bucket, key, err) return s3err.ErrAccessDenied } - if strings.HasSuffix(object.Key, "/") { - return s3a.deleteDirectoryMarker(r, bucket, object.Key) + if strings.HasSuffix(key, "/") { + return s3a.deleteDirectoryMarker(r, bucket, key) } - if err := s3a.deleteUnversionedObjectWithClient(r.Context(), client, bucket, object.Key, false); err != nil { - glog.Errorf("DeleteMultipleObjectsHandler: failed to delete %s/%s: %v", bucket, object.Key, err) + if err := s3a.deleteUnversionedObjectWithClient(r.Context(), client, bucket, key, false); err != nil { + glog.Errorf("DeleteMultipleObjectsHandler: failed to delete %s/%s: %v", bucket, key, err) return s3err.ErrInternalError } diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index ad88378916..2ccd42d6e4 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -78,6 +78,9 @@ type S3ApiServerOption struct { // AllowUntrustedRemoteEndpoints lets a read of a remote-only object dial a // mounted endpoint that resolves to a loopback / private / metadata host. AllowUntrustedRemoteEndpoints bool + // AllowBackslashInKey keeps '\' a literal object-key character instead of + // normalizing it to '/', matching AWS S3 where the two are distinct keys. + AllowBackslashInKey bool } // s3ChunkCacheChunkSizeMB is the assumed chunk size (in MiB) used to convert @@ -532,6 +535,10 @@ func NewS3ApiServerWithStore(router *mux.Router, option *S3ApiServerOption, expl // pointer-to-missing-file states without waiting for a client GET. s3ApiServer.versionsReconcilerStop = s3ApiServer.startVersioningReconciler() + // Publish the key mode only once the server is fully constructed so a + // failed construction cannot flip the mode of a running server. + SetAllowBackslashInObjectKey(option.AllowBackslashInKey) + return s3ApiServer, nil }