Cut 4 string_view everything else

Use std::optional and std::string_view throughout the CGI parser and
core. Removes some of the hacks and tidies up some of the error
handling.

7 files changed, 104 insertions, 58 deletions

diff --git a/icespider/core/ihttpRequest.cpp b/icespider/core/ihttpRequest.cpp
index ea94fb2..cef738d 100644
--- a/icespider/core/ihttpRequest.cpp
+++ b/icespider/core/ihttpRequest.cpp
@@ -25,7 +25,7 @@ namespace IceSpider {
 	{
 		try {
 			return Slicer::StreamDeserializerFactory::createNew(
-				getEnv(E::CONTENT_TYPE) / []() -> std::string {
+				getEnv(E::CONTENT_TYPE) / []() -> std::string_view {
 					throw Http400_BadRequest();
 				}, getInputStream());
 		}
@@ -125,8 +125,8 @@ namespace IceSpider {
 	// Set-Cookie: value[; expires=date][; domain=domain][; path=path][; secure]
 	// Sat, 02 May 2009 23:38:25 GMT
 	void IHttpRequest::setCookie(const std::string_view & name, const std::string_view & value,
-			const Ice::optional<std::string> & d, const Ice::optional<std::string> & p, bool s,
-			Ice::optional<time_t> e)
+			const OptionalString & d, const OptionalString & p, bool s,
+			std::optional<time_t> e)
 	{
 		std::stringstream o;
 		o << XWwwFormUrlEncoded::urlencode(name) <<
@@ -146,13 +146,13 @@ namespace IceSpider {
 	}
 
 	template <typename T>
-	inline Ice::optional<T> optionalLexicalCast(const Ice::optional<std::string> & p)
+	inline std::optional<T> optionalLexicalCast(const OptionalString & p)
 	{
 		if (p) return wrapLexicalCast<T>(*p);
-		return Ice::optional<T>();
+		return {};
 	}
 
-	void IHttpRequest::responseRedirect(const std::string_view & url, const Ice::optional<std::string> & statusMsg) const
+	void IHttpRequest::responseRedirect(const std::string_view & url, const OptionalString & statusMsg) const
 	{
 		setHeader(H::LOCATION, url);
 		response(303, (statusMsg ? *statusMsg : S::MOVED));
@@ -167,29 +167,45 @@ namespace IceSpider {
 		s.second->Serialize(mp);
 	}
 
+	std::optional<std::string>
+	operator!(const std::optional<std::string_view> & sv)
+	{
+		// TODO: Fix this mess (only required if operation requires std::string)
+		if (sv) return { std::string(*sv) };
+		return {};
+	}
+
 #define getParams(T) \
 	template<> void IHttpRequest::setCookie<T>(const std::string_view & n, const T & v, \
-					const Ice::optional<std::string> & d, const Ice::optional<std::string> & p, \
-					bool s, Ice::optional<time_t> e) { \
+					const OptionalString & d, const OptionalString & p, \
+					bool s, std::optional<time_t> e) { \
 		auto vs = boost::lexical_cast<std::string>(v); \
 		setCookie(n, std::string_view(vs), d, p, s, e); \
 	} \
 	template<> T IHttpRequest::getURLParam<T>(unsigned int idx) const { \
 		return wrapLexicalCast<T>(getURLParam(idx)); } \
-	template<> Ice::optional<T> IHttpRequest::getQueryStringParam<T>(const std::string_view & key) const { \
+	template<> std::optional<T> IHttpRequest::getQueryStringParam<T>(const std::string_view & key) const { \
 		return optionalLexicalCast<T>(getQueryStringParam(key)); } \
-	template<> Ice::optional<T> IHttpRequest::getCookieParam<T>(const std::string_view & key) const { \
+	template<> std::optional<T> IHttpRequest::getCookieParam<T>(const std::string_view & key) const { \
 		return optionalLexicalCast<T>(getCookieParam(key)); } \
-	template<> Ice::optional<T> IHttpRequest::getHeaderParam<T>(const std::string_view & key) const { \
+	template<> std::optional<T> IHttpRequest::getHeaderParam<T>(const std::string_view & key) const { \
 		return optionalLexicalCast<T>(getHeaderParam(key)); }
-	template<> std::string IHttpRequest::getURLParam<std::string>(unsigned int idx) const {
+	template<> std::string_view IHttpRequest::getURLParam<std::string_view>(unsigned int idx) const {
 		return getURLParam(idx); }
-	template<> Ice::optional<std::string> IHttpRequest::getQueryStringParam<std::string>(const std::string_view & key) const { \
+	template<> OptionalString IHttpRequest::getQueryStringParam<std::string_view>(const std::string_view & key) const { \
 		return getQueryStringParam(key); }
-	template<> Ice::optional<std::string> IHttpRequest::getCookieParam<std::string>(const std::string_view & key) const { \
+	template<> OptionalString IHttpRequest::getCookieParam<std::string_view>(const std::string_view & key) const { \
 		return getCookieParam(key); }
-	template<> Ice::optional<std::string> IHttpRequest::getHeaderParam<std::string>(const std::string_view & key) const { \
+	template<> OptionalString IHttpRequest::getHeaderParam<std::string_view>(const std::string_view & key) const { \
 		return getHeaderParam(key); }
+	template<> std::string IHttpRequest::getURLParam<std::string>(unsigned int idx) const {
+		return getURLParam(idx); }
+	template<> std::optional<std::string> IHttpRequest::getQueryStringParam<std::string>(const std::string_view & key) const { \
+		return !getQueryStringParam(key); }
+	template<> std::optional<std::string> IHttpRequest::getCookieParam<std::string>(const std::string_view & key) const { \
+		return !getCookieParam(key); }
+	template<> std::optional<std::string> IHttpRequest::getHeaderParam<std::string>(const std::string_view & key) const { \
+		return !getHeaderParam(key); }
 
 	getParams(bool);
 	getParams(Ice::Short);
diff --git a/icespider/core/ihttpRequest.h b/icespider/core/ihttpRequest.h
index fdc3b5d..aaa9d50 100644
--- a/icespider/core/ihttpRequest.h
+++ b/icespider/core/ihttpRequest.h
@@ -15,7 +15,7 @@ namespace IceSpider {
 	class IRouteHandler;
 
 	typedef std::vector<std::string> PathElements;
-	typedef Ice::optional<std::string> OptionalString;
+	typedef std::optional<std::string_view> OptionalString;
 	typedef std::pair<MimeType, Slicer::SerializerPtr> ContentTypeSerializer;
 
 	class DLL_PUBLIC IHttpRequest {
@@ -44,37 +44,37 @@ namespace IceSpider {
 			template<typename T>
 			T getURLParam(unsigned int) const;
 			template<typename T>
-			Ice::optional<T> getBody() const
+			std::optional<T> getBody() const
 			{
 				return Slicer::DeserializeAnyWith<T>(getDeserializer());
 			}
 			template<typename T>
-			Ice::optional<T> getBodyParam(const Ice::optional<IceSpider::StringMap> & map, const std::string_view & key) const
+			std::optional<T> getBodyParam(const Ice::optional<IceSpider::StringMap> & map, const std::string_view & key) const
 			{
 				if (!map) {
-					return Ice::optional<T>();
+					return {};
 				}
 				auto i = map->find(key);
 				if (i == map->end()) {
-					return Ice::optional<T>();
+					return {};
 				}
 				else {
 					return boost::lexical_cast<T>(i->second);
 				}
 			}
-			void responseRedirect(const std::string_view & url, const Ice::optional<std::string> & = IceUtil::None) const;
+			void responseRedirect(const std::string_view & url, const OptionalString & = {}) const;
 			void setCookie(const std::string_view &, const std::string_view &,
-					const Ice::optional<std::string> & = IceUtil::None, const Ice::optional<std::string> & = IceUtil::None,
-					bool = false, Ice::optional<time_t> = IceUtil::None);
+					const OptionalString & = {}, const OptionalString & = {},
+					bool = false, std::optional<time_t> = {});
 			template<typename T>
-			void setCookie(const std::string_view &, const T &, const Ice::optional<std::string> & = IceUtil::None,
-					const Ice::optional<std::string> & = IceUtil::None, bool = false, Ice::optional<time_t> = IceUtil::None);
+			void setCookie(const std::string_view &, const T &, const OptionalString & = {},
+					const OptionalString & = {}, bool = false, std::optional<time_t> = {});
 			template<typename T>
-			Ice::optional<T> getQueryStringParam(const std::string_view & key) const;
+			std::optional<T> getQueryStringParam(const std::string_view & key) const;
 			template<typename T>
-			Ice::optional<T> getHeaderParam(const std::string_view & key) const;
+			std::optional<T> getHeaderParam(const std::string_view & key) const;
 			template<typename T>
-			Ice::optional<T> getCookieParam(const std::string_view & key) const;
+			std::optional<T> getCookieParam(const std::string_view & key) const;
 			virtual void response(short, const std::string_view &) const = 0;
 			template<typename T>
 			void response(const IRouteHandler * route, const T & t) const
diff --git a/icespider/core/util.h b/icespider/core/util.h
index 658a1cc..0edc3d2 100644
--- a/icespider/core/util.h
+++ b/icespider/core/util.h
@@ -13,6 +13,15 @@ namespace std::experimental::Ice {
 	}
 }
 
+namespace std {
+	template <typename T, typename TF>
+	auto operator/(const std::optional<T> & o, const TF & tf) -> decltype(tf())
+	{
+		if (o) return *o;
+		return tf();
+	}
+}
+
 template <typename T>
 T orelse(const T & a, const T & b)
 {
diff --git a/icespider/fcgi/cgiRequestBase.cpp b/icespider/fcgi/cgiRequestBase.cpp
index a42a78f..5bcb746 100644
--- a/icespider/fcgi/cgiRequestBase.cpp
+++ b/icespider/fcgi/cgiRequestBase.cpp
@@ -3,6 +3,7 @@
 #include <boost/algorithm/string/case_conv.hpp>
 #include <boost/algorithm/string/split.hpp>
 #include <boost/algorithm/string/classification.hpp>
+#include <boost/algorithm/string/predicate.hpp>
 #include <util.h>
 #include <slicer/modelPartsTypes.h>
 #include <slicer/common.h>
@@ -14,7 +15,9 @@ using namespace std::literals;
 #define CGI_CONST(NAME) static const std::string_view NAME(#NAME)
 
 namespace IceSpider {
-	static const std::string HEADER_PREFIX("HTTP_");
+	static const std::string_view amp("&");
+	static const std::string_view semi("; ");
+	static const std::string_view HEADER_PREFIX("HTTP_");
 	CGI_CONST(REDIRECT_URL);
 	CGI_CONST(SCRIPT_NAME);
 	CGI_CONST(QUERY_STRING);
@@ -37,20 +40,13 @@ namespace IceSpider {
 		}
 	}
 
-	inline
-	std::string
-	operator*(const std::string_view & t)
-	{
-		return std::string(t);
-	}
-
 	template<typename in, typename out>
 	inline
 	void
 	mapVars(const std::string_view & vn, const in & envmap, out & map, const std::string_view & sp) {
 		auto qs = envmap.find(vn);
 		if (qs != envmap.end()) {
-			XWwwFormUrlEncoded::iterateVars(qs->second, [&map](auto k, auto v) {
+			XWwwFormUrlEncoded::iterateVars(qs->second, [&map](const auto & k, const auto & v) {
 				map.insert({ k, v });
 			}, sp);
 		}
@@ -58,18 +54,18 @@ namespace IceSpider {
 
 	template<typename Ex, typename Map>
 	const std::string_view &
-	findFirstOrElse(const Map &, const std::string & msg)
+	findFirstOrElse(const Map &)
 	{
-		throw Ex(msg);
+		throw Ex();
 	}
 
 	template<typename Ex, typename Map, typename ... Ks>
 	const std::string_view &
-	findFirstOrElse(const Map & map, const std::string & msg, const std::string_view & k, const Ks & ... ks)
+	findFirstOrElse(const Map & map, const std::string_view & k, const Ks & ... ks)
 	{
 		auto i = map.find(k);
 		if (i == map.end()) {
-			return findFirstOrElse<Ex>(map, msg, ks...);
+			return findFirstOrElse<Ex>(map, ks...);
 		}
 		return i->second;
 	}
@@ -78,15 +74,18 @@ namespace IceSpider {
 	CgiRequestBase::initialize()
 	{
 		namespace ba = boost::algorithm;
-		auto path = findFirstOrElse<std::runtime_error>(envmap, "Couldn't determine request path"s,
-				REDIRECT_URL,
-				SCRIPT_NAME).substr(1);
-		if (!path.empty()) {
+		if (auto path = findFirstOrElse<Http400_BadRequest>(envmap, REDIRECT_URL, SCRIPT_NAME).substr(1);
+				!path.empty()) {
 			ba::split(pathElements, path, ba::is_any_of("/"), ba::token_compress_off);
 		}
 
-		mapVars(QUERY_STRING, envmap, qsmap, "&"sv);
-		mapVars(HTTP_COOKIE, envmap, cookiemap, "; "sv);
+		mapVars(QUERY_STRING, envmap, qsmap, amp);
+		mapVars(HTTP_COOKIE, envmap, cookiemap, semi);
+		for (auto header = envmap.lower_bound(HEADER_PREFIX);
+				header != envmap.end() && ba::starts_with(header->first, HEADER_PREFIX);
+				header++) {
+			hdrmap.insert({ header->first.substr(HEADER_PREFIX.length()), header->second });
+		}
 	}
 
 	AdHocFormatter(VarFmt, "\t%?: [%?]\n");
@@ -119,9 +118,9 @@ namespace IceSpider {
 	{
 		auto i = vm.find(key);
 		if (i == vm.end()) {
-			return IceUtil::None;
+			return {};
 		}
-		return *i->second;
+		return i->second;
 	}
 
 	OptionalString
@@ -129,7 +128,7 @@ namespace IceSpider {
 	{
 		auto i = vm.find(key);
 		if (i == vm.end()) {
-			return IceUtil::None;
+			return {};
 		}
 		return i->second;
 	}
@@ -151,7 +150,10 @@ namespace IceSpider {
 	{
 		try {
 			auto i = envmap.find(REQUEST_METHOD);
-			return Slicer::ModelPartForEnum<HttpMethod>::lookup(*i->second);
+			if (i == envmap.end()) {
+				throw IceSpider::Http400_BadRequest();
+			}
+			return Slicer::ModelPartForEnum<HttpMethod>::lookup(i->second);
 		}
 		catch (const Slicer::InvalidEnumerationSymbol &) {
 			throw IceSpider::Http405_MethodNotAllowed();
@@ -179,10 +181,7 @@ namespace IceSpider {
 	OptionalString
 	CgiRequestBase::getHeaderParam(const std::string_view & key) const
 	{
-		// TODO: Fix this mess
-		std::string ks(key);
-		boost::algorithm::to_upper(ks);
-		return optionalLookup(HEADER_PREFIX + ks, envmap);
+		return optionalLookup(key, hdrmap);
 	}
 
 	void CgiRequestBase::response(short statusCode, const std::string_view & statusMsg) const
diff --git a/icespider/fcgi/cgiRequestBase.h b/icespider/fcgi/cgiRequestBase.h
index efe2746..0ceb201 100644
--- a/icespider/fcgi/cgiRequestBase.h
+++ b/icespider/fcgi/cgiRequestBase.h
@@ -37,6 +37,7 @@ namespace IceSpider {
 			VarMap envmap;
 			StringMap qsmap;
 			StringMap cookiemap;
+			VarMap hdrmap;
 			PathElements pathElements;
 	};
 }
diff --git a/icespider/testing/testRequest.cpp b/icespider/testing/testRequest.cpp
index 11fd721..1c0d9cb 100644
--- a/icespider/testing/testRequest.cpp
+++ b/icespider/testing/testRequest.cpp
@@ -62,7 +62,7 @@ namespace IceSpider {
 	{
 		auto i = vars.find(key);
 		if (i == vars.end()) {
-			return IceUtil::None;
+			return {};
 		}
 		return i->second;
 	}
diff --git a/icespider/unittests/testFcgi.cpp b/icespider/unittests/testFcgi.cpp
index 6d3d7d5..60f2aa7 100644
--- a/icespider/unittests/testFcgi.cpp
+++ b/icespider/unittests/testFcgi.cpp
@@ -10,7 +10,7 @@
 using namespace std::literals;
 namespace std {
 	template<typename T>
-	ostream & operator<<(ostream & s, const Ice::optional<T> & o) {
+	ostream & operator<<(ostream & s, const std::optional<T> & o) {
 		if (o) s << *o;
 		return s;
 	}
@@ -112,7 +112,7 @@ BOOST_AUTO_TEST_CASE( NoEnvironment )
 	BOOST_REQUIRE_THROW({
 		CharPtrPtrArray env;
 		TestRequest r(this, env);
-	}, std::runtime_error);
+	}, IceSpider::Http400_BadRequest);
 }
 
 BOOST_AUTO_TEST_CASE( script_name_root )
@@ -220,6 +220,27 @@ BOOST_AUTO_TEST_CASE( requestmethod_bad )
 	BOOST_REQUIRE_THROW(r.getRequestMethod(), IceSpider::Http405_MethodNotAllowed);
 }
 
+BOOST_AUTO_TEST_CASE( requestmethod_missing )
+{
+	CharPtrPtrArray env ({ "SCRIPT_NAME=/" });
+	TestRequest r(this, env);
+	BOOST_REQUIRE_THROW(r.getRequestMethod(), IceSpider::Http400_BadRequest);
+}
+
+BOOST_AUTO_TEST_CASE( acceptheader )
+{
+	CharPtrPtrArray env ({ "SCRIPT_NAME=/", "HTTP_ACCEPT=text/html" });
+	TestRequest r(this, env);
+	BOOST_REQUIRE_EQUAL("text/html", *r.getHeaderParam("ACCEPT"));
+}
+
+BOOST_AUTO_TEST_CASE( missingheader )
+{
+	CharPtrPtrArray env ({ "SCRIPT_NAME=/", "HTTP_ACCEPT=text/html" });
+	TestRequest r(this, env);
+	BOOST_REQUIRE(!r.getHeaderParam("ACCEPT_LANGUAGE"));
+}
+
 BOOST_AUTO_TEST_CASE( postxwwwformurlencoded_simple )
 {
 	CharPtrPtrArray env ({ "SCRIPT_NAME=/", "REQUEST_METHOD=No", "CONTENT_TYPE=application/x-www-form-urlencoded" });