Feature/improve upload handler

config/eval.conf

@@ -35,12 +35,13 @@ server {
     }
 
     location /cgi-py/ {
-        allow_methods GET POST;
+        cgi_allow_methods GET POST;
         cgi_handler .py /usr/bin/python3;
+        allow_uploads on;
     }
 
     location /cgi-bin/ {
-        allow_methods GET POST;
+        cgi_allow_methods GET POST;
         cgi_handler .bla /bin/sh;
         allow_uploads on;
     }

src/core/client.cpp

@@ -120,7 +120,7 @@ static void print_raw_data(const char* s, size_t n)
             std::cout.put(c);
         }
     }
-    std::cout.flush();
+    std::cout << "\n";
 }
 
 void Client::read_from_socket()

src/core/server.cpp

@@ -48,7 +48,7 @@ void Server::init()
 {
     epoll_fd_ = epoll_create1(0);
     if (epoll_fd_ == -1)
-        throw std::runtime_error("epoll_create1 failed");
+        throw std::runtime_error("Server::init: epoll_create1 failed");
 
     for (size_t i = 0; i < config_.servers.size(); i++) {
         const ServerConfig& sconf = config_.servers[i];
@@ -59,18 +59,18 @@ void Server::init()
         for (size_t j = 0; j < sconf.listen_addrs.size(); j++) {
             int fd = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0);
             if (fd == -1)
-                throw std::runtime_error("socket failed");
+                throw std::runtime_error("Server::init: socket failed");
 
             int yes = 1;
             setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(yes));
 
             const sockaddr_in& addr = sconf.listen_addrs[j];
 
             if (bind(fd, (sockaddr*) &addr, sizeof(addr)) == -1)
-                throw std::runtime_error("bind failed");
+                throw std::runtime_error("Server::init: bind failed");
 
             if (listen(fd, WEBSERV_DEFAULT_MAX_PENDING_CONNECTIONS) == -1)
-                throw std::runtime_error("listen failed");
+                throw std::runtime_error("Server::init: listen failed");
 
             listen_fds_.push_back(fd);
             server_map_[fd] = vs;
@@ -79,7 +79,7 @@ void Server::init()
             ev.events = EPOLLIN;
             ev.data.fd = fd;
             if (epoll_ctl(epoll_fd_, EPOLL_CTL_ADD, fd, &ev) == -1)
-                throw std::runtime_error("epoll_ctl failed");
+                throw std::runtime_error("Server::init: EPOLL_CTL_ADD failed");
         }
     }
 }
@@ -96,7 +96,7 @@ void Server::accept_connection(int fd)
         if (errno == EAGAIN || errno == EWOULDBLOCK) {
             return;
         }
-        throw std::runtime_error("accept4 failed");
+        throw std::runtime_error("Server::accept_connection: accept4 failed");
     }
 
     VirtualServer* vs = server_map_[fd];
@@ -109,7 +109,7 @@ void Server::accept_connection(int fd)
     ev.events = EPOLLRDHUP | EPOLLIN;
     ev.data.fd = client_fd;
     if (epoll_ctl(epoll_fd_, EPOLL_CTL_ADD, client_fd, &ev) == -1) {
-        throw std::runtime_error("EPOLL_CTL_ADD failed");
+        throw std::runtime_error("Server::accept_connection: EPOLL_CTL_ADD failed");
     }
 
     LOG(DEBUG) << "Client #" << client_fd << ": connection accepted";
@@ -124,7 +124,7 @@ void Server::close_connection(Client& client)
         int fd = it->first;
 
         if (epoll_ctl(epoll_fd_, EPOLL_CTL_DEL, fd, NULL) == -1)
-            throw std::runtime_error("EPOLL_CTL_DEL failed");
+            throw std::runtime_error("Server::close_connection: EPOLL_CTL_DEL failed");
 
         client_map_.erase(fd);
     }
@@ -144,7 +144,7 @@ void Server::close_connection(Client& client)
 Client& Server::get_client(int fd)
 {
     if (client_map_.count(fd) == 0)
-        throw std::runtime_error("Attempted to retrieve invalid client fd");
+        throw std::runtime_error("Server::get_client: Attempted to retrieve invalid client fd");
 
     return *client_map_[fd];
 }
@@ -160,12 +160,7 @@ bool Server::is_listen_fd(int fd) const
 
 bool Server::is_client_fd(int fd) const
 {
-    for (std::map<int, Client*>::const_iterator it = client_map_.begin(); it != client_map_.end();
-         ++it) {
-        if (it->first == fd)
-            return true;
-    }
-    return false;
+    return client_map_.find(fd) != client_map_.end();
 }
 
 void Server::run()
@@ -178,7 +173,7 @@ void Server::run()
             if (errno == EINTR) {
                 continue;
             }
-            throw std::runtime_error("epoll_wait failed");
+            throw std::runtime_error("Server::run: epoll_wait failed");
         }
 
         for (int i = 0; i < n_events; ++i) {
@@ -192,7 +187,7 @@ void Server::run()
                 handle_events(fd, ev);
             }
             else {
-                // FD was removed from our client list?
+                LOG(WARN) << "File descriptor: " << fd << " not tracked anymore";
                 continue;
             }
         }
@@ -238,21 +233,25 @@ void Server::update_interest_list(Client& client)
         ev.events = mask;
 
         if (client_map_.count(fd) == 0) {
+
             if (epoll_ctl(epoll_fd_, EPOLL_CTL_ADD, fd, &ev) == -1) {
-                throw std::runtime_error("EPOLL_CTL_ADD failed");
+                throw std::runtime_error("Server::update_interest_list: EPOLL_CTL_ADD failed");
             }
+
             client_map_[fd] = &client;
         }
         else {
             if (mask == 0) {
+                /*
                 if (epoll_ctl(epoll_fd_, EPOLL_CTL_DEL, fd, &ev) == -1) {
-                    throw std::runtime_error("EPOLL_CTL_DEL failed");
+                    throw std::runtime_error("Server::update_interest_list: EPOLL_CTL_DEL failed");
                 }
+                */
                 client_map_.erase(fd);
             }
             else {
                 if (epoll_ctl(epoll_fd_, EPOLL_CTL_MOD, fd, &ev) == -1) {
-                    throw std::runtime_error("EPOLL_CTL_MOD failed");
+                    throw std::runtime_error("Server::update_interest_list: EPOLL_CTL_MOD failed");
                 }
             }
         }

src/core/server.hpp

@@ -20,20 +20,17 @@ class Server {
     void run();
 
 private:
-    Server(const Server& other);
-    Server& operator=(const Server& other);
+    Server(const Server&);
+    Server& operator=(const Server&);
 
     void accept_connection(int fd);
+    void close_connection(Client& conn);
     void handle_events(int fd, uint32_t events);
-    void read_from_socket(Client& conn);
-    void write_to_socket(Client& conn);
     void update_interest_list(Client& conn);
 
+    Client& get_client(int fd);
     bool is_listen_fd(int fd) const;
     bool is_client_fd(int fd) const;
-    Client& get_client(int fd);
-    uint64_t add_connection(int client_fd, const sockaddr_in& addr);
-    void close_connection(Client& conn);
 
     const HttpConfig& config_;
 

src/handler/upload_handler.cpp

@@ -11,16 +11,16 @@
 #include <cstring>
 #include <iostream>
 #include <sstream>
+#include <stdexcept>
 
 UploadHandler::UploadHandler(const std::string& path, const RouteConfig& rc, const HttpRequest& req)
     : path_(path),
       rc_(rc),
       req_(req),
-      out_off_(0),
-      bytes_written_(0),
+      total_bytes_written_(0),
       fd_(-1),
-      done_(false)
-
+      is_upload_finished_(false),
+      has_error_(false)
 {
     if (req.content_length == 0) {
         // set_error(HttpResponse::kStatusBadRequest);
@@ -46,50 +46,75 @@ UploadHandler::~UploadHandler()
         close(fd_);
 }
 
+bool UploadHandler::has_output() const
+{
+    return !out_buf_.empty();
+}
+
+bool UploadHandler::needs_input() const
+{
+    return !has_error_ && !is_upload_finished_ && total_bytes_written_ < req_.content_length;
+}
+
+bool UploadHandler::is_done() const
+{
+    return !needs_input() && !has_output();
+}
+
 size_t UploadHandler::read_output(char* buf, size_t n)
 {
-    if (out_off_ >= out_buf_.size()) {
+    if (!has_output()) {
+        LOG(WARN) << "Tried to read 0 bytes from UploadHandler";
         return 0;
     }
-    size_t bytes_left = out_buf_.size() - out_off_;
-    size_t to_copy = std::min(bytes_left, n);
-    std::memcpy(buf, out_buf_.data() + out_off_, to_copy);
-    out_off_ += to_copy;
-    return (to_copy);
+
+    size_t to_copy = std::min(out_buf_.size(), n);
+    std::memcpy(buf, out_buf_.data(), to_copy);
+    out_buf_.erase(0, to_copy);
+
+    return to_copy;
 }
 
 size_t UploadHandler::write_input(const char* buf, size_t n)
 {
-    ssize_t bytes = write(fd_, buf, n);
-    if (bytes == 0) {
-        // should never happen ?
+    if (!needs_input()) {
+        LOG(WARN) << "Tried to write 0 bytes to UploadHandler";
+        return 0;
     }
-    if (bytes < 0) {
-        if (out_buf_.empty()) {
-            set_error(HttpResponse::kStatusDiskFull);
-        }
-        done_ = true; // to ensure needs_input returns false
+    if (total_bytes_written_ + n > rc_.shared.max_body_size) {
+        set_error(HttpResponse::kStatusContentTooLarge);
         return 0;
     }
-    bytes_written_ += bytes;
-    if (bytes_written_ < rc_.shared.max_body_size && bytes_written_ < req_.content_length) {
-        return (bytes);
+
+    ssize_t written = write(fd_, buf, n);
+    if (written == -1) {
+        set_error(HttpResponse::kStatusInternalServerError);
+        return 0;
     }
-    if (bytes_written_ >= rc_.shared.max_body_size) {
-        if (out_buf_.empty())
-            set_error(HttpResponse::kStatusBadRequest);
+    if (written == 0) {
+        return 0;
     }
-    if (out_buf_.empty()) {
-        res_ = HttpResponse::make_response_headers_only(HttpResponse::kStatusCreated, "", 0, req_);
-        out_buf_ = res_.to_string();
-        done_ = true;
+
+    assert(static_cast<size_t>(written) == n && "Partial write on regular file??");
+
+    total_bytes_written_ += written;
+
+    if (total_bytes_written_ == req_.content_length) {
+        finalize_upload();
     }
-    return (0);
+    return written;
 }
 
 void UploadHandler::set_error(const HttpResponse::Status code)
 {
     res_ = HttpResponse::make_error(code, rc_.shared.error_pages, req_);
     out_buf_ = res_.to_string();
-    done_ = true;
+    has_error_ = true;
+}
+
+void UploadHandler::finalize_upload()
+{
+    res_ = HttpResponse::make_response_headers_only(HttpResponse::kStatusCreated, "", 0, req_);
+    out_buf_ = res_.to_string();
+    is_upload_finished_ = true;
 }

src/handler/upload_handler.hpp

@@ -14,23 +14,23 @@ class UploadHandler : public Handler {
     explicit UploadHandler(const std::string& path, const RouteConfig& rc, const HttpRequest& req);
     virtual ~UploadHandler();
 
-    virtual size_t read_output(char* buf, size_t n); // we should not read from this handler
+    virtual size_t read_output(char* buf, size_t n);
     virtual size_t write_input(const char* buf, size_t n);
 
     virtual bool is_regular_file() const { return true; }
-    virtual bool has_output() const { return out_off_ < out_buf_.size(); }
-    virtual bool needs_input() const { return bytes_written_ < req_.content_length; }
-    virtual bool is_done() const { return done_ && out_off_ >= out_buf_.size(); }
+    virtual bool has_output() const;
+    virtual bool needs_input() const;
+    virtual bool is_done() const;
 
     virtual int cgi_read_fd() const { return -1; }
     virtual int cgi_write_fd() const { return -1; }
 
-    const std::string& path() const { return path_; } // still required ?
-
 private:
     UploadHandler(const UploadHandler&);
     UploadHandler& operator=(const UploadHandler&);
+
     void set_error(const HttpResponse::Status code);
+    void finalize_upload();
 
     // constructor args
     const std::string& path_;
@@ -40,11 +40,11 @@ class UploadHandler : public Handler {
     HttpResponse res_;
     // Serialized response and offset
     std::string out_buf_;
-    size_t out_off_;
     // other handler specifc variables
-    size_t bytes_written_;
+    size_t total_bytes_written_;
     int fd_;
-    bool done_;
+    bool is_upload_finished_;
+    bool has_error_;
 };
 
 #endif