From a3b011c92111c144cdbc726a6e1458ca25eb807f Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Sat, 17 Sep 2016 03:04:57 +0100 Subject: Bad request on missing or invalid parameters --- icespider/core/ihttpRequest.cpp | 15 +++++++++-- icespider/core/irouteHandler.cpp | 6 +++++ icespider/core/irouteHandler.h | 9 ++++--- icespider/unittests/testApp.cpp | 51 ++++++++++++++++++++++++++++++++++--- icespider/unittests/testCompile.cpp | 4 +-- icespider/unittests/testRoutes.json | 16 ++++++++++++ 6 files changed, 91 insertions(+), 10 deletions(-) diff --git a/icespider/core/ihttpRequest.cpp b/icespider/core/ihttpRequest.cpp index d9abecd..ea2c0bd 100644 --- a/icespider/core/ihttpRequest.cpp +++ b/icespider/core/ihttpRequest.cpp @@ -82,10 +82,21 @@ namespace IceSpider { return url[idx]; } + template + inline T wrapLexicalCast(const Y & y) + { + try { + return boost::lexical_cast(y); + } + catch (const boost::bad_lexical_cast &) { + throw Http400_BadRequest(); + } + } + template inline IceUtil::Optional optionalLexicalCast(const IceUtil::Optional & p) { - if (p) return boost::lexical_cast(*p); + if (p) return wrapLexicalCast(*p); return IceUtil::Optional(); } @@ -99,7 +110,7 @@ namespace IceSpider { #define getParams(T) \ template<> T IHttpRequest::getURLParam(unsigned int idx) const { \ - return boost::lexical_cast(getURLParam(idx)); } \ + return wrapLexicalCast(getURLParam(idx)); } \ template<> IceUtil::Optional IHttpRequest::getQueryStringParam(const std::string & key) const { \ return optionalLexicalCast(getQueryStringParam(key)); } \ template<> IceUtil::Optional IHttpRequest::getHeaderParam(const std::string & key) const { \ diff --git a/icespider/core/irouteHandler.cpp b/icespider/core/irouteHandler.cpp index 9d33a6b..6681697 100644 --- a/icespider/core/irouteHandler.cpp +++ b/icespider/core/irouteHandler.cpp @@ -40,6 +40,12 @@ namespace IceSpider { }; } + void + IRouteHandler::requiredParameterNotFound(const char *, const std::string &) const + { + throw Http400_BadRequest(); + } + void IRouteHandler::addRouteSerializer(const MimeType & ct, StreamSerializerFactoryPtr ssfp) { diff --git a/icespider/core/irouteHandler.h b/icespider/core/irouteHandler.h index e75ec8f..595ea95 100644 --- a/icespider/core/irouteHandler.h +++ b/icespider/core/irouteHandler.h @@ -3,6 +3,7 @@ #include "ihttpRequest.h" #include "util.h" +#include "exceptions.h" #include #include #include @@ -28,11 +29,13 @@ namespace IceSpider { typedef std::map RouteSerializers; RouteSerializers routeSerializers; + void requiredParameterNotFound(const char *, const std::string & key) const; + template - inline T requiredParameterNotFound(const char *, const K & key) const + inline T requiredParameterNotFound(const char * s, const K & key) const { - throw std::runtime_error("Required parameter not found: " + - boost::lexical_cast(key)); + requiredParameterNotFound(s, key); + return T(); } void addRouteSerializer(const MimeType &, StreamSerializerFactoryPtr); diff --git a/icespider/unittests/testApp.cpp b/icespider/unittests/testApp.cpp index 80cecda..0806542 100644 --- a/icespider/unittests/testApp.cpp +++ b/icespider/unittests/testApp.cpp @@ -27,7 +27,7 @@ void forceEarlyChangeDir() BOOST_AUTO_TEST_CASE( testLoadConfiguration ) { - BOOST_REQUIRE_EQUAL(9, AdHoc::PluginManager::getDefault()->getAll().size()); + BOOST_REQUIRE_EQUAL(10, AdHoc::PluginManager::getDefault()->getAll().size()); } class TestRequest : public IHttpRequest { @@ -55,7 +55,7 @@ class TestRequest : public IHttpRequest { IceUtil::Optional getQueryStringParam(const std::string & key) const override { - return AdHoc::safeMapLookup(qs, key); + return qs.find(key) == qs.end() ? IceUtil::Optional() : qs.find(key)->second; } IceUtil::Optional getHeaderParam(const std::string & key) const override @@ -90,7 +90,7 @@ BOOST_AUTO_TEST_CASE( testCoreSettings ) { BOOST_REQUIRE_EQUAL(5, routes.size()); BOOST_REQUIRE_EQUAL(1, routes[0].size()); - BOOST_REQUIRE_EQUAL(3, routes[1].size()); + BOOST_REQUIRE_EQUAL(4, routes[1].size()); BOOST_REQUIRE_EQUAL(1, routes[2].size()); BOOST_REQUIRE_EQUAL(2, routes[3].size()); BOOST_REQUIRE_EQUAL(2, routes[4].size()); @@ -430,5 +430,50 @@ BOOST_AUTO_TEST_CASE( testCall405 ) BOOST_REQUIRE(requestGetIndex.output.eof()); } +BOOST_AUTO_TEST_CASE( testCallSearch ) +{ + TestRequest request(this, HttpMethod::GET, "/search"); + request.qs["s"] = "something"; + request.qs["i"] = "1234"; + process(&request); + auto h = parseHeaders(request.output); + BOOST_REQUIRE_EQUAL(h["Status"], "200 OK"); + Slicer::DeserializeAny(request.output); +} + +BOOST_AUTO_TEST_CASE( testCallSearchBadLexicalCast ) +{ + TestRequest request(this, HttpMethod::GET, "/search"); + request.qs["s"] = "something"; + request.qs["i"] = "bar"; + process(&request); + auto h = parseHeaders(request.output); + BOOST_REQUIRE_EQUAL(h["Status"], "400 Bad Request"); + request.output.get(); + BOOST_REQUIRE(request.output.eof()); +} + +BOOST_AUTO_TEST_CASE( testCallSearchMissingS ) +{ + TestRequest request(this, HttpMethod::GET, "/search"); + request.qs["i"] = "1234"; + process(&request); + auto h = parseHeaders(request.output); + BOOST_REQUIRE_EQUAL(h["Status"], "400 Bad Request"); + request.output.get(); + BOOST_REQUIRE(request.output.eof()); +} + +BOOST_AUTO_TEST_CASE( testCallSearchMissingI ) +{ + TestRequest request(this, HttpMethod::GET, "/search"); + request.qs["s"] = "something"; + process(&request); + auto h = parseHeaders(request.output); + BOOST_REQUIRE_EQUAL(h["Status"], "400 Bad Request"); + request.output.get(); + BOOST_REQUIRE(request.output.eof()); +} + BOOST_AUTO_TEST_SUITE_END(); diff --git a/icespider/unittests/testCompile.cpp b/icespider/unittests/testCompile.cpp index 425821f..063d691 100644 --- a/icespider/unittests/testCompile.cpp +++ b/icespider/unittests/testCompile.cpp @@ -36,7 +36,7 @@ BOOST_AUTO_TEST_CASE( testLoadConfiguration ) rc.applyDefaults(cfg, u); BOOST_REQUIRE_EQUAL("common", cfg->name); - BOOST_REQUIRE_EQUAL(9, cfg->routes.size()); + BOOST_REQUIRE_EQUAL(10, cfg->routes.size()); BOOST_REQUIRE_EQUAL("index", cfg->routes[0]->name); BOOST_REQUIRE_EQUAL("/", cfg->routes[0]->path); @@ -132,7 +132,7 @@ BOOST_AUTO_TEST_CASE( testLoad ) BOOST_TEST_INFO(dlerror()); BOOST_REQUIRE(lib); - BOOST_REQUIRE_EQUAL(9, AdHoc::PluginManager::getDefault()->getAll().size()); + BOOST_REQUIRE_EQUAL(10, AdHoc::PluginManager::getDefault()->getAll().size()); // smoke test (block ensure dlclose dones't cause segfault) { auto route = AdHoc::PluginManager::getDefault()->get("common::index"); diff --git a/icespider/unittests/testRoutes.json b/icespider/unittests/testRoutes.json index d2d713a..daf090d 100644 --- a/icespider/unittests/testRoutes.json +++ b/icespider/unittests/testRoutes.json @@ -115,6 +115,22 @@ } ], "operation": "TestIceSpider.TestApi.index" + }, + { + "name": "search", + "path": "/search", + "method": "GET", + "operation": "TestIceSpider.TestApi.withParams", + "params": [ + { + "name": "s", + "source": "QueryString" + }, + { + "name": "i", + "source": "QueryString" + } + ] } ], "slices": [ -- cgit v1.2.3