From 0260abd5d265bd63ea9c89f4082c31ba1c5ae8fa Mon Sep 17 00:00:00 2001 From: "Robin H. Johnson" Date: Wed, 22 Apr 2015 12:53:06 -0700 Subject: [PATCH 1/3] rgw: improve content-length env var handling The FastCGI specification, section 6.3 on Authorizers, describes a case where HTTP_CONTENT_LENGTH will be set in the environment and CONTENT_LENGTH will NOT be set. Further documention in the code. Fixes: #11419 Signed-off-by: Robin H. Johnson (cherry picked from commit 3e38eab44bfb082fdd2b6f29b8b0357f8f5c11bb) --- src/rgw/rgw_rest.cc | 72 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/src/rgw/rgw_rest.cc b/src/rgw/rgw_rest.cc index 1ac46e982e5fb2..9b62ad5d41b78f 100644 --- a/src/rgw/rgw_rest.cc +++ b/src/rgw/rgw_rest.cc @@ -1348,7 +1348,77 @@ int RGWREST::preprocess(struct req_state *s, RGWClientIO *cio) } url_decode(s->info.request_uri, s->decoded_uri); - s->length = info.env->get("CONTENT_LENGTH"); + + /* FastCGI specification, section 6.3 + * http://www.fastcgi.com/devkit/doc/fcgi-spec.html#S6.3 + * === + * The Authorizer application receives HTTP request information from the Web + * server on the FCGI_PARAMS stream, in the same format as a Responder. The + * Web server does not send CONTENT_LENGTH, PATH_INFO, PATH_TRANSLATED, and + * SCRIPT_NAME headers. + * === + * Ergo if we are in Authorizer role, we MUST look at HTTP_CONTENT_LENGTH + * instead of CONTENT_LENGTH for the Content-Length. + * + * There is one slight wrinkle in this, and that's older versions of + * nginx/lighttpd/apache setting BOTH headers. As a result, we have to check + * both headers and can't always simply pick A or B. + */ + const char* s_contentlength = info.env->get("CONTENT_LENGTH"); + const char* s_httpcontentlength = info.env->get("HTTP_CONTENT_LENGTH"); + if(s_httpcontentlength && !s_contentlength) { + /* Easy case #1: CONTENT_LENGTH is missing or empty */ + s_contentlength = s_httpcontentlength; + s_httpcontentlength = NULL; + } else if (s_contentlength && s_httpcontentlength) { + /* Hard case: Both are set, we have to disambiguate */ + int64_t i_contentlength = -1, i_httpcontentlength = -1; + // Test CONTENT_LENGTH + if(*s_contentlength == '\0') { + i_contentlength = 0; + } else { + string err; + i_contentlength = strict_strtoll(s->length, 10, &err); + if (!err.empty()) { + i_contentlength = -1; // Set an error to allow fallthrough + } + } + // Test HTTP_CONTENT_LENGTH + if(*s_httpcontentlength == '\0') { + i_httpcontentlength = 0; + } else { + string err; + i_httpcontentlength = strict_strtoll(s->length, 10, &err); + if (!err.empty()) { + i_httpcontentlength = -1; // Set an error to allow fallthrough + } + } + // Now check them: + if(i_httpcontentlength == -1) { + // HTTP_CONTENT_LENGTH is invalid, ignore it + } else if(i_contentlength == -1) { + // CONTENT_LENGTH is invalid, and HTTP_CONTENT_LENGTH is valid + // Swap entries + s_contentlength = s_httpcontentlength; + s_httpcontentlength = NULL; + } else { + // both CONTENT_LENGTH and HTTP_CONTENT_LENGTH are valid + // Let's pick the larger size + if(i_contentlength >= i_httpcontentlength) { + // No need to swap + } else { + s_contentlength = s_httpcontentlength; + s_httpcontentlength = NULL; + } + } + + } else { + // By implication, httpcontentlength is NULL here + /* Easy case #2: HTTP_CONTENT_LENGTH is not set */ + s->length = s_contentlength; + } + + if (s->length) { if (*s->length == '\0') { s->content_length = 0; From d9bbef3e470c6406bb65dc40e7e9c08c5d599f73 Mon Sep 17 00:00:00 2001 From: "Robin H. Johnson" Date: Fri, 24 Apr 2015 10:49:16 -0700 Subject: [PATCH 2/3] rgw: make compatability deconfliction optional. Per request from Yehuda, the deconfliction for having both HTTP_CONTENT_LENGTH and CONTENT_LENGTH set is now optional, and controlled by new configuration boolean, which defaults to false. rgw content length compat X-URL: https://github.com/ceph/ceph/pull/4436#issuecomment-95994887 Signed-off-by: Robin H. Johnson (cherry picked from commit 79d17af1a1ec0659884f768945a7bac6282b5e0b) --- doc/radosgw/config-ref.rst | 6 ++++++ src/common/config_opts.h | 1 + src/rgw/rgw_rest.cc | 4 ++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/doc/radosgw/config-ref.rst b/doc/radosgw/config-ref.rst index c78919cfa8e71a..a5eed1f6bb2bea 100644 --- a/doc/radosgw/config-ref.rst +++ b/doc/radosgw/config-ref.rst @@ -316,6 +316,12 @@ Ceph configuration file, the default value will be set automatically. :Default: ``admin`` +``rgw content length compat`` + +:Description: Enable compatability handling of FCGI requests with both CONTENT_LENGTH AND HTTP_CONTENT_LENGTH set. +:Type: Boolean +:Default: ``false`` + Regions ======= diff --git a/src/common/config_opts.h b/src/common/config_opts.h index 9c004322e7b56d..7fc24b1d2b1e13 100644 --- a/src/common/config_opts.h +++ b/src/common/config_opts.h @@ -938,6 +938,7 @@ OPTION(rgw_socket_path, OPT_STR, "") // path to unix domain socket, if not spe OPTION(rgw_host, OPT_STR, "") // host for radosgw, can be an IP, default is 0.0.0.0 OPTION(rgw_port, OPT_STR, "") // port to listen, format as "8080" "5000", if not specified, rgw will not run external fcgi OPTION(rgw_dns_name, OPT_STR, "") +OPTION(rgw_content_length_compat, OPT_BOOL, false) // Check both HTTP_CONTENT_LENGTH and CONTENT_LENGTH in fcgi env OPTION(rgw_script_uri, OPT_STR, "") // alternative value for SCRIPT_URI if not set in request OPTION(rgw_request_uri, OPT_STR, "") // alternative value for REQUEST_URI if not set in request OPTION(rgw_swift_url, OPT_STR, "") // the swift url, being published by the internal swift auth diff --git a/src/rgw/rgw_rest.cc b/src/rgw/rgw_rest.cc index 9b62ad5d41b78f..a5ce99567e9cc8 100644 --- a/src/rgw/rgw_rest.cc +++ b/src/rgw/rgw_rest.cc @@ -1370,7 +1370,7 @@ int RGWREST::preprocess(struct req_state *s, RGWClientIO *cio) /* Easy case #1: CONTENT_LENGTH is missing or empty */ s_contentlength = s_httpcontentlength; s_httpcontentlength = NULL; - } else if (s_contentlength && s_httpcontentlength) { + } else if (s->cct->_conf->rgw_content_length_compat && s_contentlength && s_httpcontentlength) { /* Hard case: Both are set, we have to disambiguate */ int64_t i_contentlength = -1, i_httpcontentlength = -1; // Test CONTENT_LENGTH @@ -1411,7 +1411,7 @@ int RGWREST::preprocess(struct req_state *s, RGWClientIO *cio) s_httpcontentlength = NULL; } } - + // End of: else if (s->cct->_conf->rgw_content_length_compat && s_contentlength && s_httpcontentlength) { } else { // By implication, httpcontentlength is NULL here /* Easy case #2: HTTP_CONTENT_LENGTH is not set */ From 56c2688b87d7d78831f8e147fc67cc0651ab644c Mon Sep 17 00:00:00 2001 From: Yehuda Sadeh Date: Fri, 24 Apr 2015 14:45:40 -0700 Subject: [PATCH 3/3] rgw: simplify content length handling Signed-off-by: Yehuda Sadeh (cherry picked from commit e97fd5052cab83c5f699531a8c960b93437a8f9f) --- src/rgw/rgw_rest.cc | 79 ++++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/src/rgw/rgw_rest.cc b/src/rgw/rgw_rest.cc index a5ce99567e9cc8..695c16e6c97329 100644 --- a/src/rgw/rgw_rest.cc +++ b/src/rgw/rgw_rest.cc @@ -1301,6 +1301,22 @@ RGWRESTMgr::~RGWRESTMgr() delete default_mgr; } +static int64_t parse_content_length(const char *content_length) +{ + int64_t len = -1; + + if (*content_length == '\0') { + len = 0; + } else { + string err; + len = strict_strtoll(content_length, 10, &err); + if (!err.empty()) { + len = -1; + } + } + + return len; +} int RGWREST::preprocess(struct req_state *s, RGWClientIO *cio) { req_info& info = s->info; @@ -1364,58 +1380,39 @@ int RGWREST::preprocess(struct req_state *s, RGWClientIO *cio) * nginx/lighttpd/apache setting BOTH headers. As a result, we have to check * both headers and can't always simply pick A or B. */ - const char* s_contentlength = info.env->get("CONTENT_LENGTH"); - const char* s_httpcontentlength = info.env->get("HTTP_CONTENT_LENGTH"); - if(s_httpcontentlength && !s_contentlength) { - /* Easy case #1: CONTENT_LENGTH is missing or empty */ - s_contentlength = s_httpcontentlength; - s_httpcontentlength = NULL; - } else if (s->cct->_conf->rgw_content_length_compat && s_contentlength && s_httpcontentlength) { + const char* content_length = info.env->get("CONTENT_LENGTH"); + const char* http_content_length = info.env->get("HTTP_CONTENT_LENGTH"); + if (!http_content_length != !content_length) { + /* Easy case: one or the other is missing */ + s->length = (content_length ? content_length : http_content_length); + } else if (s->cct->_conf->rgw_content_length_compat && content_length && http_content_length) { /* Hard case: Both are set, we have to disambiguate */ - int64_t i_contentlength = -1, i_httpcontentlength = -1; - // Test CONTENT_LENGTH - if(*s_contentlength == '\0') { - i_contentlength = 0; - } else { - string err; - i_contentlength = strict_strtoll(s->length, 10, &err); - if (!err.empty()) { - i_contentlength = -1; // Set an error to allow fallthrough - } - } - // Test HTTP_CONTENT_LENGTH - if(*s_httpcontentlength == '\0') { - i_httpcontentlength = 0; - } else { - string err; - i_httpcontentlength = strict_strtoll(s->length, 10, &err); - if (!err.empty()) { - i_httpcontentlength = -1; // Set an error to allow fallthrough - } - } + int64_t content_length_i, http_content_length_i; + + content_length_i = parse_content_length(content_length); + http_content_length_i = parse_content_length(http_content_length); + // Now check them: - if(i_httpcontentlength == -1) { + if (http_content_length_i < 0) { // HTTP_CONTENT_LENGTH is invalid, ignore it - } else if(i_contentlength == -1) { + } else if (content_length_i < 0) { // CONTENT_LENGTH is invalid, and HTTP_CONTENT_LENGTH is valid // Swap entries - s_contentlength = s_httpcontentlength; - s_httpcontentlength = NULL; + content_length = http_content_length; } else { // both CONTENT_LENGTH and HTTP_CONTENT_LENGTH are valid // Let's pick the larger size - if(i_contentlength >= i_httpcontentlength) { - // No need to swap - } else { - s_contentlength = s_httpcontentlength; - s_httpcontentlength = NULL; + if (content_length_i < http_content_length_i) { + // prefer the larger value + content_length = http_content_length; } } - // End of: else if (s->cct->_conf->rgw_content_length_compat && s_contentlength && s_httpcontentlength) { + s->length = content_length; + // End of: else if (s->cct->_conf->rgw_content_length_compat && content_length && + // http_content_length) } else { - // By implication, httpcontentlength is NULL here - /* Easy case #2: HTTP_CONTENT_LENGTH is not set */ - s->length = s_contentlength; + /* no content length was defined */ + s->length = NULL; }