rgw: only fetch cors info when neededwip-5831

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>

7 files changed, 116 insertions, 109 deletions

diff --git a/src/rgw/rgw_main.cc b/src/rgw/rgw_main.cc
index 514e9e47171..07bf07366d5 100644
--- 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 0c3f4955940..d34e18bc4ba 100644
--- 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 5da2e4f472c..d158f831cc7 100644
--- 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 e4933a67a39..a0870708c44 100644
--- 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 0b7204fe9cb..ded5b88366a 100644
--- 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 6c1738218e6..bbf363804bd 100644
--- 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 a0af4eac9fd..b0d3c30384a 100644
--- 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();
 };