Bad request on missing or invalid parameters

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 <typename T, typename Y>
+	inline T wrapLexicalCast(const Y & y)
+	{
+		try {
+			return boost::lexical_cast<T>(y);
+		}
+		catch (const boost::bad_lexical_cast &) {
+			throw Http400_BadRequest();
+		}
+	}
+
 	template <typename T>
 	inline IceUtil::Optional<T> optionalLexicalCast(const IceUtil::Optional<std::string> & p)
 	{
-		if (p) return boost::lexical_cast<T>(*p);
+		if (p) return wrapLexicalCast<T>(*p);
 		return IceUtil::Optional<T>();
 	}
 
@@ -99,7 +110,7 @@ namespace IceSpider {
 
 #define getParams(T) \
 	template<> T IHttpRequest::getURLParam<T>(unsigned int idx) const { \
-		return boost::lexical_cast<T>(getURLParam(idx)); } \
+		return wrapLexicalCast<T>(getURLParam(idx)); } \
 	template<> IceUtil::Optional<T> IHttpRequest::getQueryStringParam<T>(const std::string & key) const { \
 		return optionalLexicalCast<T>(getQueryStringParam(key)); } \
 	template<> IceUtil::Optional<T> IHttpRequest::getHeaderParam<T>(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
@@ -41,6 +41,12 @@ namespace IceSpider {
 	}
 
 	void
+	IRouteHandler::requiredParameterNotFound(const char *, const std::string &) const
+	{
+		throw Http400_BadRequest();
+	}
+
+	void
 	IRouteHandler::addRouteSerializer(const MimeType & ct, StreamSerializerFactoryPtr ssfp)
 	{
 		routeSerializers.erase(ct);
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 <pathparts.h>
 #include <routes.h>
 #include <factory.h>
@@ -28,11 +29,13 @@ namespace IceSpider {
 			typedef std::map<MimeType, StreamSerializerFactoryPtr> RouteSerializers;
 			RouteSerializers routeSerializers;
 
+			void requiredParameterNotFound(const char *, const std::string & key) const;
+
 			template <typename T, typename K>
-			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<std::string>(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<IceSpider::RouteHandlerFactory>().size());
+	BOOST_REQUIRE_EQUAL(10, AdHoc::PluginManager::getDefault()->getAll<IceSpider::RouteHandlerFactory>().size());
 }
 
 class TestRequest : public IHttpRequest {
@@ -55,7 +55,7 @@ class TestRequest : public IHttpRequest {
 
 		IceUtil::Optional<std::string> getQueryStringParam(const std::string & key) const override
 		{
-			return AdHoc::safeMapLookup<std::runtime_error>(qs, key);
+			return qs.find(key) == qs.end() ? IceUtil::Optional<std::string>() : qs.find(key)->second;
 		}
 
 		IceUtil::Optional<std::string> 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<Slicer::JsonStreamDeserializer, TestIceSpider::SomeModelPtr>(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<IceSpider::RouteHandlerFactory>().size());
+	BOOST_REQUIRE_EQUAL(10, AdHoc::PluginManager::getDefault()->getAll<IceSpider::RouteHandlerFactory>().size());
 	// smoke test (block ensure dlclose dones't cause segfault)
 	{
 		auto route = AdHoc::PluginManager::getDefault()->get<IceSpider::RouteHandlerFactory>("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": [