From 387e56a7be6711f3e449323e5d9c35ef03840cd6 Mon Sep 17 00:00:00 2001 From: Kenneth Barbour Date: Fri, 9 Mar 2018 18:18:39 -0500 Subject: [PATCH] HttpRequest no longer responsible for parsing itself --- src/HttpRequest.cpp | 103 +----------------------------------- src/HttpRequest.h | 9 ---- test/test_HttpRequest.cpp | 26 +-------- test/test_Kernel.cpp | 39 +------------- test/test_RequestRouter.cpp | 27 ++++++---- 5 files changed, 19 insertions(+), 185 deletions(-) diff --git a/src/HttpRequest.cpp b/src/HttpRequest.cpp index 8c09dce..6ce8457 100644 --- a/src/HttpRequest.cpp +++ b/src/HttpRequest.cpp @@ -1,95 +1,8 @@ #include "HttpRequest.h" - -#define HTTPREQUEST_TRIM(stream) while(client_peek(stream)==' ') { client_read(stream); } -#define HTTPREQUEST_BUFFER_UNTIL(buff, strm, stop, n)\ - for (n=0; client_peek(strm) != stop;n++) \ - { buff[n] = (char) client_read(strm); } +#include "string.h" HttpRequest::HttpRequest(): method(), url(), httpver(), message(), message_length(0) {} -HttpRequest::HttpRequest(Stream &client):method(), url(), httpver(), headers(), message(), message_length(0) -{ - this->capture(client); -} - -void HttpRequest::capture(Stream &client) -{ - _timeout_millis = millis() + HTTPREQUEST_TIMEOUT; - long int i, k; - char buffer[HTTPREQUEST_MAX_MESSAGE_SIZE] = {0}; - - HTTPREQUEST_TRIM(client); - - // Read method until ' ' - //for (i=0; client.peek() != ' '; i++) buffer[i] = (char) client.read(); - HTTPREQUEST_BUFFER_UNTIL(buffer, client, ' ', i); - setMethod(buffer); - for (k=0; k <= i; k++) buffer[k] = '\0'; //reset buffer - - HTTPREQUEST_TRIM(client); - - // Read url until ' ' - for (i=0; client_peek(client) != ' '; i++) buffer[i] = (char) client_read(client); - setUrl(buffer, i); - for (k=0; k <= i; k++) buffer[k] = '\0'; //reset buffer - - HTTPREQUEST_TRIM(client); - - // Read httpver until '\r' - for (i=0; client_peek(client) != '\r'; i++) buffer[i] = (char) client_read(client); - setHttpVer(buffer); - for (k=0; k <= i; k++) buffer[k] = '\0'; //reset buffer - - // discard line break - if (client_peek(client) == '\r') client_read(client); - if (client_peek(client) == '\n') client_read(client); - - //read each line, parsing headers, discarding \r\n until a blank line is reached - while (client_peek(client) != '\r' && client_peek(client) != '\n') { - for (i=0; client_peek(client) != ':'; i++) // copy client contents into buffer - buffer[i] = (char) client_read(client); - client_read(client); // discard the ':' - HTTPREQUEST_TRIM(client); // discard whitespace - buffer[i++] = '\0'; - k = i; // start of value - for (; client_peek(client) != '\r'; i++) buffer[i] = (char) client_read(client); - headers.set(buffer, (buffer + k) ); - for (k=0; k <= i; k++) buffer[k] = '\0'; //reset buffer - - // discard next line break - if (client_peek(client) == '\r') client_read(client); - if (client_peek(client) == '\n') client_read(client); - } - client_read(client); - if (client_peek(client) == '\n') client_read(client); // consume \r\n - - // Determine or guess the length of the message - long int content_length; - if (headers.has("Content-Length") && !headers.has("Transfer-Encoding")) { - char* length_hdr = headers.get("Content-Length"); - char* length_hdr_end = strlen(length_hdr) + length_hdr; - content_length = strtol(length_hdr, &length_hdr_end, 10); - } else { - content_length = HTTPREQUEST_MAX_MESSAGE_SIZE; - } - - // copy rest of message - message = (char*) malloc( content_length + 1); - if (!message) { - //todo handle out of memory - } - for (i = 0; i < content_length && client.available(); i++) { - message[i] = (char) client_read(client); - } - message[i] = '\0'; - - // handle content_length too short - if (client.available()) { - //todo: content length too short - } - message_length = i; - -} const char * HttpRequest::setMethod(const char * method) { strncpy(this->method, method, HTTPREQUEST_METHOD_SIZE); @@ -151,20 +64,6 @@ const char * HttpRequest::getMessage() return this->message; } -char HttpRequest::client_read(Stream& client) -{ - while(!client.available() && millis() < _timeout_millis) - delay(1); - return client.read(); -} - -char HttpRequest::client_peek(Stream& client) -{ - while (!client.available() && millis() < _timeout_millis) - delay(1); - return client.peek(); -} - HttpRequest::~HttpRequest() { if (url) { diff --git a/src/HttpRequest.h b/src/HttpRequest.h index 0fb3a0e..642bbf3 100644 --- a/src/HttpRequest.h +++ b/src/HttpRequest.h @@ -9,10 +9,6 @@ #define HTTPREQUEST_MAX_MESSAGE_SIZE 512 #endif -#ifdef _TEST_ -#include "Stream.h" -#endif - #include "Arduino.h" #define HTTPREQUEST_METHOD_SIZE 8 @@ -22,9 +18,7 @@ class HttpRequest { public: HttpRequest(); - HttpRequest(Stream&); ~HttpRequest(); - void capture(Stream&); const char * setMethod(const char *); const char * getMethod(); const char * setUrl(const char *); @@ -41,9 +35,6 @@ class HttpRequest char httpver [HTTPREQUEST_HTTPVER_SIZE]; HttpHeaders headers; protected: - char client_read(Stream&); long int message_length; char * message; - char client_peek(Stream&); - unsigned long _timeout_millis; }; diff --git a/test/test_HttpRequest.cpp b/test/test_HttpRequest.cpp index 0b7274d..a9143fc 100644 --- a/test/test_HttpRequest.cpp +++ b/test/test_HttpRequest.cpp @@ -4,31 +4,7 @@ #include "DummyStream.h" using Catch::Matchers::Equals; - -TEST_CASE("Create and capture an HTTP Request","[HttpRequest]") { - const char * req_str = "GET / HTTP/1.1\r\nHost: localhost\r\n\r\n"; - DummyStream client(req_str); - HttpRequest req(client); - - CHECK_THAT(req.method, Equals("GET")); - CHECK_THAT(req.url, Equals("/")); - CHECK_THAT(req.httpver, Equals("HTTP/1.1")); - - REQUIRE(req.headers.has("Host")); - CHECK_THAT(req.headers.get("Host"), Equals("localhost")); - -} - -TEST_CASE("Capture a complicated HTTP Request","[HttpRequest]") { - const char * req_str = "POST /foo HTTP/1.1\r\nHost: localhost\r\nContent-Length: 11\r\n\r\nfoo=bar&baz"; - DummyStream client(req_str); - HttpRequest req; - req.capture(client); - - CHECK_THAT(req.url, Equals("/foo")); - CHECK_THAT(req.headers.get("Content-Length"), Equals("11")); - CHECK_THAT(req.getMessage(), Equals("foo=bar&baz")); -} + TEST_CASE("Test get/set method","[HttpRequest]") { diff --git a/test/test_Kernel.cpp b/test/test_Kernel.cpp index a251074..87d6cea 100644 --- a/test/test_Kernel.cpp +++ b/test/test_Kernel.cpp @@ -1,9 +1,5 @@ #include "catch.hpp" -#include "RouteDispatcher.h" -#include "HttpRequest.h" -#include "HttpResponse.h" -#include "Buffer.h" -#include "DummyStream.h" +#include "WebKernel.h" #include @@ -32,37 +28,4 @@ Route routes[] = { { POST, "/foo", save }, { GET, "/foo", create } }; -RequestRouter router(routes, sizeof routes / sizeof routes[1]); -RouteDispatcher dispatcher(router); -TEST_CASE("Test index","[Kernel]") -{ - Buffer requestBuffer(requestData, 256); - Buffer responseBuffer(responseData, 256); - requestBuffer.write("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n"); - - HttpRequest request(requestBuffer); - HttpResponse response(responseBuffer); - - dispatcher.handle(request, response); - - CHECK_THAT((const char *)responseData, Equals("Site Index")); - CHECK(response.code == 200); - CHECK_THAT(response.getReason(), Equals("OK")); -} - -TEST_CASE("Test post","[Kernel]") -{ - Buffer requestBuffer(requestData, 256); - Buffer responseBuffer(responseData, 256); - requestBuffer.write("POST /foo HTTP/1.1\r\nHost: localhost\r\n\r\nfoo=bar"); - - HttpRequest request(requestBuffer); - HttpResponse response(responseBuffer); - - dispatcher.handle(request, response); - - CHECK_THAT((const char *)responseData, Equals("You sent post data")); - CHECK(response.code == 201); - CHECK_THAT(response.getReason(), Equals("Created")); -} diff --git a/test/test_RequestRouter.cpp b/test/test_RequestRouter.cpp index d8058c5..b531056 100644 --- a/test/test_RequestRouter.cpp +++ b/test/test_RequestRouter.cpp @@ -39,20 +39,22 @@ TEST_CASE("Match","[RequestRouter]") RequestRouter router(routes, 3 ); const Route * match; - DummyStream client("POST /bar HTTP/1.1\r\nHost: localhost\r\n\r\nfoobar"); - DummyStream client2("GET /baz HTTP/1.1\r\nHost: localhost\r\n\r\nfoobar"); - DummyStream client3("POST /baz HTTP/1.1\r\nHost: localhost\r\n\r\nbaaz"); - - HttpRequest request(client); - HttpRequest request2(client2); - HttpRequest request3(client3); + HttpRequest request; + HttpRequest request2; + HttpRequest request3; + request.setMethod("POST"); + request.setUrl("/bar"); match = router.match(request); CHECK(match == &(routes[1])); + request2.setMethod("GET"); + request2.setUrl("/baz"); match = router.match(request2); CHECK(match == &(routes[2])); + request3.setMethod("POST"); + request3.setUrl("/baz"); match = router.match(request3); CHECK(match == &(routes[2])); } @@ -65,8 +67,9 @@ TEST_CASE("Not found","[RequestRouter]") RequestRouter router(routes, 1 ); const Route * match; - DummyStream client("GET /foo HTTP/1.1\r\nHost: localhost\r\n\r\n"); - HttpRequest request(client); + HttpRequest request; + request.setMethod("GET"); + request.setUrl("/foo"); match = router.match(request); CHECK(match == nullptr); @@ -81,8 +84,9 @@ TEST_CASE("Method not allowed", "[RequestRouter]") RequestRouter router(routes, 1); const Route * match; - DummyStream client("PUT / HTTP/1.1\r\nHost: localhost\r\n\r\nfoo"); - HttpRequest request(client); + HttpRequest request; + request.setMethod("PUT"); + request.setUrl("/"); match = router.match(request); CHECK(match == nullptr); @@ -95,6 +99,7 @@ TEST_CASE("Url Wildcards","[RequestRouter]") CHECK(RequestRouter::urlMatches("/foo","/foo") == true); CHECK(RequestRouter::urlMatches("/foo/","/foo") == false); + CHECK(RequestRouter::urlMatches("/foo/","/foo/") == true); CHECK(RequestRouter::urlMatches("/*","/foo") == true); CHECK(RequestRouter::urlMatches("/*/","/foo") == false); CHECK(RequestRouter::urlMatches("/foo/*/bar","/foo/baz/bar") == true);