Merge branch 'master' of https://github.com/oktal/pistache

.travis.yml

 language: cpp
 
-dist: trusty
+dist: bionic
 sudo: true
 
 branches:
   only:
     - master
 
+arch:
+  - amd64
+  - ppc64le
+  - s390x
+  - arm64
+
 matrix:
   include:
   # Linux clang builds
@@ -19,7 +25,7 @@ matrix:
     addons:
       apt:
         sources: ['ubuntu-toolchain-r-test', 'llvm-toolchain-trusty-4.0']
-        packages: [ 'cmake', 'clang-4.0', 'llvm-4.0-tools', 'libstdc++-6-dev', 'libssl-dev', 'libcurl4-openssl-dev', 'gdb', 'lcov', 'cppcheck' ]
+        packages: [ 'apport', 'cmake', 'clang-4.0', 'llvm-4.0-tools', 'libstdc++-6-dev', 'libssl-dev', 'libcurl4-openssl-dev', 'gdb', 'lcov', 'cppcheck' ]
 
   - os: linux
     compiler: clang
@@ -30,7 +36,7 @@ matrix:
     addons:
       apt:
         sources: ['ubuntu-toolchain-r-test', 'llvm-toolchain-trusty-5.0']
-        packages: [ 'cmake', 'clang-5.0', 'llvm-5.0-tools', 'libstdc++-6-dev', 'libssl-dev', 'libcurl4-openssl-dev', 'gdb', 'lcov', 'cppcheck' ]
+        packages: [ 'apport', 'cmake', 'clang-5.0', 'llvm-5.0-tools', 'libstdc++-6-dev', 'libssl-dev', 'libcurl4-openssl-dev', 'gdb', 'lcov', 'cppcheck' ]
 
   - os: linux
     compiler: clang
@@ -41,7 +47,7 @@ matrix:
     addons:
       apt:
         sources: ['ubuntu-toolchain-r-test', 'llvm-toolchain-trusty-6.0']
-        packages: [ 'cmake', 'clang-6.0', 'llvm-6.0-tools', 'libstdc++-6-dev', 'libssl-dev', 'libcurl4-openssl-dev', 'gdb', 'lcov', 'cppcheck' ]
+        packages: [ 'apport', 'cmake', 'clang-6.0', 'llvm-6.0-tools', 'libstdc++-6-dev', 'libssl-dev', 'libcurl4-openssl-dev', 'gdb', 'lcov', 'cppcheck' ]
 
   # Linux GCC builds
   - os: linux
@@ -53,7 +59,7 @@ matrix:
     addons:
       apt:
         sources: ['ubuntu-toolchain-r-test']
-        packages: ['g++-7', 'libssl-dev', 'libcurl4-openssl-dev', 'gdb', 'lcov', 'cppcheck']
+        packages: ['apport', 'g++-7', 'libssl-dev', 'libcurl4-openssl-dev', 'gdb', 'lcov', 'cppcheck']
 
   - os: linux
     compiler: gcc
@@ -64,7 +70,7 @@ matrix:
     addons:
       apt:
         sources: ['ubuntu-toolchain-r-test']
-        packages: ['g++-8', 'libssl-dev', 'libcurl4-openssl-dev', 'gdb', 'lcov', 'cppcheck']
+        packages: ['apport', 'g++-8', 'libssl-dev', 'libcurl4-openssl-dev', 'gdb', 'lcov', 'cppcheck']
 
 install:
   - DEPS_DIR="${TRAVIS_BUILD_DIR}/deps"
@@ -81,7 +87,7 @@ before_script:
 
   # Print debug system information
   - cat /proc/sys/kernel/core_pattern
-  - cat /etc/default/apport
+  - cat /etc/default/apport || true
   - service --status-all || true
   - initctl list || true
 
@@ -93,6 +99,14 @@ before_script:
     -DPISTACHE_BUILD_TESTS=true
     -DPISTACHE_USE_SSL=true
 
+  # Debug build, no SSL
+  - cmake -H.
+    -BBuild-Debug-nossl
+    -DCMAKE_BUILD_TYPE=Debug
+    -DPISTACHE_BUILD_EXAMPLES=true
+    -DPISTACHE_BUILD_TESTS=true
+    -DPISTACHE_USE_SSL=false
+
   # Release build
   - cmake -H.
     -BBuild-Release
@@ -104,6 +118,10 @@ script:
   - cd Build-Debug
   - make -j 2 all test ARGS="-V"
 
+  # Debug build, no SSL
+  - cd ../Build-Debug-nossl
+  - make -j 2 all test ARGS="-V"
+
   # Release build
   - cd ../Build-Release
   - make -j 2

include/pistache/http.h

@@ -220,8 +220,6 @@ public:
     other.timerFd = -1;
   }
 
-  ~Timeout() { disarm(); }
-
   Timeout &operator=(Timeout &&other) {
     handler = other.handler;
     transport = other.transport;
@@ -233,6 +231,8 @@ public:
     return *this;
   }
 
+  ~Timeout();
+
   template <typename Duration> void arm(Duration duration) {
     Async::Promise<uint64_t> p([=](Async::Deferred<uint64_t> deferred) {
       timerFd = TRY_RET(timerfd_create(CLOCK_MONOTONIC, TFD_NONBLOCK));
@@ -250,25 +250,15 @@ public:
     armed = true;
   }
 
-  void disarm() {
-    if (armed) {
-      transport->disarmTimer(timerFd);
-    }
-  }
+  void disarm();
 
-  bool isArmed() const { return armed; }
+  bool isArmed() const;
 
 private:
-  Timeout(const Timeout &other)
-      : handler(other.handler), request(other.request),
-        transport(other.transport), armed(other.armed), timerFd(other.timerFd),
-        peer() {}
-
-  Timeout(Tcp::Transport *transport_, Handler *handler_, Request request_)
-      : handler(handler_), request(std::move(request_)), transport(transport_),
-        armed(false), timerFd(-1), peer() {}
+  Timeout(const Timeout &other) = default;
 
-  template <typename Ptr> void associatePeer(const Ptr &ptr) { peer = ptr; }
+  Timeout(Tcp::Transport *transport_, Handler *handler_, Request request_,
+          std::weak_ptr<Tcp::Peer> peer_);
 
   void onTimeout(uint64_t numWakeup);
 
@@ -371,7 +361,7 @@ public:
   // have to define it ourself
   ResponseWriter(ResponseWriter &&other);
 
-  ResponseWriter &operator=(ResponseWriter &&other);
+  ResponseWriter &operator=(ResponseWriter &&other) = default;
 
   void setMime(const Mime::MediaType &mime);
 
@@ -412,6 +402,13 @@ public:
 
   std::shared_ptr<Tcp::Peer> peer() const;
 
+  // Returns total count of HTTP bytes (headers, cookies, body) written when
+  // sending the response.  Result valid AFTER ResponseWriter.send() is called.
+  ssize_t getResponseSize() const { return sent_bytes_; }
+
+  // Returns HTTP result code that was sent with the response.
+  Code getResponseCode() const { return response_.code(); }
+
   // Unsafe API
 
   DynamicStreamBuf *rdbuf();
@@ -421,7 +418,8 @@ public:
   ResponseWriter clone() const;
 
 private:
-  ResponseWriter(Tcp::Transport *transport, Request request, Handler *handler);
+  ResponseWriter(Tcp::Transport *transport, Request request, Handler *handler,
+                 std::weak_ptr<Tcp::Peer> peer);
 
   ResponseWriter(const ResponseWriter &other);
 
@@ -429,14 +427,6 @@ private:
                                    const size_t size,
                                    const Mime::MediaType &mime);
 
-  template <typename Ptr> void associatePeer(const Ptr &peer) {
-    if (peer_.use_count() > 0)
-      throw std::runtime_error("A peer was already associated to the response");
-
-    peer_ = peer;
-    timeout_.associatePeer(peer_);
-  }
-
   Async::Promise<ssize_t> putOnWire(const char *data, size_t len);
 
   Response response_;
@@ -444,6 +434,7 @@ private:
   DynamicStreamBuf buf_;
   Tcp::Transport *transport_;
   Timeout timeout_;
+  ssize_t sent_bytes_;
 };
 
 Async::Promise<ssize_t>

include/pistache/route_bind.h

@@ -40,13 +40,18 @@ template <typename Result, typename Cls, typename... Args, typename Obj>
 Route::Handler bind(Result (Cls::*func)(Args...), Obj obj) {
   details::static_checks<Args...>();
 
-#define CALL_MEMBER_FN(obj, pmf) ((obj)->*(pmf))
-
   return [=](const Rest::Request &request, Http::ResponseWriter response) {
-    CALL_MEMBER_FN(obj, func)(request, std::move(response));
+    (obj->*func)(request, std::move(response));
   };
+}
 
-#undef CALL_MEMBER_FN
+template <typename Result, typename Cls, typename... Args, typename Obj>
+Route::Handler bind(Result (Cls::*func)(Args...), std::shared_ptr<Obj> objPtr) {
+  details::static_checks<Args...>();
+
+  return [=](const Rest::Request &request, Http::ResponseWriter response) {
+    (objPtr.get()->*func)(request, std::move(response));
+  };
 }
 
 template <typename Result, typename... Args>

include/pistache/router.h

@@ -241,6 +241,9 @@ namespace Private {
 
 class RouterHandler : public Http::Handler {
 public:
+
+  HTTP_PROTOTYPE(RouterHandler)
+
   /**
    * Used for immutable router. Useful if all the routes are
    * defined at compile time (and for backward compatibility)
@@ -259,10 +262,6 @@ public:
                  Http::ResponseWriter response) override;
 
 private:
-  std::shared_ptr<Tcp::Handler> clone() const final {
-    return std::make_shared<RouterHandler>(router);
-  }
-
   std::shared_ptr<Rest::Router> router;
 };
 } // namespace Private
@@ -326,15 +325,22 @@ template <typename Result, typename Cls, typename... Args, typename Obj>
 Route::Handler bind(Result (Cls::*func)(Args...), Obj obj) {
   details::static_checks<Args...>();
 
-#define CALL_MEMBER_FN(obj, pmf) ((obj)->*(pmf))
-
   return [=](const Rest::Request &request, Http::ResponseWriter response) {
-    CALL_MEMBER_FN(obj, func)(request, std::move(response));
+    (obj->*func)(request, std::move(response));
 
     return Route::Result::Ok;
   };
+}
 
-#undef CALL_MEMBER_FN
+template <typename Result, typename Cls, typename... Args, typename Obj>
+Route::Handler bind(Result (Cls::*func)(Args...), std::shared_ptr<Obj> objPtr) {
+  details::static_checks<Args...>();
+
+  return [=](const Rest::Request &request, Http::ResponseWriter response) {
+    (objPtr.get()->*func)(request, std::move(response));
+
+    return Route::Result::Ok;
+  };
 }
 
 template <typename Result, typename... Args>

src/common/http.cc

@@ -653,25 +653,17 @@ void ResponseStream::ends() {
 ResponseWriter::ResponseWriter(ResponseWriter &&other)
     : response_(std::move(other.response_)), peer_(other.peer_),
       buf_(std::move(other.buf_)), transport_(other.transport_),
-      timeout_(std::move(other.timeout_)) {}
+      timeout_(std::move(other.timeout_)), sent_bytes_(0) {}
 
 ResponseWriter::ResponseWriter(Tcp::Transport *transport, Request request,
-                               Handler *handler)
-    : response_(request.version()), peer_(), buf_(DefaultStreamSize),
-      transport_(transport), timeout_(transport, handler, std::move(request)) {}
+                               Handler *handler, std::weak_ptr<Tcp::Peer> peer)
+    : response_(request.version()), peer_(peer), buf_(DefaultStreamSize),
+      transport_(transport),
+      timeout_(transport, handler, std::move(request), peer), sent_bytes_(0) {}
 
 ResponseWriter::ResponseWriter(const ResponseWriter &other)
     : response_(other.response_), peer_(other.peer_), buf_(DefaultStreamSize),
-      transport_(other.transport_), timeout_(other.timeout_) {}
-
-ResponseWriter &ResponseWriter::operator=(ResponseWriter &&other) {
-  response_ = std::move(other.response_);
-  peer_ = std::move(other.peer_);
-  transport_ = other.transport_;
-  buf_ = std::move(other.buf_);
-  timeout_ = std::move(other.timeout_);
-  return *this;
-}
+      transport_(other.transport_), timeout_(other.timeout_), sent_bytes_(0) {}
 
 void ResponseWriter::setMime(const Mime::MediaType &mime) {
   auto ct = response_.headers().tryGet<Header::ContentType>();
@@ -789,6 +781,7 @@ Async::Promise<ssize_t> ResponseWriter::putOnWire(const char *data,
     }
 
     auto buffer = buf_.buffer();
+    sent_bytes_ += buffer.size();
 
     timeout_.disarm();
 
@@ -908,8 +901,7 @@ void Handler::onInput(const char *buffer, size_t len,
     auto state = parser.parse();
 
     if (state == Private::State::Done) {
-      ResponseWriter response(transport(), parser.request, this);
-      response.associatePeer(peer);
+      ResponseWriter response(transport(), parser.request, this, peer);
 
 #ifdef LIBSTDCPP_SMARTPTR_LOCK_FIXME
       parser.request.associatePeer(peer);
@@ -931,15 +923,13 @@ void Handler::onInput(const char *buffer, size_t len,
     }
 
   } catch (const HttpError &err) {
-    ResponseWriter response(transport(), parser.request, this);
-    response.associatePeer(peer);
+    ResponseWriter response(transport(), parser.request, this, peer);
     response.send(static_cast<Code>(err.code()), err.reason());
     parser.reset();
   }
 
   catch (const std::exception &e) {
-    ResponseWriter response(transport(), parser.request, this);
-    response.associatePeer(peer);
+    ResponseWriter response(transport(), parser.request, this, peer);
     response.send(Code::Internal_Server_Error, e.what());
     parser.reset();
   }
@@ -951,18 +941,30 @@ void Handler::onConnection(const std::shared_ptr<Tcp::Peer> &peer) {
 
 void Handler::onDisconnection(const std::shared_ptr<Tcp::Peer> & /*peer*/) {}
 
-void Handler::onTimeout(const Request &request, ResponseWriter response) {
-  UNUSED(request)
-  UNUSED(response)
+void Handler::onTimeout(const Request & /*request*/,
+                        ResponseWriter /*response*/) {}
+
+Timeout::~Timeout() { disarm(); }
+
+void Timeout::disarm() {
+  if (transport && armed) {
+    transport->disarmTimer(timerFd);
+  }
 }
 
+bool Timeout::isArmed() const { return armed; }
+
+Timeout::Timeout(Tcp::Transport *transport_, Handler *handler_,
+                 Request request_, std::weak_ptr<Tcp::Peer> peer_)
+    : handler(handler_), request(std::move(request_)), transport(transport_),
+      armed(false), timerFd(-1), peer(peer_) {}
+
 void Timeout::onTimeout(uint64_t numWakeup) {
   UNUSED(numWakeup)
   if (!peer.lock())
     return;
 
-  ResponseWriter response(transport, request, handler);
-  response.associatePeer(peer);
+  ResponseWriter response(transport, request, handler, peer);
 
   handler->onTimeout(request, std::move(response));
 }

src/server/listener.cc

@@ -310,26 +310,27 @@ void Listener::handleNewConnection() {
   struct sockaddr_in peer_addr;
   int client_fd = acceptConnection(peer_addr);
 
-#ifdef PISTACHE_USE_SSL
-  SSL *ssl = nullptr;
+  void *ssl = nullptr;
 
+#ifdef PISTACHE_USE_SSL
   if (this->useSSL_) {
 
-    ssl = SSL_new((SSL_CTX *)this->ssl_ctx_);
-    if (ssl == NULL) {
+    SSL* ssl_data = SSL_new((SSL_CTX *)this->ssl_ctx_);
+    if (ssl_data == nullptr) {
       close(client_fd);
       throw std::runtime_error("Cannot create SSL connection");
     }
 
-    SSL_set_fd(ssl, client_fd);
-    SSL_set_accept_state(ssl);
+    SSL_set_fd(ssl_data, client_fd);
+    SSL_set_accept_state(ssl_data);
 
-    if (SSL_accept(ssl) <= 0) {
+    if (SSL_accept(ssl_data) <= 0) {
       ERR_print_errors_fp(stderr);
-      SSL_free(ssl);
+      SSL_free(ssl_data);
       close(client_fd);
       return;
     }
+    ssl = static_cast<void*>(ssl_data);
   }
 #endif /* PISTACHE_USE_SSL */
 

tests/http_server_test.cc

@@ -340,3 +340,71 @@ TEST(http_server_test, server_request_copies_address) {
 
   ASSERT_EQ("127.0.0.1", resultData);
 }
+
+struct ResponseSizeHandler : public Http::Handler {
+  HTTP_PROTOTYPE(ResponseSizeHandler)
+
+  explicit ResponseSizeHandler(size_t &rsize, Http::Code &rcode)
+      : rsize_(rsize), rcode_(rcode) {}
+
+  void onRequest(const Http::Request &request,
+                 Http::ResponseWriter writer) override {
+    std::string requestAddress = request.address().host();
+    writer.send(Http::Code::Ok, requestAddress);
+    std::cout << "[server] Sent: " << requestAddress << std::endl;
+    rsize_ = writer.getResponseSize();
+    rcode_ = writer.getResponseCode();
+  }
+
+  size_t &rsize_;
+  Http::Code &rcode_;
+};
+
+TEST(http_server_test, response_size_captured) {
+  const Pistache::Address address("localhost", Pistache::Port(0));
+
+  size_t rsize = 0;
+  Http::Code rcode;
+
+  Http::Endpoint server(address);
+  auto flags = Tcp::Options::ReuseAddr;
+  auto server_opts = Http::Endpoint::options().flags(flags);
+  server.init(server_opts);
+  server.setHandler(Http::make_handler<ResponseSizeHandler>(rsize, rcode));
+  server.serveThreaded();
+
+  const std::string server_address = "localhost:" + server.getPort().toString();
+  std::cout << "Server address: " << server_address << "\n";
+
+  // Use the built-in http client, but this test is interested in testing
+  // that the ResponseWriter in the server stashed the correct size and code
+  // values.
+  Http::Client client;
+  client.init();
+  auto rb = client.get(server_address);
+  auto response = rb.send();
+  std::string resultData;
+  response.then(
+      [&resultData](Http::Response resp) {
+        std::cout << "Response code is " << resp.code() << std::endl;
+        if (resp.code() == Http::Code::Ok) {
+          resultData = resp.body();
+        }
+      },
+      Async::Throw);
+
+  const int WAIT_TIME = 2;
+  Async::Barrier<Http::Response> barrier(response);
+  barrier.wait_for(std::chrono::seconds(WAIT_TIME));
+
+  client.shutdown();
+  server.shutdown();
+
+  // Sanity check (stolen from AddressEchoHandler test).
+  ASSERT_EQ("127.0.0.1", resultData);
+
+  std::cout << "Response size is " << rsize << "\n";
+  ASSERT_GT(rsize, 1);
+  ASSERT_LT(rsize, 300);
+  ASSERT_EQ(rcode, Http::Code::Ok);
+}

tests/router_test.cc

@@ -230,3 +230,51 @@ TEST(router_test, test_route_head_request) {
 
   endpoint->shutdown();
 }
+
+class MyHandler {
+public:
+
+  MyHandler()
+  : count_(0)
+  {}
+  ~MyHandler() {}
+
+  void handle(
+    const Pistache::Rest::Request &,
+    Pistache::Http::ResponseWriter response) {
+    count_++;
+    response.send(Pistache::Http::Code::Ok);
+  }
+
+  int getCount() { return count_; }
+
+private:
+
+  int count_;
+
+};
+
+TEST(router_test, test_bind_shared_ptr) {
+  Address addr(Ipv4::any(), 0);
+  auto endpoint = std::make_shared<Http::Endpoint>(addr);
+
+  auto opts = Http::Endpoint::options().threads(1).maxRequestSize(4096);
+  endpoint->init(opts);
+
+  std::shared_ptr<MyHandler> sharedPtr = std::make_shared<MyHandler>();
+
+  Rest::Router router;
+
+  Routes::Head(router, "/tinkywinky", Routes::bind(&MyHandler::handle, sharedPtr));
+
+  endpoint->setHandler(router.handler());
+  endpoint->serveThreaded();
+  const auto bound_port = endpoint->getPort();
+  httplib::Client client("localhost", bound_port);
+
+  ASSERT_EQ(sharedPtr->getCount(), 0);
+  client.Head("/tinkywinky");
+  ASSERT_EQ(sharedPtr->getCount(), 1);
+
+  endpoint->shutdown();
+}