Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Response divergence from AWS S3 #709

Closed
tonyipm opened this issue Aug 1, 2024 · 2 comments · Fixed by #714
Closed

Response divergence from AWS S3 #709

tonyipm opened this issue Aug 1, 2024 · 2 comments · Fixed by #714
Assignees
Labels
bug Something isn't working

Comments

@tonyipm
Copy link

tonyipm commented Aug 1, 2024

Describe the bug
Versity Gateway shows divergent behaviour from Amazon S3 in the following scenario. We have created a bucket called 'titest' and uploaded a file called 'test21'. We then used the s3api interface to retrieve the file as follows:

Using aws:

aws s3api get-object --bucket titest --key test1 test1 [operation succeeds]

aws s3api get-object --bucket titest --key test1/ x1

An error occurred (NoSuchKey) when calling the GetObject operation: The specified key does not exist.

In the second instance we are specifying a trailing / in the key and S3 responds with 'key does not exist'.

If we try the second scenario with Versity, the second operation (with trailing /) succeeds:

vgaws s3api get-object --bucket titest --key test1/ x1
{
"AcceptRanges": "",
"LastModified": "Thu, 01 Aug 2024 10:55:26 GMT",
"ContentLength": 1024,
"ETag": "187448fd5bc7e992c124a773eccee369",
"ContentRange": "",
"ContentType": "text/plain; charset=utf-8",
"Metadata": {},
"StorageClass": ""
}

We get an identical response whether or not the key has a trailing /.

Using the delve debugger, we can see how this comes about via the parsing of the gofiber front-end package.

Base.go line 78 onwards:

78:
79: func (c S3ApiController) GetActions(ctx *fiber.Ctx) error {
80: bucket := ctx.Params("bucket")
81: key := ctx.Params("key")
82: keyEnd := ctx.Params("*1")
=> 83: uploadId := ctx.Query("uploadId")
84: maxParts := int32(ctx.QueryInt("max-parts", -1))
85: partNumberMarker := ctx.Query("part-number-marker")
86: acceptRange := ctx.Get("Range")
87: acct := ctx.Locals("account").(auth.Account)
88: isRoot := ctx.Locals("isRoot").(bool)
(dlv) p key
"test1"
(dlv) p keyEnd
""

(dlv) p ctx

[some lines removed for clarity]

method: "GET",
methodINT: 0,
baseURI: "",
path: "/titest/test1/",
pathBuffer: []uint8 len: 14, cap: 16, [47,116,105,116,101,115,116,47,116,101,115,116,49,47],
detectionPath: "/titest/test1/",
detectionPathBuffer: []uint8 len: 14, cap: 16, [47,116,105,116,101,115,116,47,116,101,115,116,49,47],
treePath: "/ti",
pathOriginal: "/titest/test1/",
values: [30]string ["titest","test1","","","","","

We can see that in the gofiber context structure, the incoming path shows the trailing slash, but this is removed when the URI is parsed into key values. Versity is never aware that the key was originally supplied with a trailing slash.

To Reproduce
Use
aws s3api get-object --bucket --key
then
aws s3api get-object --bucket --key

AWS S3 fails the second command with 'key not found'. Versity treats the two cases the same and both succeed.

Expected behavior
The second case described above should return the same error as aws s3, "specified key does not exist"

Server Version

./versitygw -version && uname -a
Version :
Build : release-rpm
BuildTime: 2024-07-26_09:16:00AM
Linux ip-10-100-0-246 4.18.0-513.18.1.el8_9.x86_64 #1 SMP Thu Feb 1 03:51:05 EST 2024 x86_64 x86_64 x86_64 GNU/Linux

Additional context

aws --version
aws-cli/1.24.10 Python/3.6.8 Linux/4.18.0-513.18.1.el8_9.x86_64 botocore/1.26.10

@tonyipm tonyipm added the bug Something isn't working label Aug 1, 2024
@benmcclelland
Copy link
Member

Thank you for the detailed bug report. I think we had a similar issue when trying to put keys with trailing / to create the resulting directory. I'll look to see if we can do a similar fix for this case.

@benmcclelland benmcclelland self-assigned this Aug 5, 2024
@benmcclelland benmcclelland moved this to In Progress in VersityGW Project Aug 5, 2024
@benmcclelland
Copy link
Member

this is affecting head-object and delete-object as well

benmcclelland added a commit that referenced this issue Aug 5, 2024
…file object exists

The API hanlders and backend were stripping trailing "/" in object
paths. So if an object exists and a request came in for head/get/delete
for that same name but with a trailing "/" indicating request should
be for directory object, the "/" would be stripped and the request
would be handlied for the incorrect file object.

This fix adds in checks to handle the case with the training "/"
in the request.

Fixes #709
benmcclelland added a commit that referenced this issue Aug 5, 2024
…ding file object exists

The API hanlders and backend were stripping trailing "/" in object
paths. So if an object exists and a request came in for head/get/delete/copy
for that same name but with a trailing "/" indicating request should
be for directory object, the "/" would be stripped and the request
would be handlied for the incorrect file object.

This fix adds in checks to handle the case with the training "/"
in the request.

Fixes #709
@github-project-automation github-project-automation bot moved this from In Progress to Done in VersityGW Project Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants