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

rgw: fix the subdir without slash of s3 website url #15703

Merged
merged 1 commit into from Jul 7, 2017

Conversation

Projects
None yet
5 participants
@DHB-liuhong

DHB-liuhong commented Jun 15, 2017

if we use the subdir without slash, we cannot visit the default index.html,
which we configured.It is not consistent with AWS S3.

Fixes: http://tracker.ceph.com/issues/20307
Signed-off-by: liuhong liuhong@cmss.chinamobile.com

@DHB-liuhong

This comment has been minimized.

DHB-liuhong commented Jun 15, 2017

@cbodley could you help me take a look? thanks:-)

@DHB-liuhong

This comment has been minimized.

DHB-liuhong commented Jun 15, 2017

jenkins retest this please

@cbodley cbodley requested a review from robbat2 Jun 16, 2017

@robbat2

This comment has been minimized.

Contributor

robbat2 commented Jun 16, 2017

I have a concern with this, and I'm not sure what the correct behavior should be:
AWS-S3 allows you to create an object with the same name as a directory, as well as an object with a trailing slash in the name! All because it's actually a flat namespace with / just available as a separator for convenience.

# Three objects:
/foo.txt
/foo.txt/
/foo.txt/bar.txt

Can you please check the AWS-S3 behavior, and make sure the new code has the same behavior?

A close reading of http://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html doesn't state if / is a safe character or not, and it's possible that it could be used as part of a filename, instead of a seperator.

s3cmd can't handle the trailing slash in the keyname, but is fine uploading a file with file that matches the start of a name plus /.

@DHB-liuhong

This comment has been minimized.

DHB-liuhong commented Jun 20, 2017

Thanks.
I haved test this for AWS s3 and the new code using ceph-request tools:

  1. foo.txt/
  2. foo1.txt/bar.html

Test:

  1. If the object name ends in slashes , AWS S3 will deal with this object as a directory。
    Visit the url: http://yly-test.s3-website-us-east-1.amazonaws.com/foo.txt/
    Return: 404 Not Found Code: NoSuchKey
    I can get this obejct
[root@yly-ceph3 ceph-request]# ceph-request get '/yly-test/foo.txt/'  --verbose -c ceph-request.cfg 
< GET /yly-test/foo.txt/ HTTP/1.1
< Host: s3.amazonaws.com:80
< Accept-Encoding: gzip, deflate
< Accept: */*
< User-Agent: python-requests/2.7.0 CPython/2.7.5 Linux/3.10.0-123.20.1.el7.x86_64
< Connection: keep-alive
< date: Tue, 20 Jun 2017 03:13:12 GMT
< Authorization: AWS AKIAIOHGX57FOXYYYOBQ:XYNQ0eROlPMhiVRidfVQwdlg/nI=
<

> HTTP/1.1 200 OK
> content-length: 28
> x-amz-id-2: 8UUSduoDoDNClmq2F9Jx8yzRXNuNZpE1/hij926vH6XtLNc770p09emofWi9Hw2xVw9/W7S+/Os=
> accept-ranges: bytes
> server: AmazonS3
> last-modified: Tue, 20 Jun 2017 03:13:50 GMT
> x-amz-request-id: CC89CF9F36817666
> etag: "66dba04c2629c5e5178bde7ad15580ce"
> date: Tue, 20 Jun 2017 03:15:09 GMT
> content-type: binary/octet-stream
>
this is a test !!!! yes !!!!

The new code has the same behavior

  1. If the object name as this : foo1.txt/bar.html ,AWS S3 will deal with this object as this : foo1.txt is a directory, bar.html is an object in foo1.txt。
    Visit the url: http://yly-test.s3-website-us-east-1.amazonaws.com/foo1.txt/bar.html
    you will download this object. As the same , I can get this object.

The new code has the same behavior

@DHB-liuhong

This comment has been minimized.

DHB-liuhong commented Jun 22, 2017

@robbat2 could you help me take a look? thanks:-)

@DHB-liuhong

This comment has been minimized.

DHB-liuhong commented Jun 27, 2017

@robbat2 could you help me take a look? thanks:-)

@robbat2

Ok, since it matches AWS S3 behavior, let's merge it.

Somewhere we should probably document that objects ending in a slash will be inaccessible via the website endpoint (really belongs in the AWS S3 documentation, unless they decide it's a bug and fix it in AWS S3).

@DHB-liuhong

This comment has been minimized.

DHB-liuhong commented Jun 28, 2017

@robbat2 thanks

@DHB-liuhong

This comment has been minimized.

DHB-liuhong commented Jun 28, 2017

@cbodley can you merge this please? thanks.

@cbodley

just some minor formatting issues

subdir_name.pop_back();
}
rgw_obj obj(s->bucket,subdir_name);

This comment has been minimized.

@cbodley

cbodley Jun 28, 2017

Contributor

add space after ,

obj_ctx.obj.set_atomic(obj);
obj_ctx.obj.set_prefetch_data(obj);
RGWObjState* state = nullptr;

This comment has been minimized.

@cbodley

cbodley Jun 28, 2017

Contributor

remove extra space before state

if (! state->exists) {
return false;
}
return true;

This comment has been minimized.

@cbodley

cbodley Jun 28, 2017

Contributor

can just return state->exists; here

obj_ctx.obj.set_prefetch_data(obj);
RGWObjState* state = nullptr;
if (store->get_obj_state(&obj_ctx, s->bucket_info, obj, &state, false) < 0 ) {

This comment has been minimized.

@cbodley

cbodley Jun 28, 2017

Contributor

remove extra space before ) {

rgw: fix the subdir without slash of s3 website url
if we use the subdir without slash, we cannot visit the default index.html,
which we configured.It is not consistent with AWS S3.

Fixes: http://tracker.ceph.com/issues/20307
Signed-off-by: liuhong <liuhong@cmss.chinamobile.com>
@DHB-liuhong

This comment has been minimized.

DHB-liuhong commented Jun 29, 2017

@cbodley thanks

@cbodley cbodley added the needs-qa label Jun 29, 2017

@yuriw yuriw merged commit 76df830 into ceph:master Jul 7, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
default Build finished.
Details
make check make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment