From fd1019e1fa5f258caa0a1acac9d751498ccf8dde Mon Sep 17 00:00:00 2001 From: Kenneth Barbour Date: Tue, 20 Mar 2018 18:47:10 -0400 Subject: [PATCH] handle 404 and 405 errors --- src/RouteDispatcher.cpp | 13 +++++++++++-- src/RouteDispatcher.h | 5 +++-- src/WebKernel.cpp | 8 +++++--- src/WebKernel.h | 20 +++++++++++++++++++- test/test_RouteDispatcher.cpp | 10 ++++++++-- test/test_WebKernel.cpp | 19 +++++++++++++++++++ 6 files changed, 65 insertions(+), 10 deletions(-) diff --git a/src/RouteDispatcher.cpp b/src/RouteDispatcher.cpp index 72dfdb3..5500e2d 100644 --- a/src/RouteDispatcher.cpp +++ b/src/RouteDispatcher.cpp @@ -15,7 +15,8 @@ void RouteDispatcher::handle(HttpRequest& request, HttpResponse& response) notFoundHandler(request, response); break; case E_METHOD_NOT_ALLOWED: - methodNotAllowedHandler(request, response, router.lastAllowedMethods()); + responseSetAllowedHeader(response, router.lastAllowedMethods()); + methodNotAllowedHandler(request, response); break; default: response.code = 500; @@ -28,11 +29,19 @@ void RouteDispatcher::handle(HttpRequest& request, HttpResponse& response) void RouteDispatcher::handleNotFound(HttpRequest& request, HttpResponse& response) { response.code = 404; + response.headers.set("Content-Type","text/plain"); + response.print(response.getReason()); } -void RouteDispatcher::handleMethodNotAllowed(HttpRequest& request, HttpResponse& response, uint8_t allowed) +void RouteDispatcher::handleMethodNotAllowed(HttpRequest& request, HttpResponse& response) { response.code = 405; + response.headers.set("Content-Type","text/plain"); + response.print(response.getReason()); +} + +void RouteDispatcher::responseSetAllowedHeader(HttpResponse& response, uint8_t allowed) +{ if (allowed & GET) response.headers.append("Allow","GET"); if (allowed & POST) diff --git a/src/RouteDispatcher.h b/src/RouteDispatcher.h index a7dad7f..ee4f521 100644 --- a/src/RouteDispatcher.h +++ b/src/RouteDispatcher.h @@ -11,12 +11,13 @@ class RouteDispatcher // Default handlers for some common http errors static void handleNotFound(HttpRequest&, HttpResponse&); - static void handleMethodNotAllowed(HttpRequest&, HttpResponse&, uint8_t); + static void handleMethodNotAllowed(HttpRequest&, HttpResponse&); // re-assignable pointers to error handlers void (*notFoundHandler)(HttpRequest&, HttpResponse&); - void (*methodNotAllowedHandler)(HttpRequest&, HttpResponse&, uint8_t); + void (*methodNotAllowedHandler)(HttpRequest&, HttpResponse&); private: RequestRouter& router; + static void responseSetAllowedHeader(HttpResponse&, uint8_t); }; diff --git a/src/WebKernel.cpp b/src/WebKernel.cpp index 0e22d6e..f6bd03c 100644 --- a/src/WebKernel.cpp +++ b/src/WebKernel.cpp @@ -38,9 +38,11 @@ void WebKernel::handleClients() case S_DISPATCHING: HttpResponse response(_resp_buffer); response.setHttpVersion(_request.getHttpVersion()); - _dispatcher.handle(_request, response); - response.headers.set("Connection","close"); - _client.print(response); + if (!_parser.error()) { + _dispatcher.handle(_request, response); + response.headers.set("Connection","close"); + _client.print(response); + } keepClient = false; break; } diff --git a/src/WebKernel.h b/src/WebKernel.h index 39eb265..b988e66 100644 --- a/src/WebKernel.h +++ b/src/WebKernel.h @@ -27,11 +27,26 @@ class WebKernel _state(S_IDLE), _dispatcher(_router), _parser(_request, _client), - _resp_buffer(_resp_data, WEBKERNEL_RESPONSE_SIZE) + _resp_buffer(_resp_data, WEBKERNEL_RESPONSE_SIZE), + _badRequestHandler(handleBadRequest) {}; void begin() { _server.begin(); } void handleClients(); + static void handleBadRequest(HttpRequest& req, HttpResponse& resp) { + resp.code = 400; + } + + void setNotFoundHandler(void (*handler)(HttpRequest&, HttpResponse&)) + { + _dispatcher.notFoundHandler = handler; + } + + void setMethodNotAllowedHandler(void (*handler)(HttpRequest&, HttpResponse&)) + { + _dispatcher.methodNotAllowedHandler = handler; + } + #ifdef _TEST_ void mock_nextClient(const char * next) { _server._next = next; } WiFiClient& mock_currentClient() { return _client; } @@ -50,4 +65,7 @@ class WebKernel uint8_t _resp_data[WEBKERNEL_RESPONSE_SIZE]; Buffer _resp_buffer; + + void (*_badRequestHandler)(HttpRequest&, HttpResponse&); + }; diff --git a/test/test_RouteDispatcher.cpp b/test/test_RouteDispatcher.cpp index a1c4702..fbee3d4 100644 --- a/test/test_RouteDispatcher.cpp +++ b/test/test_RouteDispatcher.cpp @@ -15,8 +15,9 @@ void dummy_notFoundHandler(HttpRequest& request, HttpResponse& response) response.setReason("Custom"); } -void dummy_methodNotAllowedHandler(HttpRequest& request, HttpResponse& response, uint8_t allowed) +void dummy_methodNotAllowedHandler(HttpRequest& request, HttpResponse& response) { + // dont set Allow header. That should be automatically done response.code = 405; response.setReason("custom_not_allowed"); } @@ -47,13 +48,15 @@ TEST_CASE("Test default methodNotAllowedHandler","[RouteDispatcher]") { GET, "/", dummy_handler } }; + uint8_t buff[100]; + Buffer buffer(buff, sizeof buff); RequestRouter router(routes,1); RouteDispatcher dispatcher(router); dispatcher.notFoundHandler = dummy_notFoundHandler; HttpRequest request; request.setMethod("POST"); request.setUrl("/"); - HttpResponse response; + HttpResponse response(buffer); dispatcher.handle(request, response); @@ -100,4 +103,7 @@ TEST_CASE("Test methodNotAllowedHandler","[RouteDispatcher]") CHECK_THAT(response.getReason(), Equals("custom_not_allowed")); + // Check Allow header is set + CHECK_THAT(response.headers.get("Allow"), Equals("GET")); + } diff --git a/test/test_WebKernel.cpp b/test/test_WebKernel.cpp index 4784ccf..8ee8ea6 100644 --- a/test/test_WebKernel.cpp +++ b/test/test_WebKernel.cpp @@ -5,6 +5,8 @@ unsigned int foo; unsigned int bar; +char error_message[24] = {}; + void do_foo(HttpRequest& request, HttpResponse& response) { foo++; @@ -19,6 +21,18 @@ void do_bar(HttpRequest& request, HttpResponse& response) response.print("Bar"); } +void handle_notFound(HttpRequest& request, HttpResponse& response) +{ + response.code = 454; // unusual code + strcpy(error_message, "File Not Found"); +} + +void handle_methodNotAllowed(HttpRequest& request, HttpResponse& response) +{ + response.code = 455; //unusual code + strcpy(error_message, "Method Not Allowed"); +} + TEST_CASE("Test WebKernel", "[WebKernel]") { Route routes[] = { @@ -94,3 +108,8 @@ TEST_CASE("Test Partial requests", "[WebKernel]") REQUIRE(bar == 1); } } + +TEST_CASE("Test WebKernel error handling", "[WebKernel]") +{ + +}