From b5f6eb12e1004888bf9dec6aa9c4b0499093ebfb Mon Sep 17 00:00:00 2001 From: Colin Patrick McCabe Date: Thu, 14 Apr 2011 16:14:48 -0700 Subject: [PATCH] rgw: rework error handling a bit Rados Gateway: get rid of RGWOp::err. We already have req_state::err and that represents the same thing. Standardize nomenclature for errors. 'errno' is our internal representation of the error. 'code' is what is returned by S3. 'message' is the message at the end. Improve rgw_err. dump_errno shouldn't modify req_state, but just dump the error. A new function set_req_state_err sets the error based on an 'errno'. Signed-off-by: Colin McCabe --- src/rgw/rgw_common.cc | 39 +++++++++++++++ src/rgw/rgw_common.h | 17 ++++--- src/rgw/rgw_fs.cc | 16 +++--- src/rgw/rgw_log.cc | 4 +- src/rgw/rgw_op.cc | 17 ++----- src/rgw/rgw_op.h | 2 - src/rgw/rgw_os_auth.cc | 4 +- src/rgw/rgw_rados.cc | 17 ++++--- src/rgw/rgw_rest.cc | 111 +++++++++++++++++++---------------------- src/rgw/rgw_rest.h | 3 +- src/rgw/rgw_rest_os.cc | 27 +++++++--- src/rgw/rgw_rest_s3.cc | 39 +++++++++++---- 12 files changed, 176 insertions(+), 120 deletions(-) diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc index a9c14543560ae..631eb5ee73646 100644 --- a/src/rgw/rgw_common.cc +++ b/src/rgw/rgw_common.cc @@ -7,6 +7,45 @@ using namespace ceph::crypto; +rgw_err:: +rgw_err() +{ + clear(); +} + +rgw_err:: +rgw_err(int code_, const std::string &message_) + : code(code_), message(message_) +{ +} + +std::string rgw_err:: +code_to_str() const +{ + char buf[20]; + snprintf(buf, sizeof(buf), "%d", code); + return std::string(buf); +} + +void rgw_err:: +clear() +{ + code = 200; + message.clear(); +} + +bool rgw_err:: +is_clear() const +{ + return (code == 200); +} + +std::ostream& operator<<(std::ostream& oss, const rgw_err &err) +{ + oss << "rgw_err(code=" << err.code << ", message='" << err.message << "') "; + return oss; +} + /* Loglevel of the gateway */ int rgw_log_level = 20; diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index 5192aa8a1d936..18a95c1abebc3 100644 --- a/src/rgw/rgw_common.h +++ b/src/rgw/rgw_common.h @@ -67,16 +67,22 @@ extern string rgw_root_bucket; #define ERR_INVALID_OBJECT_NAME 2001 #define ERR_NO_SUCH_BUCKET 2002 #define ERR_METHOD_NOT_ALLOWED 2003 +#define ERR_INVALID_DIGEST 2004 +#define ERR_BAD_DIGEST 2005 typedef void *RGWAccessHandle; /** Store error returns for output at a different point in the program */ struct rgw_err { - const char *num; - const char *code; - const char *message; - - rgw_err() : num(NULL), code(NULL), message(NULL) {} + rgw_err(); + rgw_err(int code_, const std::string &message_); + std::string code_to_str() const; + void clear(); + bool is_clear() const; + friend std::ostream& operator<<(std::ostream& oss, const rgw_err &err); + + int code; + std::string message; }; /* Helper class used for XMLArgs parsing */ @@ -228,7 +234,6 @@ struct req_state { const char *query; const char *length; const char *content_type; - bool err_exist; struct rgw_err err; const char *status; bool expect_cont; diff --git a/src/rgw/rgw_fs.cc b/src/rgw/rgw_fs.cc index 15192e99092b3..4ca3c30d138fc 100644 --- a/src/rgw/rgw_fs.cc +++ b/src/rgw/rgw_fs.cc @@ -480,16 +480,16 @@ int RGWFS::prepare_get_obj(std::string& bucket, std::string& obj, r = -ECANCELED; if (mod_ptr) { if (st.st_mtime < *mod_ptr) { - err->num = "304"; - err->code = "NotModified"; + err->code = 304; + err->message = "NotModified"; goto done_err; } } if (unmod_ptr) { if (st.st_mtime >= *unmod_ptr) { - err->num = "412"; - err->code = "PreconditionFailed"; + err->code = 412; + err->message = "PreconditionFailed"; goto done_err; } } @@ -502,8 +502,8 @@ int RGWFS::prepare_get_obj(std::string& bucket, std::string& obj, if (if_match) { RGW_LOG(10) << "ETag: " << etag << " " << " If-Match: " << if_match << endl; if (strcmp(if_match, etag)) { - err->num = "412"; - err->code = "PreconditionFailed"; + err->code = 412; + err->message = "PreconditionFailed"; goto done_err; } } @@ -511,8 +511,8 @@ int RGWFS::prepare_get_obj(std::string& bucket, std::string& obj, if (if_nomatch) { RGW_LOG(10) << "ETag: " << etag << " " << " If_NoMatch: " << if_nomatch << endl; if (strcmp(if_nomatch, etag) == 0) { - err->num = "412"; - err->code = "PreconditionFailed"; + err->code = 412; + err->message = "PreconditionFailed"; goto done_err; } } diff --git a/src/rgw/rgw_log.cc b/src/rgw/rgw_log.cc index bc77b919082a2..b68b0b5ec8f9f 100644 --- a/src/rgw/rgw_log.cc +++ b/src/rgw/rgw_log.cc @@ -47,8 +47,8 @@ int rgw_log_op(struct req_state *s) else entry.http_status = "200"; // default - if (s->err_exist) - entry.error_code = s->err.code; + if (!s->err.is_clear()) + entry.error_code = s->err.code_to_str(); else entry.error_code = "-"; diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index bc4b201607d39..32605b661ffe4 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -172,7 +172,7 @@ void RGWGetObj::execute() init_common(); ret = rgwstore->prepare_get_obj(s->bucket_str, s->object_str, ofs, &end, &attrs, mod_ptr, - unmod_ptr, &lastmod, if_match, if_nomatch, &total_len, &handle, &err); + unmod_ptr, &lastmod, if_match, if_nomatch, &total_len, &handle, &s->err); if (ret < 0) goto done; @@ -384,7 +384,6 @@ void RGWPutObj::execute() ret = policy.create_canned(s->user.user_id, s->user.display_name, s->canned_acl); if (!ret) { - err.code = "InvalidArgument"; ret = -EINVAL; goto done; } @@ -400,8 +399,7 @@ void RGWPutObj::execute() supplied_md5_b64, supplied_md5_b64 + strlen(supplied_md5_b64)); RGW_LOG(15) << "ceph_armor ret=" << ret << endl; if (ret != CEPH_CRYPTO_MD5_DIGESTSIZE) { - err.code = "InvalidDigest"; - ret = -EINVAL; + ret = -ERR_INVALID_DIGEST; goto done; } @@ -431,8 +429,7 @@ void RGWPutObj::execute() buf_to_hex(m, CEPH_CRYPTO_MD5_DIGESTSIZE, calc_md5); if (supplied_md5_b64 && strcmp(calc_md5, supplied_md5)) { - err.code = "BadDigest"; - ret = -EINVAL; + ret = -ERR_BAD_DIGEST; goto done; } bufferlist aclbl; @@ -498,7 +495,6 @@ static bool parse_copy_source(const char *src, string& bucket, string& object) int RGWCopyObj::init_common() { - struct rgw_err err; RGWAccessControlPolicy dest_policy; bufferlist aclbl; bufferlist bl; @@ -516,14 +512,12 @@ int RGWCopyObj::init_common() ret = dest_policy.create_canned(s->user.user_id, s->user.display_name, s->canned_acl); if (!ret) { - err.code = "InvalidArgument"; ret = -EINVAL; return ret; } ret = parse_copy_source(s->copy_source, src_bucket, src_object); if (!ret) { - err.code = "InvalidArgument"; ret = -EINVAL; return ret; } @@ -578,7 +572,7 @@ void RGWCopyObj::execute() unmod_ptr, if_match, if_nomatch, - attrs, &err); + attrs, &s->err); done: send_response(); @@ -798,9 +792,8 @@ void RGWHandler::init_state(struct req_state *s, struct fcgx_state *fcgx) } s->fcgx = fcgx; s->content_started = false; - s->err_exist = false; + s->err.clear(); s->format = 0; - memset(&s->err, 0, sizeof(s->err)); if (s->acl) { delete s->acl; s->acl = new RGWAccessControlPolicy; diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h index e84c8022e7f01..a8bfe098474bd 100644 --- a/src/rgw/rgw_op.h +++ b/src/rgw/rgw_op.h @@ -34,14 +34,12 @@ extern int read_acls(struct req_state *s, bool only_bucket = false); class RGWOp { protected: struct req_state *s; - struct rgw_err err; public: RGWOp() {} virtual ~RGWOp() {} virtual void init(struct req_state *s) { this->s = s; - memset(&err, 0, sizeof(err)); } virtual void execute() = 0; }; diff --git a/src/rgw/rgw_os_auth.cc b/src/rgw/rgw_os_auth.cc index e795767e68cf1..dbb94d15239c9 100644 --- a/src/rgw/rgw_os_auth.cc +++ b/src/rgw/rgw_os_auth.cc @@ -202,9 +202,9 @@ void RGW_OS_Auth_Get::execute() ret = 204; done: - dump_errno(s, ret); + set_req_state_err(s, ret); + dump_errno(s); end_header(s); - } bool RGWHandler_OS_Auth::authorize(struct req_state *s) diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc index ea10287f8c096..4a4f23f99a9e4 100644 --- a/src/rgw/rgw_rados.cc +++ b/src/rgw/rgw_rados.cc @@ -570,8 +570,9 @@ int RGWRados::prepare_get_obj(std::string& bucket, std::string& oid, if (mod_ptr) { RGW_LOG(10) << "If-Modified-Since: " << *mod_ptr << " Last-Modified: " << ctime << endl; if (ctime < *mod_ptr) { - err->num = "304"; - err->code = "NotModified"; + err->code = 304; + err->message = "NotModified"; + goto done_err; } } @@ -579,8 +580,8 @@ int RGWRados::prepare_get_obj(std::string& bucket, std::string& oid, if (unmod_ptr) { RGW_LOG(10) << "If-UnModified-Since: " << *unmod_ptr << " Last-Modified: " << ctime << endl; if (ctime > *unmod_ptr) { - err->num = "412"; - err->code = "PreconditionFailed"; + err->code = 412; + err->message = "PreconditionFailed"; goto done_err; } } @@ -593,8 +594,8 @@ int RGWRados::prepare_get_obj(std::string& bucket, std::string& oid, if (if_match) { RGW_LOG(10) << "ETag: " << etag.c_str() << " " << " If-Match: " << if_match << endl; if (strcmp(if_match, etag.c_str())) { - err->num = "412"; - err->code = "PreconditionFailed"; + err->code = 412; + err->message = "PreconditionFailed"; goto done_err; } } @@ -602,8 +603,8 @@ int RGWRados::prepare_get_obj(std::string& bucket, std::string& oid, if (if_nomatch) { RGW_LOG(10) << "ETag: " << etag.c_str() << " " << " If-NoMatch: " << if_nomatch << endl; if (strcmp(if_nomatch, etag.c_str()) == 0) { - err->num = "304"; - err->code = "NotModified"; + err->code = 304; + err->message = "NotModified"; goto done_err; } } diff --git a/src/rgw/rgw_rest.cc b/src/rgw/rgw_rest.cc index b24d094064530..599bd2ffd2650 100644 --- a/src/rgw/rgw_rest.cc +++ b/src/rgw/rgw_rest.cc @@ -28,64 +28,54 @@ static void dump_status(struct req_state *s, const char *status) CGI_PRINTF(s,"Status: %s\n", status); } -struct errno_http { - int err; - const char *http_str; - const char *default_code; +struct rgw_html_errors { + int err_no; + int code; + const char *message; }; -const static struct errno_http hterrs[] = { - { 0, "200", "" }, - { 201, "201", "Created" }, - { 204, "204", "NoContent" }, - { 206, "206", "" }, - { EINVAL, "400", "InvalidArgument" }, - { ERR_INVALID_BUCKET_NAME, "400", "InvalidBucketName" }, - { ERR_INVALID_OBJECT_NAME, "400", "InvalidObjectName" }, - { EACCES, "403", "AccessDenied" }, - { EPERM, "403", "AccessDenied" }, - { ENOENT, "404", "NoSuchKey" }, - { ERR_NO_SUCH_BUCKET, "404", "NoSuchBucket" }, - { ERR_METHOD_NOT_ALLOWED, "405", "MethodNotAllowed" }, - { ETIMEDOUT, "408", "RequestTimeout" }, - { EEXIST, "409", "BucketAlreadyExists" }, - { ENOTEMPTY, "409", "BucketNotEmpty" }, - { ERANGE, "416", "InvalidRange" }, - { 0, NULL, NULL }}; - -void dump_errno(struct req_state *s, int err, struct rgw_err *rgwerr) -{ - int orig_err = err; - const char *err_str; - const char *code = (rgwerr ? rgwerr->code : NULL); - - if (!rgwerr || !rgwerr->num) { - err_str = "500"; - - if (err < 0) - err = -err; - - int i=0; - while (hterrs[i].http_str) { - if (err == hterrs[i].err) { - err_str = hterrs[i].http_str; - if (!code) - code = hterrs[i].default_code; - break; - } +const static struct rgw_html_errors RGW_HTML_ERRORS[] = { + { 0, 200, "" }, + { 201, 201, "Created" }, + { 204, 204, "NoContent" }, + { 206, 206, "" }, + { EINVAL, 400, "InvalidArgument" }, + { ERR_INVALID_DIGEST, 500, "InvalidDigest" }, + { ERR_BAD_DIGEST, 500, "BadDigest" }, + { ERR_INVALID_BUCKET_NAME, 400, "InvalidBucketName" }, + { ERR_INVALID_OBJECT_NAME, 400, "InvalidObjectName" }, + { EACCES, 403, "AccessDenied" }, + { EPERM, 403, "AccessDenied" }, + { ENOENT, 404, "NoSuchKey" }, + { ERR_NO_SUCH_BUCKET, 404, "NoSuchBucket" }, + { ERR_METHOD_NOT_ALLOWED, 405, "MethodNotAllowed" }, + { ETIMEDOUT, 408, "RequestTimeout" }, + { EEXIST, 409, "BucketAlreadyExists" }, + { ENOTEMPTY, 409, "BucketNotEmpty" }, + { ERANGE, 416, "InvalidRange" }, +}; - i++; +void set_req_state_err(struct req_state *s, int err_no) +{ + if (err_no < 0) + err_no = -err_no; + for (size_t i = 0; i < sizeof(RGW_HTML_ERRORS)/sizeof(RGW_HTML_ERRORS[0]); ++i) { + const struct rgw_html_errors *r = RGW_HTML_ERRORS + i; + if (err_no == r->err_no) { + s->err.code = r->code; + s->err.message = r->message; + return; } - } else { - err_str = rgwerr->num; } + s->err.code = 500; + s->err.message = "UnknownError"; +} - dump_status(s, err_str); - if (orig_err < 0) { - s->err_exist = true; - s->err.code = code; - s->err.message = (rgwerr ? rgwerr->message : NULL); - } +void dump_errno(struct req_state *s) +{ + char buf[32]; + snprintf(buf, sizeof(buf), "%d", s->err.code); + dump_status(s, buf); } void dump_content_length(struct req_state *s, size_t len) @@ -170,22 +160,22 @@ void end_header(struct req_state *s, const char *content_type) } } CGI_PRINTF(s,"Content-type: %s\r\n\r\n", content_type); - if (s->err_exist) { + if (!s->err.is_clear()) { dump_start(s); - struct rgw_err &err = s->err; s->formatter->open_obj_section("Error"); - if (err.code) - s->formatter->dump_value_int("Code", "%s", err.code); - if (err.message) - s->formatter->dump_value_str("Message", err.message); + if (s->err.code) + s->formatter->dump_value_int("Code", "%d", s->err.code); + if (!s->err.message.empty()) + s->formatter->dump_value_str("Message", s->err.message.c_str()); s->formatter->close_section("Error"); } s->header_ended = true; } -void abort_early(struct req_state *s, int err) +void abort_early(struct req_state *s, int err_no) { - dump_errno(s, err); + set_req_state_err(s, err_no); + dump_errno(s); end_header(s); s->formatter->flush(); } @@ -308,7 +298,6 @@ void init_entities_from_header(struct req_state *s) s->object_str = ""; s->status = NULL; - s->err_exist = false; s->header_ended = false; s->bytes_sent = 0; diff --git a/src/rgw/rgw_rest.h b/src/rgw/rgw_rest.h index 09403f8064c15..e280f88c4bf08 100644 --- a/src/rgw/rgw_rest.h +++ b/src/rgw/rgw_rest.h @@ -110,7 +110,8 @@ public: int *init_error); }; -extern void dump_errno(struct req_state *s, int err, struct rgw_err *rgwerr = NULL); +extern void set_req_state_err(struct req_state *s, int err_no); +extern void dump_errno(struct req_state *s); extern void end_header(struct req_state *s, const char *content_type = NULL); extern void dump_start(struct req_state *s); extern void list_all_buckets_start(struct req_state *s); diff --git a/src/rgw/rgw_rest_os.cc b/src/rgw/rgw_rest_os.cc index 988e72c75fa93..47b6eb1bcb724 100644 --- a/src/rgw/rgw_rest_os.cc +++ b/src/rgw/rgw_rest_os.cc @@ -4,7 +4,8 @@ void RGWListBuckets_REST_OS::send_response() { - dump_errno(s, ret); + set_req_state_err(s, ret); + dump_errno(s); dump_start(s); @@ -50,7 +51,8 @@ RGW_LOG(0) << "formatter->get_len=" << s->formatter->get_len() << std::endl; void RGWListBucket_REST_OS::send_response() { - dump_errno(s, (ret < 0 ? ret : 0)); + set_req_state_err(s, (ret < 0 ? ret : 0)); + dump_errno(s); dump_start(s); if (ret < 0) { @@ -120,7 +122,9 @@ void RGWStatBucket_REST_OS::send_response() if (ret >= 0) dump_container_metadata(s, bucket); - dump_errno(s, (ret < 0 ? ret : 0)); + if (ret < 0) + set_req_state_err(s, ret); + dump_errno(s); end_header(s); dump_start(s); @@ -129,7 +133,9 @@ void RGWStatBucket_REST_OS::send_response() void RGWCreateBucket_REST_OS::send_response() { - dump_errno(s, ret); + if (ret) + set_req_state_err(s, ret); + dump_errno(s); end_header(s); s->formatter->flush(); } @@ -140,7 +146,8 @@ void RGWDeleteBucket_REST_OS::send_response() if (!r) r = 204; - dump_errno(s, r); + set_req_state_err(s, r); + dump_errno(s); end_header(s); s->formatter->flush(); } @@ -150,7 +157,8 @@ void RGWPutObj_REST_OS::send_response() if (!ret) ret = 201; // "created" dump_etag(s, etag.c_str()); - dump_errno(s, ret, &err); + set_req_state_err(s, ret); + dump_errno(s); end_header(s); s->formatter->flush(); } @@ -161,7 +169,8 @@ void RGWDeleteObj_REST_OS::send_response() if (!r) r = 204; - dump_errno(s, r); + set_req_state_err(s, r); + dump_errno(s); end_header(s); s->formatter->flush(); } @@ -204,7 +213,9 @@ int RGWGetObj_REST_OS::send_response(void *handle) if (range_str && !ret) ret = 206; /* partial content */ - dump_errno(s, ret, &err); + if (ret) + set_req_state_err(s, ret); + dump_errno(s); if (!content_type) content_type = "binary/octet-stream"; end_header(s, content_type); diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index 70e208f02e08f..ca5af8c477caa 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -67,7 +67,9 @@ int RGWGetObj_REST_S3::send_response(void *handle) if (range_str && !ret) ret = 206; /* partial content */ - dump_errno(s, ret, &err); + if (ret) + set_req_state_err(s, ret); + dump_errno(s); if (!content_type) content_type = "binary/octet-stream"; end_header(s, content_type); @@ -85,7 +87,9 @@ send_data: void RGWListBuckets_REST_S3::send_response() { - dump_errno(s, ret); + if (ret) + set_req_state_err(s, ret); + dump_errno(s); dump_start(s); list_all_buckets_start(s); @@ -108,7 +112,9 @@ void RGWListBuckets_REST_S3::send_response() void RGWListBucket_REST_S3::send_response() { - dump_errno(s, (ret < 0 ? ret : 0)); + if (ret < 0) + set_req_state_err(s, ret); + dump_errno(s); end_header(s, "application/xml"); dump_start(s); @@ -154,7 +160,9 @@ void RGWListBucket_REST_S3::send_response() void RGWCreateBucket_REST_S3::send_response() { - dump_errno(s, ret); + if (ret) + set_req_state_err(s, ret); + dump_errno(s); end_header(s); s->formatter->flush(); } @@ -165,7 +173,9 @@ void RGWDeleteBucket_REST_S3::send_response() if (!r) r = 204; - dump_errno(s, r); + if (ret) + set_req_state_err(s, r); + dump_errno(s); end_header(s); s->formatter->flush(); } @@ -173,7 +183,9 @@ void RGWDeleteBucket_REST_S3::send_response() void RGWPutObj_REST_S3::send_response() { dump_etag(s, etag.c_str()); - dump_errno(s, ret, &err); + if (ret) + set_req_state_err(s, ret); + dump_errno(s); end_header(s); s->formatter->flush(); } @@ -184,14 +196,17 @@ void RGWDeleteObj_REST_S3::send_response() if (!r) r = 204; - dump_errno(s, r); + set_req_state_err(s, r); + dump_errno(s); end_header(s); s->formatter->flush(); } void RGWCopyObj_REST_S3::send_response() { - dump_errno(s, ret, &err); + if (ret) + set_req_state_err(s, ret); + dump_errno(s); end_header(s, "binary/octet-stream"); if (ret == 0) { @@ -212,7 +227,9 @@ void RGWCopyObj_REST_S3::send_response() void RGWGetACLs_REST_S3::send_response() { - dump_errno(s, ret); + if (ret) + set_req_state_err(s, ret); + dump_errno(s); end_header(s, "application/xml"); dump_start(s); s->formatter->flush(); @@ -221,7 +238,9 @@ void RGWGetACLs_REST_S3::send_response() void RGWPutACLs_REST_S3::send_response() { - dump_errno(s, ret); + if (ret) + set_req_state_err(s, ret); + dump_errno(s); end_header(s, "application/xml"); dump_start(s); s->formatter->flush(); -- 2.47.3