Merge pull request #8379: hammer: RGW shouldn't send Content-Type nor…

… Content-Length for 304 responses

Reviewed-by: Loic Dachary <ldachary@redhat.com>

src/rgw/rgw_civetweb.cc

@@ -27,7 +27,8 @@ int RGWMongoose::write_data(const char *buf, int len)
   return r;
 }
 
-RGWMongoose::RGWMongoose(mg_connection *_conn, int _port) : conn(_conn), port(_port), header_done(false), sent_header(false), has_content_length(false),
+RGWMongoose::RGWMongoose(mg_connection *_conn, int _port) : conn(_conn), port(_port), status_num(0), header_done(false),
+                                                 sent_header(false), has_content_length(false),
                                                  explicit_keepalive(false), explicit_conn_close(false)
 {
 }
@@ -45,9 +46,23 @@ int RGWMongoose::complete_request()
 {
   if (!sent_header) {
     if (!has_content_length) {
+
       header_done = false; /* let's go back to writing the header */
 
-      if (0 && data.length() == 0) {
+      /*
+       * Status 204 should not include a content-length header
+       * RFC7230 says so
+       *
+       * Same goes for status 304: Not Modified
+       *
+       * 'If a cache uses a received 304 response to update a cache entry,'
+       * 'the cache MUST update the entry to reflect any new field values'
+       * 'given in the response.'
+       *
+       */
+      if (status_num == 204 || status_num == 304) {
+        has_content_length = true;
+      } else if (0 && data.length() == 0) {
         has_content_length = true;
         print("Transfer-Enconding: %s\r\n", "chunked");
         data.append("0\r\n\r\n", sizeof("0\r\n\r\n")-1);
@@ -75,6 +90,7 @@ void RGWMongoose::init_env(CephContext *cct)
 {
   env.init(cct);
   struct mg_request_info *info = mg_get_request_info(conn);
+
   if (!info)
     return;
 
@@ -114,7 +130,7 @@ void RGWMongoose::init_env(CephContext *cct)
       *dest = c;
     }
     *dest = '\0';
-    
+
     env.set(buf, header->value);
   }
 
@@ -136,21 +152,21 @@ void RGWMongoose::init_env(CephContext *cct)
   }
 }
 
-int RGWMongoose::send_status(const char *status, const char *status_name)
+int RGWMongoose::send_status(int status, const char *status_name)
 {
   char buf[128];
 
   if (!status_name)
     status_name = "";
 
-  snprintf(buf, sizeof(buf), "HTTP/1.1 %s %s\r\n", status, status_name);
+  snprintf(buf, sizeof(buf), "HTTP/1.1 %d %s\r\n", status, status_name);
 
   bufferlist bl;
   bl.append(buf);
   bl.append(header_data);
   header_data = bl;
 
-  int status_num = atoi(status);
+  status_num = status;
   mg_set_http_status(conn, status_num);
 
   return 0;

src/rgw/rgw_civetweb.h

@@ -19,6 +19,7 @@ class RGWMongoose : public RGWClientIO
   bufferlist data;
 
   int port;
+  int status_num;
 
   bool header_done;
   bool sent_header;
@@ -32,7 +33,7 @@ class RGWMongoose : public RGWClientIO
   int write_data(const char *buf, int len);
   int read_data(char *buf, int len);
 
-  int send_status(const char *status, const char *status_name);
+  int send_status(int status, const char *status_name);
   int send_100_continue();
   int complete_header();
   int complete_request();

src/rgw/rgw_client_io.h

@@ -34,7 +34,7 @@ class RGWClientIO {
   virtual void flush() = 0;
   int read(char *buf, int max, int *actual);
 
-  virtual int send_status(const char *status, const char *status_name) = 0;
+  virtual int send_status(int status, const char *status_name) = 0;
   virtual int send_100_continue() = 0;
   virtual int complete_header() = 0;
   virtual int complete_request() = 0;

src/rgw/rgw_fcgi.cc

@@ -31,14 +31,15 @@ void RGWFCGX::init_env(CephContext *cct)
   env.init(cct, (char **)fcgx->envp);
 }
 
-int RGWFCGX::send_status(const char *status, const char *status_name)
+int RGWFCGX::send_status(int status, const char *status_name)
 {
-  return print("Status: %s %s\r\n", status, status_name);
+  status_num = status;
+  return print("Status: %d %s\r\n", status, status_name);
 }
 
 int RGWFCGX::send_100_continue()
 {
-  int r = send_status("100", "Continue");
+  int r = send_status(100, "Continue");
   if (r >= 0) {
     flush();
   }
@@ -47,6 +48,13 @@ int RGWFCGX::send_100_continue()
 
 int RGWFCGX::send_content_length(uint64_t len)
 {
+  /*
+   * Status 204 should not include a content-length header
+   * RFC7230 says so
+   */
+  if (status_num == 204)
+    return 0;
+
   char buf[21];
   snprintf(buf, sizeof(buf), "%" PRIu64, len);
   return print("Content-Length: %s\r\n", buf);
@@ -56,4 +64,3 @@ int RGWFCGX::complete_header()
 {
   return print("\r\n");
 }
-

src/rgw/rgw_fcgi.h

@@ -13,18 +13,21 @@ struct FCGX_Request;
 class RGWFCGX : public RGWClientIO
 {
   FCGX_Request *fcgx;
+
+  int status_num;
+
 protected:
   void init_env(CephContext *cct);
   int write_data(const char *buf, int len);
   int read_data(char *buf, int len);
 
-  int send_status(const char *status, const char *status_name);
+  int send_status(int status, const char *status_name);
   int send_100_continue();
   int complete_header();
   int complete_request() { return 0; }
   int send_content_length(uint64_t len);
 public:
-  RGWFCGX(FCGX_Request *_fcgx) : fcgx(_fcgx) {}
+  RGWFCGX(FCGX_Request *_fcgx) : fcgx(_fcgx), status_num(0) {}
   void flush();
 };
 

src/rgw/rgw_loadgen.cc

@@ -92,7 +92,7 @@ void RGWLoadGenIO::init_env(CephContext *cct)
   env.set("SERVER_PORT", port_buf);
 }
 
-int RGWLoadGenIO::send_status(const char *status, const char *status_name)
+int RGWLoadGenIO::send_status(int status, const char *status_name)
 {
   return 0;
 }

src/rgw/rgw_loadgen.h

@@ -34,7 +34,7 @@ class RGWLoadGenIO : public RGWClientIO
   int write_data(const char *buf, int len);
   int read_data(char *buf, int len);
 
-  int send_status(const char *status, const char *status_name);
+  int send_status(int status, const char *status_name);
   int send_100_continue();
   int complete_header();
   int complete_request();

src/rgw/rgw_rest.cc

@@ -246,7 +246,7 @@ static bool rgw_find_host_in_domains(const string& host, string *domain, string
   return false;
 }
 
-static void dump_status(struct req_state *s, const char *status, const char *status_name)
+static void dump_status(struct req_state *s, int status, const char *status_name)
 {
   int r = s->cio->send_status(status, status_name);
   if (r < 0) {
@@ -305,16 +305,12 @@ void set_req_state_err(struct req_state *s, int err_no)
 
 void dump_errno(struct req_state *s)
 {
-  char buf[32];
-  snprintf(buf, sizeof(buf), "%d", s->err.http_ret);
-  dump_status(s, buf, http_status_names[s->err.http_ret]);
+  dump_status(s, s->err.http_ret, http_status_names[s->err.http_ret]);
 }
 
-void dump_errno(struct req_state *s, int err)
+void dump_errno(struct req_state *s, int http_ret)
 {
-  char buf[32];
-  snprintf(buf, sizeof(buf), "%d", err);
-  dump_status(s, buf, http_status_names[s->err.http_ret]);
+  dump_status(s, http_ret, http_status_names[http_ret]);
 }
 
 void dump_string_header(struct req_state *s, const char *name, const char *val)

src/rgw/rgw_rest_s3.cc

@@ -167,10 +167,14 @@ int RGWGetObj_ObjStore_S3::send_response_data(bufferlist& bl, off_t bl_ofs, off_
     s->cio->print("%s: %s\r\n", riter->first.c_str(), riter->second.c_str());
   }
 
-  if (!content_type)
-    content_type = "binary/octet-stream";
+  if (ret == ERR_NOT_MODIFIED) {
+      end_header(s, this);
+  } else {
+      if (!content_type)
+          content_type = "binary/octet-stream";
 
-  end_header(s, this, content_type);
+      end_header(s, this, content_type);
+  }
 
   if (metadata_bl.length()) {
     s->cio->write(metadata_bl.c_str(), metadata_bl.length());