From 804bb853f1c7b5ce546c100d61c6fc526838c134 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Tue, 11 Dec 2018 16:13:58 -0500 Subject: [PATCH] rgw: parse_copy_location defers url-decode don't url-decode until after we search for the ?, or we'll truncate object names that contain a url-encoded ? Fixes: http://tracker.ceph.com/issues/27217 Signed-off-by: Casey Bodley --- src/rgw/rgw_op.cc | 10 ++++------ src/rgw/rgw_rest_s3.cc | 2 +- src/rgw/rgw_rest_swift.cc | 6 ++---- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 65f8ce159b1..c4f4c19b771 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -4613,6 +4613,7 @@ bool RGWCopyObj::parse_copy_location(const boost::string_view& url_src, boost::string_view name_str; boost::string_view params_str; + // search for ? before url-decoding so we don't accidentally match %3F size_t pos = url_src.find('?'); if (pos == string::npos) { name_str = url_src; @@ -4626,14 +4627,11 @@ bool RGWCopyObj::parse_copy_location(const boost::string_view& url_src, dec_src.remove_prefix(1); pos = dec_src.find('/'); - if (pos ==string::npos) + if (pos == string::npos) return false; - boost::string_view bn_view{dec_src.substr(0, pos)}; - bucket_name = std::string{bn_view.data(), bn_view.size()}; - - boost::string_view kn_view{dec_src.substr(pos + 1)}; - key.name = std::string{kn_view.data(), kn_view.size()}; + bucket_name = url_decode(dec_src.substr(0, pos)); + key.name = url_decode(dec_src.substr(pos + 1)); if (key.name.empty()) { return false; diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index acb7b308fa6..f9d8491a7c0 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -3476,7 +3476,7 @@ int RGWHandler_REST_S3::init(RGWRados *store, struct req_state *s, (! s->info.env->get("HTTP_X_AMZ_COPY_SOURCE_RANGE")) && (! s->info.args.exists("uploadId"))) { - ret = RGWCopyObj::parse_copy_location(url_decode(copy_source), + ret = RGWCopyObj::parse_copy_location(copy_source, s->init_state.src_bucket, s->src_object); if (!ret) { diff --git a/src/rgw/rgw_rest_swift.cc b/src/rgw/rgw_rest_swift.cc index 00ce5cdc777..d2f741d0267 100644 --- a/src/rgw/rgw_rest_swift.cc +++ b/src/rgw/rgw_rest_swift.cc @@ -3021,8 +3021,7 @@ int RGWHandler_REST_SWIFT::init(RGWRados* store, struct req_state* s, s->dialect = "swift"; - std::string copy_source = - url_decode(s->info.env->get("HTTP_X_COPY_FROM", "")); + std::string copy_source = s->info.env->get("HTTP_X_COPY_FROM", ""); if (! copy_source.empty()) { bool result = RGWCopyObj::parse_copy_location(copy_source, t->src_bucket, s->src_object); @@ -3031,8 +3030,7 @@ int RGWHandler_REST_SWIFT::init(RGWRados* store, struct req_state* s, } if (s->op == OP_COPY) { - std::string req_dest = - url_decode(s->info.env->get("HTTP_DESTINATION", "")); + std::string req_dest = s->info.env->get("HTTP_DESTINATION", ""); if (req_dest.empty()) return -ERR_BAD_URL; -- 2.39.5