From c8e03f2df8ecffc502030f54c02580fbc8e9d954 Mon Sep 17 00:00:00 2001
From: Fred Nicolson <fred.nicolson@outlook.com>
Date: Mon, 25 Sep 2017 16:19:47 +0100
Subject: [PATCH] Added more tests. Tweaks.

Receiving an http request/response will now return errors like HttpHeaderTooBig, instead of the type being set to that.

Added fr::Socket::status_to_string for converting status values into English strings.
---
 include/frnetlib/Http.h         |   7 +-
 include/frnetlib/HttpRequest.h  |   3 +-
 include/frnetlib/HttpResponse.h |   9 +--
 include/frnetlib/SSLSocket.h    |   4 +-
 include/frnetlib/Sendable.h     |   2 +-
 include/frnetlib/Socket.h       |  16 ++++-
 src/Http.cpp                    |   6 +-
 src/HttpRequest.cpp             |  30 ++++----
 src/HttpResponse.cpp            |  30 ++++----
 src/Socket.cpp                  |  25 +++++++
 tests/HttpRequestTest.cpp       |  32 ++++++++-
 tests/HttpResponseTest.cpp      | 118 ++++++++++++++++++++++++++++++++
 tests/SocketTest.cpp            |  32 +++++++++
 13 files changed, 266 insertions(+), 48 deletions(-)
 create mode 100644 tests/HttpResponseTest.cpp
 create mode 100644 tests/SocketTest.cpp

diff --git a/include/frnetlib/Http.h b/include/frnetlib/Http.h
index 8e68bf4..8c8233b 100644
--- a/include/frnetlib/Http.h
+++ b/include/frnetlib/Http.h
@@ -27,9 +27,6 @@ namespace fr
         };
         enum RequestStatus
         {
-            ParseError = 0,
-            HttpHeaderTooBig = 1,
-            HttpBodyTooBig = 2,
             Continue = 100,
             SwitchingProtocols = 101,
             Ok = 200,
@@ -100,9 +97,9 @@ namespace fr
          *
          * @param data The request/response to parse
          * @param datasz The length of data in bytes
-         * @return True if more data is needed, false if finished.
+         * @return NotEnoughData if parse needs to be called again. Success on success, other on error.
          */
-        virtual bool parse(const char *data, size_t datasz)=0;
+        virtual fr::Socket::Status parse(const char *data, size_t datasz)=0;
 
         /*!
          * Constructs a HTTP request/response to send.
diff --git a/include/frnetlib/HttpRequest.h b/include/frnetlib/HttpRequest.h
index 81a8b52..36774a9 100644
--- a/include/frnetlib/HttpRequest.h
+++ b/include/frnetlib/HttpRequest.h
@@ -30,7 +30,7 @@ namespace fr
          * @param datasz The length of data in bytes
          * @return True if more data is needed, false if finished.
          */
-        bool parse(const char *data, size_t datasz) override;
+        fr::Socket::Status parse(const char *data, size_t datasz) override;
 
         /*!
          * Constructs a Http Request, ready to send.
@@ -44,6 +44,7 @@ namespace fr
          * Parses the request header.
          *
          * @param header_end_pos The position in 'body' of the end of the header
+         * @return True on success, false on failure
          */
         bool parse_header(int64_t header_end_pos);
 
diff --git a/include/frnetlib/HttpResponse.h b/include/frnetlib/HttpResponse.h
index 172ae72..96b9788 100644
--- a/include/frnetlib/HttpResponse.h
+++ b/include/frnetlib/HttpResponse.h
@@ -24,13 +24,14 @@ namespace fr
         virtual ~HttpResponse() = default;
 
         /*!
-         * Parse a HTTP response.
+         * Parse a raw request or response from a string
+         * into the object.
          *
-         * @param data The HTTP response to parse
+         * @param data The request/response to parse
          * @param datasz The length of data in bytes
-         * @return True if more data is needed, false if finished.
+         * @return NotEnoughData if parse needs to be called again. Success on success, other on error.
          */
-        bool parse(const char *data, size_t datasz) override;
+        fr::Socket::Status parse(const char *data, size_t datasz) override;
 
         /*!
          * Constructs a HttpResponse, ready to send.
diff --git a/include/frnetlib/SSLSocket.h b/include/frnetlib/SSLSocket.h
index 0c2b52f..0d909f7 100644
--- a/include/frnetlib/SSLSocket.h
+++ b/include/frnetlib/SSLSocket.h
@@ -84,6 +84,8 @@ namespace fr
          */
         int32_t get_socket_descriptor() const override
         {
+            if(!ssl_socket_descriptor)
+                return -1;
             return ssl_socket_descriptor->fd;
         }
 
@@ -104,7 +106,7 @@ namespace fr
          */
         inline bool connected() const final
         {
-            return ssl_socket_descriptor->fd > -1;
+            return ssl_socket_descriptor && ssl_socket_descriptor->fd > -1;
         }
 
         /*!
diff --git a/include/frnetlib/Sendable.h b/include/frnetlib/Sendable.h
index e0e4c62..cb9ccf1 100644
--- a/include/frnetlib/Sendable.h
+++ b/include/frnetlib/Sendable.h
@@ -19,7 +19,7 @@ namespace fr
          * @param socket The socket to send through
          * @return Status indicating if the send succeeded or not.
          */
-        virtual Socket::Status send(Socket *socket) = 0;
+        virtual Socket::Status send(Socket *socket) = 0; //TODO: RETURN PROPER VALUE FROM HTTP PARSE
 
         /*!
          * Overrideable receive, to allow
diff --git a/include/frnetlib/Socket.h b/include/frnetlib/Socket.h
index f3fddf8..b38db19 100644
--- a/include/frnetlib/Socket.h
+++ b/include/frnetlib/Socket.h
@@ -29,7 +29,11 @@ namespace fr
             HandshakeFailed = 8,
             VerificationFailed = 9,
             MaxPacketSizeExceeded = 10,
-            NotEnoughData = 11
+            NotEnoughData = 11,
+            ParseError = 12,
+            HttpHeaderTooBig = 13,
+            HttpBodyTooBig = 14,
+            //Remember to update status_to_string if more are added
         };
 
         enum IP
@@ -215,6 +219,16 @@ namespace fr
         {
             return max_receive_size;
         }
+
+        /*!
+         * Converts an fr::Socket::Status value to a printable string
+         *
+         * Throws an std::logic_error if status is out of range.
+         *
+         * @param status Status value to convert
+         * @return A string form version
+         */
+        static const std::string &status_to_string(fr::Socket::Status status);
     protected:
 
         /*!
diff --git a/src/Http.cpp b/src/Http.cpp
index 14e14fb..4167f12 100644
--- a/src/Http.cpp
+++ b/src/Http.cpp
@@ -953,6 +953,7 @@ namespace fr
     {
         char recv_buffer[RECV_CHUNK_SIZE];
         size_t received = 0;
+        fr::Socket::Status state;
         do
         {
             //Receive the request
@@ -961,8 +962,9 @@ namespace fr
                 return status;
 
             //Parse it
-        } while(parse(recv_buffer, received));
+            state = parse(recv_buffer, received);
+        } while(state == fr::Socket::NotEnoughData);
 
-        return Socket::Success;
+        return state;
     }
 }
\ No newline at end of file
diff --git a/src/HttpRequest.cpp b/src/HttpRequest.cpp
index 427be65..ddf1eee 100644
--- a/src/HttpRequest.cpp
+++ b/src/HttpRequest.cpp
@@ -15,20 +15,13 @@ namespace fr
 
     }
 
-    bool HttpRequest::parse(const char *request, size_t requestsz)
+    fr::Socket::Status HttpRequest::parse(const char *request, size_t requestsz)
     {
         body += std::string(request, requestsz);
 
         //Ensure that the whole header has been parsed first
         if(!header_ended)
         {
-            //Ensure that the header doesn't exceed max length
-            if(body.size() > MAX_HTTP_HEADER_SIZE)
-            {
-                status = HttpHeaderTooBig;
-                return false; //End parse
-            }
-
             //Check to see if this request data contains the end of the header
             uint16_t header_end_size = 4;
             auto header_end = body.find("\r\n\r\n");
@@ -39,13 +32,19 @@ namespace fr
             }
             header_ended = header_end != std::string::npos;
 
-            //If the header end has not been found, return true, indicating that we need more data.
+            //Ensure that the header doesn't exceed max length
+            if(!header_ended && body.size() > MAX_HTTP_HEADER_SIZE || header_ended && header_end > MAX_HTTP_HEADER_SIZE)
+            {
+                return fr::Socket::HttpHeaderTooBig;
+            }
+
+            //If the header end has not been found, ask for more data.
             if(!header_ended)
-                return true;
+                return fr::Socket::NotEnoughData;
 
             //Else parse it
             if(!parse_header(header_end))
-                return false;
+                return fr::Socket::ParseError;
 
             //Leave things after the header intact
             body.erase(0, header_end + header_end_size);
@@ -54,8 +53,7 @@ namespace fr
         //Ensure that body doesn't exceed maximum length
         if(body.size() > MAX_HTTP_BODY_SIZE)
         {
-            status = HttpBodyTooBig;
-            return false; //End parse
+            return fr::Socket::HttpBodyTooBig;
         }
 
         //If we've got the whole request, parse the POST if it exists
@@ -63,10 +61,10 @@ namespace fr
         {
             if(request_type == RequestType::Post)
                 parse_post_body();
-            return false;
+            return fr::Socket::Success;
         }
 
-        return true;
+        return fr::Socket::NotEnoughData;
     }
 
     bool HttpRequest::parse_header(int64_t header_end_pos)
@@ -77,7 +75,7 @@ namespace fr
             size_t line = 0;
             std::vector<std::string> header_lines = split_string(body.substr(0, (unsigned long)header_end_pos));
             if(header_lines.empty())
-                return false;
+                return true;
 
             //Parse request type & uri
             parse_header_type(header_lines[line]);
diff --git a/src/HttpResponse.cpp b/src/HttpResponse.cpp
index 2418b16..fced97e 100644
--- a/src/HttpResponse.cpp
+++ b/src/HttpResponse.cpp
@@ -7,20 +7,13 @@
 
 namespace fr
 {
-    bool HttpResponse::parse(const char *response_data, size_t datasz)
+    fr::Socket::Status HttpResponse::parse(const char *response_data, size_t datasz)
     {
         body += std::string(response_data, datasz);
 
         //Ensure that the whole header has been parsed first
         if(!header_ended)
         {
-            //Ensure that the header doesn't exceed max length
-            if(body.size() > MAX_HTTP_HEADER_SIZE)
-            {
-                status = HttpHeaderTooBig;
-                return false; //End parse
-            }
-
             //Check to see if this request data contains the end of the header
             uint16_t header_end_size = 4;
             auto header_end = body.find("\r\n\r\n");
@@ -31,13 +24,19 @@ namespace fr
             }
             header_ended = header_end != std::string::npos;
 
-            //If the header end has not been found, return true, indicating that we need more data.
+            //Ensure that the header doesn't exceed max length
+            if(!header_ended && body.size() > MAX_HTTP_HEADER_SIZE || header_ended && header_end > MAX_HTTP_HEADER_SIZE)
+            {
+                return fr::Socket::HttpHeaderTooBig;
+            }
+
+            //If the header end has not been found, ask for more data.
             if(!header_ended)
-                return true;
+                return fr::Socket::NotEnoughData;
 
             //Else parse it
-            parse_header(header_end);
-            body.clear();
+            if(!parse_header(header_end))
+                return fr::Socket::ParseError;
 
             //Leave things after the header intact
             body.erase(0, header_end + header_end_size);
@@ -46,14 +45,15 @@ namespace fr
         //Ensure that body doesn't exceed maximum length
         if(body.size() > MAX_HTTP_BODY_SIZE)
         {
-            status = HttpBodyTooBig;
-            return false; //End parse
+            return fr::Socket::HttpBodyTooBig;
         }
 
         //Cut off any data if it exceeds content length
         if(body.size() > content_length)
             body.resize(content_length);
-        return body.size() < content_length;
+        else if(body.size() < content_length)
+            return fr::Socket::NotEnoughData;
+        return fr::Socket::Success;
 
     }
 
diff --git a/src/Socket.cpp b/src/Socket.cpp
index 080d15d..e4dbf22 100644
--- a/src/Socket.cpp
+++ b/src/Socket.cpp
@@ -5,6 +5,7 @@
 #include <mutex>
 #include <csignal>
 #include <iostream>
+#include <vector>
 #include "frnetlib/NetworkEncoding.h"
 #include "frnetlib/Socket.h"
 #include "frnetlib/Sendable.h"
@@ -102,4 +103,28 @@ namespace fr
     {
         max_receive_size = sz;
     }
+
+    const std::string &Socket::status_to_string(fr::Socket::Status status)
+    {
+        static std::vector<std::string> map = {
+        "Unknown",
+        "Success",
+        "Listen Failed",
+        "Bind Failed",
+        "Disconnected",
+        "Error",
+        "Would Block",
+        "Connection Failed",
+        "Handshake Failed",
+        "Verification Failed",
+        "Max packet size exceeded",
+        "Not enough data",
+        "Parse error",
+        "HTTP header too big",
+        "HTTP body too big"};
+
+        if(status < 0 || status > map.size())
+            throw std::logic_error("Socket::status_to_string(): Invalid status value " + std::to_string(status));
+        return map[status];
+    }
 }
\ No newline at end of file
diff --git a/tests/HttpRequestTest.cpp b/tests/HttpRequestTest.cpp
index 12c22d3..0dc225d 100644
--- a/tests/HttpRequestTest.cpp
+++ b/tests/HttpRequestTest.cpp
@@ -14,7 +14,7 @@ TEST(HttpRequestTest, get_request_parse)
 
     //Parse it
     fr::HttpRequest request;
-    ASSERT_EQ(request.parse(raw_request.c_str(), raw_request.size()), false);
+    ASSERT_EQ(request.parse(raw_request.c_str(), raw_request.size()), fr::Socket::Success);
 
     //Check that the request type is intact
     ASSERT_EQ(request.get_type(), fr::Http::Get);
@@ -56,7 +56,7 @@ TEST(HttpRequestTest, post_request_parse)
 
     //Parse it
     fr::HttpRequest request;
-    ASSERT_EQ(request.parse(raw_request.c_str(), raw_request.size()), false);
+    ASSERT_EQ(request.parse(raw_request.c_str(), raw_request.size()), fr::Socket::Success);
 
     //Check that the request type is intact
     ASSERT_EQ(request.get_type(), fr::Http::Post);
@@ -157,4 +157,32 @@ TEST(HttpRequestTest, post_request_construction)
     ASSERT_EQ(request.post("my_post"), "post_data");
     ASSERT_EQ(request.post("some_post"), "more_post");
     ASSERT_EQ(request.get_type(), fr::Http::Post);
+}
+
+TEST(HttpRequestTest, partial_parse)
+{
+    //The test request to parse
+    const std::string raw_request1 =
+            "GET /index.html?var=bob&other=trob HTTP/1.1\n"
+                    "Host: frednicolson.co.uk\r\n"
+                    "Content-Type: application/x-www-form-urlencoded\r\n"
+                    "My-Header:   ";
+
+    const std::string raw_request2 =
+            " header1\n"
+            "My-Other-Header:header2\r\n"
+            "Cache-Control: no-cache\r\n\r\n";
+
+
+    //Parse part 1
+    fr::HttpRequest request;
+    ASSERT_EQ(request.parse(raw_request1.c_str(), raw_request1.size()), fr::Socket::NotEnoughData);
+
+    //Parse part 2
+    ASSERT_EQ(request.parse(raw_request2.c_str(), raw_request2.size()), fr::Socket::Success);
+
+    //Verify it
+    ASSERT_EQ(request.get_type(), fr::Http::Get);
+    ASSERT_EQ(request.header("content-type"), "application/x-www-form-urlencoded");
+    ASSERT_EQ(request.header("Cache-Control"), "no-cache");
 }
\ No newline at end of file
diff --git a/tests/HttpResponseTest.cpp b/tests/HttpResponseTest.cpp
new file mode 100644
index 0000000..a262d39
--- /dev/null
+++ b/tests/HttpResponseTest.cpp
@@ -0,0 +1,118 @@
+//
+// Created by fred.nicolson on 25/09/17.
+//
+
+#include <gtest/gtest.h>
+#include <frnetlib/HttpResponse.h>
+
+TEST(HttpResponseTest, response_parse)
+{
+    const std::string raw_response =
+            "HTTP/1.1 301 Moved Permanently\n"
+            "Server: nginx/1.10.2\n"
+            "Date: Mon, 25 Sep 2017 13:51:56 GMT\n"
+            "Content-Type: text/html\n"
+            "Content-Length: 177\n"
+            "Connection: keep-alive\n"
+            "Location: https://frednicolson.co.uk/\n"
+            "\n"
+            "<html>\n"
+            "<head><title>301 Moved Permanently</title></head>\n"
+            "<body bgcolor=\"white\">\n"
+            "<center><h1>301 Moved Permanently</h1></center>\n"
+            "<hr><center>nginx/1.10.2</center>\n"
+            "</body>\n"
+            "</html>";
+
+    const std::string response_body =
+            "<html>\n"
+            "<head><title>301 Moved Permanently</title></head>\n"
+            "<body bgcolor=\"white\">\n"
+            "<center><h1>301 Moved Permanently</h1></center>\n"
+            "<hr><center>nginx/1.10.2</center>\n"
+            "</body>\n"
+            "</html>";
+
+    //Parse response
+    fr::HttpResponse test;
+    ASSERT_EQ(test.parse(raw_response.c_str(), raw_response.size()), fr::Socket::Success);
+
+    //Verify it
+    ASSERT_EQ(test.get_status(), fr::Http::MovedPermanently);
+    ASSERT_EQ(test.header("Content-length"), "177");
+    ASSERT_EQ(test.get_body(), response_body);
+}
+
+TEST(HttpResponseTest, response_partial_parse)
+{
+    const std::string raw_response1 =
+            "HTTP/1.1 301 Moved Permanently\n"
+                    "Server: nginx/1.10.2\n"
+                    "Date: Mon, 25 Sep 2017 13:51:56 GMT\n"
+                    "Content-Type: text/html\n"
+                    "Content-Length: 177\n"
+                    "Connection: keep-alive\n";
+
+    std::string raw_response2 =
+                    "Location: https://frednicolson.co.uk/\n"
+                    "\n"
+                    "<html>\n"
+                    "<head><title>301 Moved Permanently</title></head>\n"
+                    "<body bgcolor=\"white\">\n";
+
+    std::string raw_response3 =
+                    "<center><h1>301 Moved Permanently</h1></center>\n"
+                    "<hr><center>nginx/1.10.2</center>\n"
+                    "</body>\n"
+                    "</html>";
+
+    const std::string response_body =
+            "<html>\n"
+            "<head><title>301 Moved Permanently</title></head>\n"
+            "<body bgcolor=\"white\">\n"
+            "<center><h1>301 Moved Permanently</h1></center>\n"
+            "<hr><center>nginx/1.10.2</center>\n"
+            "</body>\n"
+            "</html>";
+
+    //Parse response
+    fr::HttpResponse test;
+    ASSERT_EQ(test.parse(raw_response1.c_str(), raw_response1.size()), fr::Socket::NotEnoughData);
+    ASSERT_EQ(test.parse(raw_response2.c_str(), raw_response2.size()), fr::Socket::NotEnoughData);
+    ASSERT_EQ(test.parse(raw_response3.c_str(), raw_response3.size()), fr::Socket::Success);
+
+    //Verify it
+    ASSERT_EQ(test.get_status(), fr::Http::MovedPermanently);
+    ASSERT_EQ(test.header("Content-length"), "177");
+    ASSERT_EQ(test.get_body(), response_body);
+}
+
+TEST(HttpResponseTest, header_length_test)
+{
+    //Try data with no header end first
+    std::string buff(MAX_HTTP_HEADER_SIZE + 1, '\0');
+    fr::HttpResponse response;
+    ASSERT_EQ(response.parse(buff.c_str(), buff.size()), fr::Socket::HttpHeaderTooBig);
+    response = {};
+
+    //Now try short header but long data, this should work
+    buff = "HTTP/1.1 301 Moved Permanently\n"
+                    "Content-Type: text/html\n"
+                    "Content-Length: " + std::to_string(MAX_HTTP_BODY_SIZE - 1) + "\n"
+                    "Connection: keep-alive\n"
+                    "\n" + std::string(MAX_HTTP_BODY_SIZE - 1, '\0');
+    ASSERT_EQ(response.parse(buff.c_str(), buff.size()), fr::Socket::Success);
+}
+
+TEST(HttpResponseTest, body_length_test)
+{
+    std::string buff =
+            "HTTP/1.1 301 Moved Permanently\n"
+            "Content-Type: text/html\n"
+            "Content-Length: " + std::to_string(MAX_HTTP_BODY_SIZE + 1) + "\n"
+            "Connection: keep-alive\n"
+            "\n";
+    buff += std::string(MAX_HTTP_BODY_SIZE + 1, '\0');
+    fr::HttpResponse response;
+    ASSERT_EQ(response.parse(buff.c_str(), buff.size()), fr::Socket::HttpBodyTooBig);
+}
\ No newline at end of file
diff --git a/tests/SocketTest.cpp b/tests/SocketTest.cpp
new file mode 100644
index 0000000..9ef1aa0
--- /dev/null
+++ b/tests/SocketTest.cpp
@@ -0,0 +1,32 @@
+//
+// Created by fred.nicolson on 25/09/17.
+//
+
+#include <gtest/gtest.h>
+#include <frnetlib/Socket.h>
+
+TEST(SocketTest, status_to_string_valid)
+{
+    ASSERT_EQ(fr::Socket::status_to_string(fr::Socket::Status::Unknown), "Unknown");
+    ASSERT_EQ(fr::Socket::status_to_string(fr::Socket::Status::HttpBodyTooBig), "HTTP body too big");
+}
+
+TEST(SocketTest, status_to_string_invalid)
+{
+    try
+    {
+        auto str = fr::Socket::status_to_string(static_cast<fr::Socket::Status>(-1));
+    }
+    catch(const std::logic_error &)
+    {
+        try
+        {
+            auto str = fr::Socket::status_to_string(static_cast<fr::Socket::Status>(99999));
+        }
+        catch(const std::logic_error &)
+        {
+            return;
+        }
+    }
+    ASSERT_TRUE(false);
+}
\ No newline at end of file