Rework http client and server code to use a form of socket read/write

calls which can be canceled without trashing the stack.  Also minor
changes to the code for parsing http headers to avoid buffer
overflows.

src/network/http/http_connection.cpp

@@ -16,47 +16,54 @@ class fc::http::connection::impl
   public:
    fc::tcp_socket sock;
    fc::ip::endpoint ep;
-   impl() {
+   impl() {}
+
+   size_t read_until(std::shared_ptr<char> buffer, size_t buffer_length, char c = '\n') 
+   {
+     size_t offset = 0;
+     while (offset < buffer_length && 
+         sock.readsome(buffer, 1, offset) == 1) 
+     {
+       if (buffer.get()[offset] == c)
+       {
+         buffer.get()[offset] = 0;
+         return offset;
+       }
+       ++offset;
+     }
+     return offset;
    }
 
-   int read_until( char* buffer, char* end, char c = '\n' ) {
+   fc::http::reply parse_reply() 
-      char* p = buffer;
+   {
-     // try {
-          while( p < end && 1 == sock.readsome(p,1) ) {
-            if( *p == c ) {
-              *p = '\0';
-              return (p - buffer)-1;
-            }
-            ++p;
-          }
-     // } catch ( ... ) {
-     //   elog("%s", fc::current_exception().diagnostic_information().c_str() );
-        //elog( "%s", fc::except_str().c_str() );
-     // }
-      return (p-buffer);
-   }
-
-   fc::http::reply parse_reply() {
       fc::http::reply rep;
-      try {
+      fc::oexception parsing_exception;
-        std::vector<char> line(1024*8);
+      try 
-        int s = read_until( line.data(), line.data()+line.size(), ' ' ); // HTTP/1.1
+      {
-        s = read_until( line.data(), line.data()+line.size(), ' ' ); // CODE
+        const size_t buffer_length = 1024 * 8;
-        rep.status = static_cast<int>(to_int64(fc::string(line.data())));
+        std::shared_ptr<char> line(new char[buffer_length], [](char* p){ delete[] p; });
-        s = read_until( line.data(), line.data()+line.size(), '\n' ); // DESCRIPTION
+        read_until(line, buffer_length, ' '); // HTTP/1.1
+        size_t bytes_read = read_until(line, buffer_length, ' '); // CODE
+        rep.status = static_cast<int>(to_int64(fc::string(line.get(), bytes_read)));
+        read_until(line, buffer_length, '\n'); // DESCRIPTION
         
         fc::optional<size_t> content_length;
         bool is_chunked = false;
-        while( (s = read_until( line.data(), line.data()+line.size(), '\n' )) > 1 ) {
+        while( (bytes_read = read_until(line, buffer_length, '\n')) > 1 ) 
+        {
           fc::http::header h;
-          char* end = line.data();
+          std::string line_string(line.get(), bytes_read);
-          while( *end != ':' )++end;
+          size_t colon_pos = line_string.find(": ");
-          h.key = fc::string(line.data(),end);
+          if (colon_pos != std::string::npos)
-          ++end; // skip ':'
+          {
-          ++end; // skip space
+            h.key = line_string.substr(0, colon_pos);
-          char* skey = end;
+            size_t value_start_pos = colon_pos + 2;
-          while( *end != '\r' ) ++end;
+            size_t carriage_return_pos = line_string.find('\r', value_start_pos);
-          h.val = fc::string(skey,end);
+            if (carriage_return_pos != std::string::npos)
+              h.val = line_string.substr(value_start_pos, carriage_return_pos - value_start_pos);
+            else
+              h.val = line_string.substr(value_start_pos);
+          }
           rep.headers.push_back(h);
           if( boost::iequals(h.key, "Content-Length") )
             content_length = static_cast<size_t>(to_uint64( fc::string(h.val) ));
@@ -70,19 +77,20 @@ class fc::http::connection::impl
           do
           {
             // Chunked means we get a hexadecimal number of bytes on a line, followed by the content
-            s = read_until( line.data(), line.data()+line.size(), '\n' ); //read chunk length 
+            bytes_read = read_until(line, buffer_length, '\n'); //read chunk length 
-            if (line[strlen(line.data())] == '\r')
+            std::string line_string(line.get(), bytes_read);
-              line[strlen(line.data())] = 0;
+            if (line_string.size() > 0 && line_string[line_string.size() - 1] == '\r')
+              line_string.erase(line_string.size() - 1);
             unsigned length;
-            if (sscanf(line.data(), "%x", &length) != 1)
+            if (sscanf(line_string.c_str(), "%x", &length) != 1)
-              FC_THROW("Invalid content length: ${length}", ("length", fc::string(line.data())));
+              FC_THROW("Invalid content length: ${length}", ("length", line_string));
             content_length = length;
             if (*content_length)
             {
-              std::vector<char> temp_data(*content_length);
+              std::shared_ptr<char> temp_data(new char[*content_length], [](char* p){ delete[] p; });
-              sock.read( temp_data.data(), *content_length );
+              sock.read(temp_data, *content_length, 0);
-              boost::push_back(rep.body, temp_data);
+              boost::push_back(rep.body, std::make_pair(temp_data.get(), temp_data.get() + *content_length));
-              read_until( line.data(), line.data()+line.size(), '\n' ); //discard cr/lf after each chunk 
+              read_until(line, buffer_length, '\n'); //discard cr/lf after each chunk 
             }
           }
           while (*content_length != 0);
@@ -92,19 +100,23 @@ class fc::http::connection::impl
         {
           if (*content_length)
           {
-            rep.body.resize(*content_length);
+            std::shared_ptr<char> temp_data(new char[*content_length], [](char* p){ delete[] p; });
-            sock.read( rep.body.data(), *content_length );
+            sock.read(temp_data, *content_length, 0);
+            boost::push_back(rep.body, std::make_pair(temp_data.get(), temp_data.get() + *content_length));
           }
         }
         else //just read until closed if no content length or chunking
         {
-          std::shared_ptr<char> buf(new char);
           while (true)
           {
             try
             {
-              sock.read(buf, sizeof(buf), 0);
+              sock.read(line, 1, 0);
-              rep.body.push_back(*buf);
+              rep.body.push_back(line.get()[0]);
+            }
+            catch (const fc::canceled_exception&)
+            {
+              throw;
             }
             catch (const fc::eof_exception&)
             {
@@ -114,26 +126,36 @@ class fc::http::connection::impl
         }
 
         return rep;
-      } catch ( fc::exception& e ) {
+      } 
-        elog( "${exception}", ("exception",e.to_detail_string() ) );
+      catch (const fc::canceled_exception&)
+      {
+        throw;
+      }
+      catch (const fc::exception& e) 
+      {
+        parsing_exception = e;
+      } 
+      assert(parsing_exception); // the only way we get here is if the last catch falls through
+      elog("${exception}", ("exception", parsing_exception->to_detail_string()));
       sock.close();
       rep.status = http::reply::InternalServerError;
       return rep;
    }
-   }
 };
 
 
 
 namespace fc { namespace http {
 
-         connection::connection()
+connection::connection() :
-         :my( new connection::impl() ){}
+  my( new connection::impl() )
-         connection::~connection(){}
+{}
 
+connection::~connection(){}
 
 // used for clients
-void       connection::connect_to( const fc::ip::endpoint& ep ) {
+void connection::connect_to( const fc::ip::endpoint& ep ) 
+{
   my->sock.close();
   my->sock.connect_to( my->ep = ep );
 }
@@ -142,43 +164,59 @@ http::reply connection::request( const fc::string& method,
                                  const fc::string& url, 
                                  const fc::string& body, 
                                  const headers& he,
-                                 const fc::string& content_type ) {
+                                 const fc::string& content_type ) 
-	
+{
   fc::url parsed_url(url);
-  if( !my->sock.is_open() ) {
+  if( !my->sock.is_open() ) 
+  {
     wlog( "Re-open socket!" );
     my->sock.connect_to( my->ep );
   }
-  try {
+  try 
+  {
     fc::stringstream req;
     req << method << " " << parsed_url.path()->generic_string() << parsed_url.args_as_string() << " HTTP/1.1\r\n";
-      req << "Host: "<<*parsed_url.host()<<"\r\n";
+    req << "Host: " << *parsed_url.host() << "\r\n";
     req << "Content-Type: " << content_type << "\r\n";
     for( auto i = he.begin(); i != he.end(); ++i )
-      {
+      req << i->key << ": " << i->val << "\r\n";
-          req << i->key <<": " << i->val<<"\r\n";
+    if( body.size() ) 
-      }
+      req << "Content-Length: "<< body.size() << "\r\n";
-      if( body.size() ) req << "Content-Length: "<< body.size() << "\r\n";
     req << "\r\n"; 
-      fc::string head = req.str();
 
-      my->sock.write( head.c_str(), head.size() );
+    {
+      fc::string head = req.str();
+      std::shared_ptr<char> write_buffer(new char[head.size()], [](char* p){ delete[] p; });
+      std::copy(head.begin(), head.end(), write_buffer.get());
+      my->sock.write(write_buffer, head.size(), 0);
       //elog("Sending header ${head}", ("head", head));
       //  fc::cerr.write( head.c_str() );
+    }
 
-      if( body.size() )  {
+    if( body.size() )  
-          my->sock.write( body.c_str(), body.size() );
+    {
+      std::shared_ptr<char> write_buffer(new char[body.size()], [](char* p){ delete[] p; });
+      std::copy(body.begin(), body.end(), write_buffer.get());
+      my->sock.write(write_buffer, body.size(), 0);
       //elog("Sending body ${body}", ("body", body));
-    //      fc::cerr.write( body.c_str() );
+      //fc::cerr.write( body.c_str() );
     }
     // fc::cerr.flush();
 
     return my->parse_reply();
-  } catch ( ... ) {
+  } 
+  catch (const fc::canceled_exception&)
+  {
+    throw;
+  }
+  catch (...) 
+  {
+    // fall through
+  }
+  // the only way we get here is if we encountered catch(...)
   my->sock.close();
   FC_THROW_EXCEPTION( exception, "Error Sending HTTP Request" ); // TODO: provide more info
   // return http::reply( http::reply::InternalServerError ); // TODO: replace with connection error
-  }
 }
 
 // used for servers
@@ -188,71 +226,95 @@ fc::tcp_socket& connection::get_socket()const {
 
 http::request connection::read_request()const {
   http::request req;
-  std::vector<char> line(1024*8);
+  const size_t buffer_length = 1024 * 8;
-  int s = my->read_until( line.data(), line.data()+line.size(), ' ' ); // METHOD
+  std::shared_ptr<char> line(new char[buffer_length], [](char* p){ delete[] p; });
-  req.method = line.data();
+  size_t bytes_read = my->read_until(line, buffer_length, ' '); // METHOD
-  s = my->read_until( line.data(), line.data()+line.size(), ' ' ); // PATH
+  req.method = std::string(line.get(), bytes_read);
-  req.path = line.data();
+  bytes_read = my->read_until(line, buffer_length, ' '); // PATH
-  s = my->read_until( line.data(), line.data()+line.size(), '\n' ); // HTTP/1.0
+  req.path = std::string(line.get(), bytes_read);
+  bytes_read = my->read_until(line, buffer_length, '\n'); // HTTP/1.0
   
-  while( (s = my->read_until( line.data(), line.data()+line.size(), '\n' )) > 1 ) {
+  while( (bytes_read = my->read_until(line, buffer_length, '\n')) > 1 ) 
+  {
     fc::http::header h;
-    char* end = line.data();
+    std::string line_string(line.get(), bytes_read);
-    while( *end != ':' )++end;
+    size_t colon_pos = line_string.find(": ");
-    h.key = fc::string(line.data(),end);
+    if (colon_pos != std::string::npos)
-    ++end; // skip ':'
+    {
-    ++end; // skip space
+      h.key = line_string.substr(0, colon_pos);
-    char* skey = end;
+      size_t value_start_pos = colon_pos + 2;
-    while( *end != '\r' ) ++end;
+      size_t carriage_return_pos = line_string.find('\r', value_start_pos);
-    h.val = fc::string(skey,end);
+      if (carriage_return_pos != std::string::npos)
+        h.val = line_string.substr(value_start_pos, carriage_return_pos - value_start_pos);
+      else
+        h.val = line_string.substr(value_start_pos);
+    }
     req.headers.push_back(h);
-    if( boost::iequals(h.key, "Content-Length")) {
+    if( boost::iequals(h.key, "Content-Length")) 
-       auto s = static_cast<size_t>(to_uint64( fc::string(h.val) ) );
+    {
-       FC_ASSERT( s < 1024*1024 );
+      size_t content_length = static_cast<size_t>(to_uint64( fc::string(h.val) ) );
+      FC_ASSERT(content_length < 1024*1024);
       req.body.resize( static_cast<size_t>(to_uint64( fc::string(h.val) ) ));
     }
-    if( boost::iequals(h.key, "Host") ) {
+    if( boost::iequals(h.key, "Host") )
       req.domain = h.val;
   }
-  }
   // TODO: some common servers won't give a Content-Length, they'll use 
   // Transfer-Encoding: chunked.  handle that here.
 
-  if( req.body.size() ) {
+  if( req.body.size() )
-    my->sock.read( req.body.data(), req.body.size() );
+  {
+    std::shared_ptr<char> body_buffer(new char[req.body.size()], [](char* p){ delete[] p; });
+    my->sock.read(body_buffer, req.body.size(), 0);
+    std::copy(body_buffer.get(), body_buffer.get() + req.body.size(), req.body.data());
   }
+
   return req;
 }
 
-fc::string request::get_header( const fc::string& key )const {
+fc::string request::get_header( const fc::string& key )const 
-  for( auto itr = headers.begin(); itr != headers.end(); ++itr ) {
+{
-    if( boost::iequals(itr->key, key) ) { return itr->val; } 
+  for( auto itr = headers.begin(); itr != headers.end(); ++itr )
-  }
+    if( boost::iequals(itr->key, key) )
+      return itr->val;
   return fc::string();
 }
-std::vector<header> parse_urlencoded_params( const fc::string& f ) {
+
+std::vector<header> parse_urlencoded_params( const fc::string& f ) 
+{
   int num_args = 0;
-  for( size_t i = 0; i < f.size(); ++i ) {
+  for( size_t i = 0; i < f.size(); ++i )
-    if( f[i] == '=' ) ++num_args;
+    if( f[i] == '=' ) 
-  }
+      ++num_args;
+  
   std::vector<header> h(num_args);
   int arg = 0;
-  for( size_t i = 0; i < f.size(); ++i ) {
+  for( size_t i = 0; i < f.size(); ++i )
-    while( f[i] != '=' && i < f.size() ) {
+  {
-      if( f[i] == '%' ) { 
+    while( f[i] != '=' && i < f.size() ) 
+    {
+      if( f[i] == '%' ) 
+      {
         h[arg].key += char((fc::from_hex(f[i+1]) << 4) | fc::from_hex(f[i+2]));
         i += 3;
-      } else {
+      }
+      else 
+      {
         h[arg].key += f[i];
         ++i;
       }
     }
     ++i;
-    while( i < f.size() && f[i] != '&' ) {
+
-      if( f[i] == '%' ) { 
+    while( i < f.size() && f[i] != '&' ) 
+    {
+      if( f[i] == '%' ) 
+      { 
         h[arg].val += char((fc::from_hex(f[i+1]) << 4) | fc::from_hex(f[i+2]));
         i += 3;
-      } else {
+      } 
+      else
+      {
         h[arg].val += f[i] == '+' ? ' ' : f[i];
         ++i;
       }

src/network/http/http_server.cpp

@@ -16,24 +16,38 @@ namespace fc { namespace http {
       :body_bytes_sent(0),body_length(0),con(c),handle_next_req(cont)
       {}
 
-      void send_header() {
+      void send_header() 
+      {
          //ilog( "sending header..." );
          fc::stringstream ss;
          ss << "HTTP/1.1 " << rep.status << " ";
-         switch( rep.status ) {
+         switch( rep.status ) 
-            case fc::http::reply::OK: ss << "OK\r\n"; break;
+         {
-            case fc::http::reply::RecordCreated: ss << "Record Created\r\n"; break;
+            case fc::http::reply::OK: 
-            case fc::http::reply::NotFound: ss << "Not Found\r\n"; break;
+              ss << "OK\r\n"; 
-            case fc::http::reply::Found: ss << "Found\r\n"; break;
+              break;
-            default: ss << "Internal Server Error\r\n"; break;
+            case fc::http::reply::RecordCreated: 
+              ss << "Record Created\r\n"; 
+              break;
+            case fc::http::reply::NotFound: 
+              ss << "Not Found\r\n"; 
+              break;
+            case fc::http::reply::Found: 
+              ss << "Found\r\n"; 
+              break;
+            default: 
+              ss << "Internal Server Error\r\n"; 
+              break;
          }
-         for( uint32_t i = 0; i < rep.headers.size(); ++i ) {
+         for( uint32_t i = 0; i < rep.headers.size(); ++i ) 
-            ss << rep.headers[i].key <<": "<<rep.headers[i].val <<"\r\n";
+            ss << rep.headers[i].key << ": " << rep.headers[i].val << "\r\n";
-         }
+         ss << "Content-Length: " << body_length << "\r\n\r\n";
-         ss << "Content-Length: "<<body_length<<"\r\n\r\n";
+         std::string s = ss.str();
-         auto s = ss.str();
+         std::shared_ptr<char> write_buffer(new char[s.size()], [](char* p){ delete[] p; });
+         std::copy(s.begin(), s.end(), write_buffer.get());
+         
          //fc::cerr<<s<<"\n";
-         con->get_socket().write( s.c_str(), s.size() );
+         con->get_socket().write(write_buffer, s.size(), 0);
       }
 
       http::reply           rep;
@@ -184,7 +198,9 @@ namespace fc { namespace http {
       my->send_header();
     }
     my->body_bytes_sent += len;
-    my->con->get_socket().write( data, static_cast<size_t>(len) ); 
+    std::shared_ptr<char> write_buffer(new char[len], [](char* p){ delete[] p; });
+    std::copy(data, data + len, write_buffer.get());
+    my->con->get_socket().write(write_buffer, static_cast<size_t>(len), 0); 
     if( my->body_bytes_sent == int64_t(my->body_length) ) {
       if( false || my->handle_next_req ) {
         ilog( "handle next request..." );