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: multipart copy-part remove '/' for s3 java sdk request header. #15283

Merged
merged 1 commit into from Jun 22, 2017

Conversation

Projects
None yet
6 participants
@donglinpeng
Contributor

donglinpeng commented May 25, 2017

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 25, 2017

Is this intended as a fix for http://tracker.ceph.com/issues/20075 ? If so, can you add a line Fixes: http://tracker.ceph.com/issues/20075 right before the Signed-off-by line. Thanks; that will help with tracking!

@donglinpeng

This comment has been minimized.

Contributor

donglinpeng commented May 26, 2017

@smithfarm Done. Could you help to review ? Thanks a lot.

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 26, 2017

@donglinpeng I don't see any change in the commit message. Did you force-push?

root
@donglinpeng

This comment has been minimized.

Contributor

donglinpeng commented May 26, 2017

@smithfarm I resubmit commit message by force-push. Thanks a lot.

@donglinpeng

This comment has been minimized.

Contributor

donglinpeng commented May 31, 2017

@mattbenjamin Hi, this pull request is intended to fix http://tracker.ceph.com/issues/20075, could you please help to review ,thank u very much.

@donglinpeng donglinpeng reopened this May 31, 2017

@smithfarm smithfarm requested a review from mattbenjamin May 31, 2017

@mattbenjamin mattbenjamin self-assigned this Jun 7, 2017

@mattbenjamin

This comment has been minimized.

Contributor

mattbenjamin commented Jun 7, 2017

http://pulpito.ceph.com/mbenjamin-2017-06-06_22:05:23-rgw-wip-donglingpeng-copypart---basic-smithi/
(this is a pass; there are valgrind errors in rgw-multisite runs, seen elsewhere)

@mattbenjamin

lgtm

@mattbenjamin

This comment has been minimized.

Contributor

mattbenjamin commented Jun 22, 2017

retest this please

@mdw-at-linuxbox

This comment has been minimized.

Contributor

mdw-at-linuxbox commented Jun 22, 2017

I had a brief look at this. It's in fact correct for x-amz-copy-source to start with a leading '/'. The aws documentation shows this exact syntax in several examples (in my copy pg.385, s3-api.pdf). Since copy_source is a C string, the commit is doing a fine job of skipping the leading '/'.

@mattbenjamin mattbenjamin merged commit 45f4853 into ceph:master Jun 22, 2017

4 of 6 checks passed

arm64 make check arm64 make check failed
Details
make check make check failed
Details
Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
Unmodified Submodules submodules for project are unmodified
Details
default Build finished.
Details

@donglinpeng donglinpeng deleted the donglinpeng:copyPartUpload branch Jun 26, 2017

@mmgaggle

This comment has been minimized.

Member

mmgaggle commented Jul 13, 2017

This fix works in my environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment