Changeset 35138

Timestamp:

08/31/09 09:17:59 (4 years ago)

Author:

kazuho

Message:

no more use std::string to build response, fix memory overruns

Location:

platform/mysql/mycached/trunk

Files:

• mycached.cc (modified) (17 diffs)
• mycached.hh (modified) (7 diffs)
• testclient.cc (modified) (1 diff)

platform/mysql/mycached/trunk/mycached.cc

@@ -77 +77 @@
 
 inline void
-escape_n_append(string& json, const char ch)
+json_escape_n_append(conn_t& conn, const char ch)
 {
   switch (ch) {
-#define MAP(val, sym) case val: json += sym; break
+#define MAP(val, sym) case val: conn.res_push(sym, 2); break
     MAP('"', "\\\"");
     MAP('\\', "\\\\");
@@ -92 +92 @@
   default:
     if ((unsigned char)ch < 0x20 || ch == 0x7f) {
-      json += "\\u00";
-      json.push_back("0123456789abcdef"[(unsigned char)ch >> 4]);
-      json.push_back("0123456789abcdef"[ch & 0xf]);
+      conn.res_push("\\u00", 3);
+      conn.res_push("0123456789abcdef"[(unsigned char)ch >> 4]);
+      conn.res_push("0123456789abcdef"[ch & 0xf]);
     } else {
-      json.push_back(ch);
+      conn.res_push(ch);
     }
     break;
@@ -103 +103 @@
     
 static void
-escape_n_append(string& json, const char* first, const char* last)
-{
-  json.push_back('"');
+json_escape_n_append(conn_t& conn, const char* first, const char* last)
+{
+  conn.res_push('"');
   for (const char* p = first; p != last; ++p) {
-    escape_n_append(json, *p);
-  }
-  json.push_back('"');
+    json_escape_n_append(conn, *p);
+  }
+  conn.res_push('"');
 }
 
 static void
-escape_n_append(string& json, const char* s)
-{
-  json.push_back('"');
+json_escape_n_append(conn_t& conn, const char* s)
+{
+  conn.res_push('"');
   for (; *s != '\0'; ++s) {
-    escape_n_append(json, *s);
-  }
-  json.push_back('"');
+    json_escape_n_append(conn, *s);
+  }
+  conn.res_push('"');
 }
 
@@ -190 +190 @@
 
 void
-fetcher_t::fetch(string& response)
+fetcher_t::fetch(conn_t& conn)
 {
   // init indexes
@@ -221 +221 @@
                                     HA_READ_KEY_EXACT)
         == 0) {
-      (*ki->response_builder)(*ki, table, response);
+      // prepare header with dummy size
+      conn.res_push("VALUE ", 6);
+      range_string entire_key(ki->entire_key());
+      conn.res_push(entire_key.first_, entire_key.last_ - entire_key.first_);
+      conn.res_push(" 0 4292967296\r\n", sizeof(" 0 4292967296\r\n") - 1);
+      // build body
+      size_t body_from = conn.res_.size_;
+      (*ki->response_builder)(*ki, table, conn);
+      // rewrite size
+      sprintf(conn.res_.buf_ + body_from - 12, "%10u",
+              (unsigned)(conn.res_.size_ - body_from));
+      conn.res_.buf_[body_from - 2] = '\r';
+      // add cr-lf
+      conn.res_push("\r\n", 2);
     }
   }
@@ -247 +260 @@
 
 void
-fetcher_t::build_nlv_response(const key_t& key, TABLE* table, string& response)
+fetcher_t::build_nlv_response(const key_t& key, TABLE* table, conn_t& conn)
 {
   char buf[769], szbuf[16];
   
-  // build name-length-value list
-  string nlv;
   for (Field** field = table->field; *field != NULL; ++field) {
-    nlv += (*field)->field_name;
+    conn.res_push((*field)->field_name, strlen((*field)->field_name));
     if ((*field)->is_null()) {
-      nlv += ":null:";
+      conn.res_push(":null:", 6);
     } else {
       String tmp(buf, sizeof(buf), &my_charset_bin);
@@ -263 +274 @@
       }
       sprintf(szbuf, ":%u:", tmp.length());
-      nlv += szbuf;
-      nlv.insert(nlv.end(), tmp.ptr(), (const char*)tmp.ptr() + tmp.length());
-    }
-  }
-  // build response line
-  response += "VALUE ";
-  range_string entire_key(key.entire_key());
-  response.insert(response.end(), entire_key.first_, entire_key.last_);
-  sprintf(buf, " 0 %u\r\n", (unsigned)nlv.size());
-  response += buf;
-  // append json to response line
-  response += nlv;
-  response += "\r\n";
-}
-  
-void
-fetcher_t::build_json_response(const key_t& key, TABLE* table, string& response)
+      conn.res_push(szbuf, strlen(szbuf));
+      conn.res_push(tmp.ptr(), tmp.length());
+    }
+  }
+}
+
+void
+fetcher_t::build_json_response(const key_t& key, TABLE* table, conn_t& conn)
 {
   char buf[769];
   
   // build json
-  string json("{");
+  conn.res_push('{');
   for (Field** field = table->field; *field != NULL; ++field) {
-    escape_n_append(json, (*field)->field_name);
+    json_escape_n_append(conn, (*field)->field_name);
     if ((*field)->is_null()) {
-      json += ":null";
+      conn.res_push(":null", 5);
     } else {
-      json.push_back(':');
+      conn.res_push(':');
       String tmp(buf, sizeof(buf), &my_charset_utf8_bin);
       if (! (*field)->val_str(&tmp)) {
@@ -299 +301 @@
       case MYSQL_TYPE_LONG: case MYSQL_TYPE_LONGLONG: case MYSQL_TYPE_FLOAT:
       case MYSQL_TYPE_DOUBLE:
-        json.insert(json.end(), (const char*)tmp.ptr(),
-                    (const char*)tmp.ptr() + tmp.length());
+        conn.res_push((const char*)tmp.ptr(), tmp.length());
         break;
       default:
-        escape_n_append(json, (const char*)tmp.ptr(),
-                        (const char*)tmp.ptr() + tmp.length());
+        json_escape_n_append(conn, (const char*)tmp.ptr(),
+                             (const char*)tmp.ptr() + tmp.length());
         break;
       }
     }
-    json.push_back(',');
-  }
-  json.end()[-1] = '}';
-  // build response line
-  response += "VALUE ";
-  range_string entire_key(key.entire_key());
-  response.insert(response.end(), entire_key.first_, entire_key.last_);
-  sprintf(buf, " 0 %u\r\n", (unsigned)json.size());
-  response += buf;
-  // append json to response line
-  response += json;
-  response += "\r\n";
+    conn.res_push(',');
+  }
+  conn.res_.buf_[conn.res_.size_ - 1] = '}';
 }
 
@@ -639 +631 @@
         EXPECT('\n');
         goto PARSE_COMPLETE;
-      } else if (*req == '\r') {
+      } else if (*req == '\n') {
         ++req;
         goto PARSE_COMPLETE;
@@ -663 +655 @@
     if (*req == ':') {
       ++req;
+      CHECK_EOF();
+      switch (*req) {
+      case 'j':
+        ++req;
+        EXPECT('s');
+        EXPECT('o');
+        EXPECT('n');
+        keys.back().response_builder = &fetcher_t::build_json_response;
+        break;
 #if MSGPACK
-      if (*req == 'm') {
+      case 'm':
         ++req;
         EXPECT('s');
@@ -673 +674 @@
         EXPECT('k');
         keys.back().response_builder = &fetcher_t::build_msgpack_response;
-      } else {
+        break;
 #endif
-      EXPECT('j');
-      EXPECT('s');
-      EXPECT('o');
-      EXPECT('n');
-      keys.back().response_builder = &fetcher_t::build_json_response;
-#if MSGPACK
-      }
-#endif
+      default:
+        return 0;
+      }
     } else {
       keys.back().response_builder = &fetcher_t::build_nlv_response;
@@ -708 +704 @@
 conn_t::build_send_response(const vector<key_t>& keys)
 {
-  assert(res_buf_.empty());
+  assert(res_.buf_ == NULL);
+  
+  res_prepare();
   
   if (! keys.empty()) {
     // fetch data from db
-    fetcher_t(thd_->thd_, keys).fetch(res_buf_);
-  }
-  res_buf_ += "END\r\n";
-  res_off_ = 0;
+    fetcher_t(thd_->thd_, keys).fetch(*this);
+  }
+  res_push("END\r\n", 5);
   
   // try to send
-  ssize_t r = write(sock_, res_buf_.c_str(), res_buf_.size());
+  ssize_t r = write(sock_, res_.buf_, res_.size_);
   if (IO_IS_FATAL(r)) {
     return false;
   } else if (r > 0) {
-    if ((size_t)r == res_buf_.size()) {
-      res_buf_.clear();
+    if ((size_t)r == res_.size_) {
+      res_clear();
     } else {
-      res_off_ = r;
+      res_.off_ = r;
     }
   }
@@ -756 +753 @@
     }
     req_off_ -= req_size;
-    if (! res_buf_.empty()) {
+    if (res_.buf_ != NULL) {
       return true;
     }
@@ -768 +765 @@
   picoev_set_timeout(loop, sock_, TIMEOUT_SECS);
 
-  if (! res_buf_.empty()) {
+  if (res_.buf_ != NULL) {
     // write response
-    ssize_t r
-      = write(sock_, res_buf_.c_str() + res_off_, res_buf_.size() - res_off_);
+    ssize_t r = write(sock_, res_.buf_ + res_.off_, res_.size_ - res_.off_);
     if (r <= 0) {
       if (IO_IS_FATAL(r)) {
@@ -778 +774 @@
       return;
     }
-    res_off_ += r;
-    if (res_off_ != res_buf_.size()) {
+    res_.off_ += r;
+    if (res_.off_ != res_.size_) {
       return;
     }
-    res_buf_.clear();
-    res_off_ = 0;
+    res_clear();
     // flows down
   }
@@ -792 +787 @@
       goto CLOSE_CONN;
     }
-    if (res_buf_.empty()) {
+    if (res_.buf_ == NULL) {
       if (req_buf_ == NULL) {
         if ((req_buf_ = thd_->steal_buf()) == NULL) {
@@ -827 +822 @@
   
   // update state and return
-  if (req_buf_ != NULL && req_off_ == 0 && res_buf_.empty()) {
+  if (req_buf_ != NULL && req_off_ == 0) {
     thd_->return_buf(req_buf_);
     req_buf_ = NULL;
   }
-  picoev_set_events(loop, sock_, res_buf_.empty() ? PICOEV_READ : PICOEV_WRITE);
+  picoev_set_events(loop, sock_,
+                    res_.buf_ == NULL ? PICOEV_READ : PICOEV_WRITE);
   return;
   

platform/mysql/mycached/trunk/mycached.hh

@@ -48 +48 @@
     error_t(const std::string& what) : std::domain_error(what) {}
   };
+  
+  struct packet_too_long_t : public error_t {
+    packet_too_long_t() : error_t("packet too long") {}
+  };
+  
+  class conn_t;
   
   struct range_string {
@@ -79 +85 @@
     }
   };
-
+  
   inline
   bool
@@ -116 +122 @@
     uchar key_value_[8];
     void (*response_builder)(const key_t& key, TABLE* table,
-                             std::string& response);
+                             conn_t& conn);
     range_string entire_key() const {
       return range_string(db_.first_, key_str_.last_);
@@ -134 +140 @@
     fetcher_t(THD* thd, const std::vector<key_t>& keys);
     ~fetcher_t();
-    void fetch(std::string& response);
+    void fetch(conn_t& conn);
     void register_table(const range_string& db, const range_string& table);
     TABLE_LIST* get_table(const range_string& db, const range_string& table,
@@ -152 +158 @@
     }
     static void build_nlv_response(const key_t& key, TABLE* table,
-                                   std::string& response);
+                                   conn_t& conn);
     static void build_json_response(const key_t& key, TABLE* table,
-                                    std::string& response);
+                                    conn_t& conn);
 #ifdef MSGPACK
     static void build_msgpack_response(const key_t& key, TABLE* table,
-                                    std::string& response);
+                                       conn_t& conn);
 #endif
   };
@@ -186 +192 @@
     thread_t* thd_;
     int sock_;
-    size_t res_off_;
-    std::string res_buf_;
     size_t req_off_;
     char* req_buf_;
+    struct {
+      char* buf_;
+      size_t off_, size_;
+    } res_;
     conn_t(thread_t* thd, int sock)
-      : thd_(thd), sock_(sock), res_off_(0), res_buf_(), req_off_(0),
-        req_buf_(NULL) {
+      : thd_(thd), sock_(sock), req_off_(0), req_buf_(NULL) {
+      res_.buf_ = NULL;
+      res_.off_ = res_.size_ = 0;
       picoev_add(thd_->loop_, sock_, PICOEV_READ, TIMEOUT_SECS, handle_conn,
                  this);
@@ -200 +209 @@
         thd_->return_buf(req_buf_);
       }
+      if (res_.buf_ != NULL) {
+        thd_->return_buf(res_.buf_);
+      }
       picoev_del(thd_->loop_, sock_);
       close(sock_);
+    }
+    void res_prepare() {
+      assert(res_.buf_ == NULL);
+      res_.buf_ = thd_->steal_buf();
+      res_.off_ = res_.size_ = 0;
+    }
+    void res_clear() {
+      assert(res_.buf_ != NULL);
+      thd_->return_buf(res_.buf_);
+      res_.buf_ = NULL;
+      res_.off_ = res_.size_ = 0;
+    }
+    void res_push(int ch) {
+      if (res_.size_ + 1 > MAX_PACKET_SIZE) {
+        throw packet_too_long_t();
+      }
+      res_.buf_[res_.size_++] = ch;
+    }
+    void res_push(const char* p, size_t s) {
+      if (res_.size_ + s > MAX_PACKET_SIZE) {
+        throw packet_too_long_t();
+      }
+      memcpy(res_.buf_ + res_.size_, p, s);
+      res_.size_ += s;
     }
     // all cleanups done in handle_conn

platform/mysql/mycached/trunk/testclient.cc

@@ -109 +109 @@
     explen += 8;
   }
-  if (len != (ssize_t)explen) {
+  if (len < (ssize_t)explen) {
     fprintf(stderr, "unexpected response for test.t1.%d, got:\n", fd);
     fwrite(buf, 1, len, stderr);