rgw: only fetch cors info when needed

Fixes: #5831
This commit moves around the cors handling code. Beforehand
we were unnecessarily reading the cors headers for every
request whether that was needed or not. Moved that code to
be only called when needed. While at it, cleaned up the
layering a bit so that not to mix S3 specific code with
the generic functionality (except for debugging).

Signed-off-by: Yehuda Sadeh <yehuda@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

diff --git a/src/rgw/rgw_main.cc b/src/rgw/rgw_main.cc
index 514e9e47171cdc1c29f328586ce106b81c5bc407..07bf07366d5bcca946d5acc30de71ff741640c96 100644 (file)
--- a/src/rgw/rgw_main.cc
+++ b/src/rgw/rgw_main.cc
@@ -341,9 +341,6 @@ void RGWProcess::handle_request(RGWRequest *req)
     goto done;
   }
 
-  req->log(s, "reading the cors attr");
-  handler->read_cors_config();
- 
   req->log(s, "verifying op mask");
   ret = op->verify_op_mask();
   if (ret < 0) {

diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
index 0c3f4955940b00c779f64d12614bb506f2dbadbb..d34e18bc4bab82d4d8b5f071da7fdf4a035024b3 100644 (file)
--- a/src/rgw/rgw_op.cc
+++ b/src/rgw/rgw_op.cc
@@ -297,6 +297,7 @@ static int read_policy(RGWRados *store, struct req_state *s,
       ret = -EACCES;
     else
       ret = -ENOENT;
+
   } else if (ret == -ENOENT) {
       ret = -ERR_NO_SUCH_BUCKET;
   }
@@ -946,7 +947,7 @@ void RGWCreateBucket::execute()
 {
   RGWAccessControlPolicy old_policy(s->cct);
   map<string, bufferlist> attrs;
-  bufferlist aclbl, corsbl;
+  bufferlist aclbl;
   bool existed;
   int r;
   rgw_obj obj(store->zone.domain_root, s->bucket_name_str);
@@ -1834,6 +1835,37 @@ void RGWPutACLs::execute()
   }
 }
 
+static int read_bucket_cors(RGWRados *store, struct req_state *s, RGWCORSConfiguration *bucket_cors, bool *exist)
+{
+  bufferlist bl;
+
+  map<string, bufferlist>::iterator aiter = s->bucket_attrs.find(RGW_ATTR_CORS);
+  if (aiter == s->bucket_attrs.end()) {
+    ldout(s->cct, 20) << "no CORS configuration attr found" << dendl;
+    *exist = false;
+    return 0; /* no CORS configuration found */
+  }
+
+  *exist = true;
+
+  bl = aiter->second;
+
+  bufferlist::iterator iter = bl.begin();
+  try {
+    bucket_cors->decode(iter);
+  } catch (buffer::error& err) {
+    ldout(s->cct, 0) << "ERROR: could not decode policy, caught buffer::error" << dendl;
+    return -EIO;
+  }
+  if (s->cct->_conf->subsys.should_gather(ceph_subsys_rgw, 15)) {
+    RGWCORSConfiguration_S3 *s3cors = static_cast<RGWCORSConfiguration_S3 *>(bucket_cors);
+    ldout(s->cct, 15) << "Read RGWCORSConfiguration";
+    s3cors->to_xml(*_dout);
+    *_dout << dendl;
+  }
+  return 0;
+}
+
 int RGWGetCORS::verify_permission()
 {
   if (s->user.user_id.compare(s->bucket_owner.get_id()) != 0)
@@ -1844,15 +1876,17 @@ int RGWGetCORS::verify_permission()
 
 void RGWGetCORS::execute()
 {
-  stringstream ss;
-  if (!s->bucket_cors) {
+  bool cors_exist;
+
+  ret = read_bucket_cors(store, s, &bucket_cors, &cors_exist);
+  if (ret < 0)
+    return ;
+
+  if (!cors_exist) {
     dout(2) << "No CORS configuration set yet for this bucket" << dendl;
     ret = -ENOENT;
     return;
   }
-  RGWCORSConfiguration_S3 *s3cors = static_cast<RGWCORSConfiguration_S3 *>(s->bucket_cors);
-  s3cors->to_xml(ss);
-  cors = ss.str(); 
 }
 
 int RGWPutCORS::verify_permission()
@@ -1865,45 +1899,17 @@ int RGWPutCORS::verify_permission()
 
 void RGWPutCORS::execute()
 {
-  bufferlist bl;
-
-  RGWCORSConfiguration_S3 *cors_config;
-  RGWCORSXMLParser_S3 parser(s->cct);
   rgw_obj obj;
 
-  ret = 0;
-
-  if (!parser.init()) {
-    ret = -EINVAL;
-    return;
-  }
   ret = get_params();
   if (ret < 0)
     return;
 
-  ldout(s->cct, 15) << "read len=" << len << " data=" << (data ? data : "") << dendl;
-  if (!parser.parse(data, len, 1)) {
-    ret = -EINVAL;
-    return;
-  }
-  cors_config = static_cast<RGWCORSConfiguration_S3 *>(parser.find_first("CORSConfiguration"));
-  if (!cors_config) {
-    ret = -EINVAL;
-    return;
-  }
-
-  if (s->cct->_conf->subsys.should_gather(ceph_subsys_rgw, 15)) {
-    ldout(s->cct, 15) << "CORSConfiguration";
-    cors_config->to_xml(*_dout);
-    *_dout << dendl;
-  }
-
   RGWObjVersionTracker *ptracker = (s->object ? NULL : &s->bucket_info.objv_tracker);
 
-  cors_config->encode(bl);
   store->get_bucket_instance_obj(s->bucket, obj);
   store->set_atomic(s->obj_ctx, obj);
-  ret = store->set_attr(s->obj_ctx, obj, RGW_ATTR_CORS, bl, ptracker);
+  ret = store->set_attr(s->obj_ctx, obj, RGW_ATTR_CORS, cors_bl, ptracker);
 }
 
 int RGWDeleteCORS::verify_permission()
@@ -1916,9 +1922,15 @@ int RGWDeleteCORS::verify_permission()
 
 void RGWDeleteCORS::execute()
 {
+  bool cors_exist;
+  RGWCORSConfiguration bucket_cors;
+  ret = read_bucket_cors(store, s, &bucket_cors, &cors_exist);
+  if (ret < 0)
+    return;
+
   bufferlist bl;
   rgw_obj obj;
-  if (!s->bucket_cors) {
+  if (!cors_exist) {
     dout(2) << "No CORS configuration set yet for this bucket" << dendl;
     ret = -ENOENT;
     return;
@@ -2510,41 +2522,6 @@ int RGWHandler::do_read_permissions(RGWOp *op, bool only_bucket)
   return ret;
 }
 
-int RGWHandler::read_cors_config(void)
-{
-  int ret = 0;
-  bufferlist bl;
-
-  if (s->bucket.name.empty())
-    return 0;
-
-  dout(10) << "Going to read cors from attrs" << dendl;
-  rgw_obj obj;
-  store->get_bucket_instance_obj(s->bucket, obj);
-  ret = store->get_attr(s->obj_ctx, obj, RGW_ATTR_CORS, bl);
-  if (ret >= 0) {
-    bufferlist::iterator iter = bl.begin();
-    s->bucket_cors = new RGWCORSConfiguration();
-    try {
-      s->bucket_cors->decode(iter);
-    } catch (buffer::error& err) {
-      ldout(s->cct, 0) << "ERROR: could not decode policy, caught buffer::error" << dendl;
-      return -EIO;
-    }
-    if (s->cct->_conf->subsys.should_gather(ceph_subsys_rgw, 15)) {
-      RGWCORSConfiguration_S3 *s3cors = static_cast<RGWCORSConfiguration_S3 *>(s->bucket_cors);
-      ldout(s->cct, 15) << "Read RGWCORSConfiguration";
-      s3cors->to_xml(*_dout);
-      *_dout << dendl;
-    }
-  } else {
-      /*Not a serious error*/
-      dout(2) << "Warning: There is no content for CORS xattr,"
-            " cors may not be set yet" << dendl;
-  }
-  return ret;
-}
-
 
 RGWOp *RGWHandler::get_op(RGWRados *store)
 {

diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h
index 5da2e4f472c7a6cb4a26ed01aacb72979623bdd9..d158f831cc718e5e70a5acdb18852b5e19f7756a 100644 (file)
--- a/src/rgw/rgw_op.h
+++ b/src/rgw/rgw_op.h
@@ -526,7 +526,7 @@ public:
 class RGWGetCORS : public RGWOp {
 protected:
   int ret;
-  string cors;
+  RGWCORSConfiguration bucket_cors;
 
 public:
   RGWGetCORS() : ret(0) {}
@@ -542,18 +542,13 @@ public:
 class RGWPutCORS : public RGWOp {
 protected:
   int ret;
-  size_t len;
-  char *data;
+  bufferlist cors_bl;
 
 public:
   RGWPutCORS() {
     ret = 0;
-    len = 0;
-    data = NULL;
-  }
-  virtual ~RGWPutCORS() {
-    free(data);
   }
+  virtual ~RGWPutCORS() { }
 
   int verify_permission();
   void execute();
@@ -849,7 +844,6 @@ protected:
   virtual RGWOp *op_options() { return NULL; }
 public:
   RGWHandler() : store(NULL), s(NULL) {}
-  int read_cors_config();
   virtual ~RGWHandler();
   virtual int init(RGWRados *store, struct req_state *_s, RGWClientIO *cio);
 

diff --git a/src/rgw/rgw_rest.cc b/src/rgw/rgw_rest.cc
index e4933a67a3924c7c49ddba587b034c8bd67a4696..a0870708c440d38d276632f3a5e0ad94dbe7f176 100644 (file)
--- a/src/rgw/rgw_rest.cc
+++ b/src/rgw/rgw_rest.cc
@@ -745,30 +745,6 @@ int RGWPutACLs_ObjStore::get_params()
   return ret;
 }
 
-int RGWPutCORS_ObjStore::get_params()
-{
-  size_t cl = 0;
-  if (s->length)
-    cl = atoll(s->length);
-  if (cl) {
-    data = (char *)malloc(cl + 1);
-    if (!data) {
-       ret = -ENOMEM;
-       return ret;
-    }
-    int read_len;
-    int r = s->cio->read(data, cl, &read_len);
-    len = read_len;
-    if (r < 0)
-      return r;
-    data[len] = '\0';
-  } else {
-    len = 0;
-  }
-
-  return ret;
-}
-
 static int read_all_chunked_input(req_state *s, char **pdata, int *plen, int max_read)
 {
 #define READ_CHUNK 4096

diff --git a/src/rgw/rgw_rest.h b/src/rgw/rgw_rest.h
index 0b7204fe9cb173936c245c5b0b4bc13d6f0d611b..ded5b88366a6cf555ab2e4d4eebcf5742f36b623 100644 (file)
--- a/src/rgw/rgw_rest.h
+++ b/src/rgw/rgw_rest.h
@@ -162,8 +162,6 @@ class RGWPutCORS_ObjStore : public RGWPutCORS {
 public:
   RGWPutCORS_ObjStore() {}
   ~RGWPutCORS_ObjStore() {}
-
-  int get_params();
 };
 
 class RGWDeleteCORS_ObjStore : public RGWDeleteCORS {

diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc
index 6c1738218e6355b2defbf5e1bad8d76f0417ea28..bbf363804bde8c4bbe28a5e29b31dca6dc0dbc57 100644 (file)
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -13,6 +13,7 @@
 #include "rgw_policy_s3.h"
 #include "rgw_user.h"
 #include "rgw_cors.h"
+#include "rgw_cors_s3.h"
 
 #include "rgw_client_io.h"
 
@@ -1391,10 +1392,73 @@ void RGWGetCORS_ObjStore_S3::send_response()
   end_header(s, "application/xml");
   dump_start(s);
   if (!ret) {
+    string cors;
+    RGWCORSConfiguration_S3 *s3cors = static_cast<RGWCORSConfiguration_S3 *>(&bucket_cors);
+    stringstream ss;
+
+    s3cors->to_xml(ss);
+    cors = ss.str();
     s->cio->write(cors.c_str(), cors.size());
   }
 }
 
+int RGWPutCORS_ObjStore_S3::get_params()
+{
+  int r;
+  char *data = NULL;
+  int len = 0;
+  size_t cl = 0;
+  RGWCORSXMLParser_S3 parser(s->cct);
+  RGWCORSConfiguration_S3 *cors_config;
+
+  if (s->length)
+    cl = atoll(s->length);
+  if (cl) {
+    data = (char *)malloc(cl + 1);
+    if (!data) {
+       r = -ENOMEM;
+       goto done_err;
+    }
+    int read_len;
+    r = s->cio->read(data, cl, &read_len);
+    len = read_len;
+    if (r < 0)
+      goto done_err;
+    data[len] = '\0';
+  } else {
+    len = 0;
+  }
+
+  if (!parser.init()) {
+    r = -EINVAL;
+    goto done_err;
+  }
+
+  if (!parser.parse(data, len, 1)) {
+    r = -EINVAL;
+    goto done_err;
+  }
+  cors_config = static_cast<RGWCORSConfiguration_S3 *>(parser.find_first("CORSConfiguration"));
+  if (!cors_config) {
+    r = -EINVAL;
+    goto done_err;
+  }
+
+  if (s->cct->_conf->subsys.should_gather(ceph_subsys_rgw, 15)) {
+    ldout(s->cct, 15) << "CORSConfiguration";
+    cors_config->to_xml(*_dout);
+    *_dout << dendl;
+  }
+
+  cors_config->encode(cors_bl);
+
+  free(data);
+  return 0;
+done_err:
+  free(data);
+  return r;
+}
+
 void RGWPutCORS_ObjStore_S3::send_response()
 {
   if (ret)

diff --git a/src/rgw/rgw_rest_s3.h b/src/rgw/rgw_rest_s3.h
index a0af4eac9fd951d3d2728396fe941697374862f7..b0d3c30384ab9f34a22283a5b4b80608c9dda958 100644 (file)
--- a/src/rgw/rgw_rest_s3.h
+++ b/src/rgw/rgw_rest_s3.h
@@ -184,6 +184,7 @@ public:
   RGWPutCORS_ObjStore_S3() {}
   ~RGWPutCORS_ObjStore_S3() {}
 
+  int get_params();
   void send_response();
 };