From 23f95e1c99b3983596048a65d0fef5b02c91e8ba Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Wed, 16 May 2018 08:20:09 +0100 Subject: 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. --- icespider/core/ihttpRequest.cpp | 46 ++++++++++++++++++++++----------- icespider/core/ihttpRequest.h | 26 +++++++++---------- icespider/core/util.h | 9 +++++++ icespider/fcgi/cgiRequestBase.cpp | 53 +++++++++++++++++++-------------------- icespider/fcgi/cgiRequestBase.h | 1 + icespider/testing/testRequest.cpp | 2 +- icespider/unittests/testFcgi.cpp | 25 ++++++++++++++++-- 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 & d, const Ice::optional & p, bool s, - Ice::optional e) + const OptionalString & d, const OptionalString & p, bool s, + std::optional e) { std::stringstream o; o << XWwwFormUrlEncoded::urlencode(name) << @@ -146,13 +146,13 @@ namespace IceSpider { } template - inline Ice::optional optionalLexicalCast(const Ice::optional & p) + inline std::optional optionalLexicalCast(const OptionalString & p) { if (p) return wrapLexicalCast(*p); - return Ice::optional(); + return {}; } - void IHttpRequest::responseRedirect(const std::string_view & url, const Ice::optional & 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 + operator!(const std::optional & 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(const std::string_view & n, const T & v, \ - const Ice::optional & d, const Ice::optional & p, \ - bool s, Ice::optional e) { \ + const OptionalString & d, const OptionalString & p, \ + bool s, std::optional e) { \ auto vs = boost::lexical_cast(v); \ setCookie(n, std::string_view(vs), d, p, s, e); \ } \ template<> T IHttpRequest::getURLParam(unsigned int idx) const { \ return wrapLexicalCast(getURLParam(idx)); } \ - template<> Ice::optional IHttpRequest::getQueryStringParam(const std::string_view & key) const { \ + template<> std::optional IHttpRequest::getQueryStringParam(const std::string_view & key) const { \ return optionalLexicalCast(getQueryStringParam(key)); } \ - template<> Ice::optional IHttpRequest::getCookieParam(const std::string_view & key) const { \ + template<> std::optional IHttpRequest::getCookieParam(const std::string_view & key) const { \ return optionalLexicalCast(getCookieParam(key)); } \ - template<> Ice::optional IHttpRequest::getHeaderParam(const std::string_view & key) const { \ + template<> std::optional IHttpRequest::getHeaderParam(const std::string_view & key) const { \ return optionalLexicalCast(getHeaderParam(key)); } - template<> std::string IHttpRequest::getURLParam(unsigned int idx) const { + template<> std::string_view IHttpRequest::getURLParam(unsigned int idx) const { return getURLParam(idx); } - template<> Ice::optional IHttpRequest::getQueryStringParam(const std::string_view & key) const { \ + template<> OptionalString IHttpRequest::getQueryStringParam(const std::string_view & key) const { \ return getQueryStringParam(key); } - template<> Ice::optional IHttpRequest::getCookieParam(const std::string_view & key) const { \ + template<> OptionalString IHttpRequest::getCookieParam(const std::string_view & key) const { \ return getCookieParam(key); } - template<> Ice::optional IHttpRequest::getHeaderParam(const std::string_view & key) const { \ + template<> OptionalString IHttpRequest::getHeaderParam(const std::string_view & key) const { \ return getHeaderParam(key); } + template<> std::string IHttpRequest::getURLParam(unsigned int idx) const { + return getURLParam(idx); } + template<> std::optional IHttpRequest::getQueryStringParam(const std::string_view & key) const { \ + return !getQueryStringParam(key); } + template<> std::optional IHttpRequest::getCookieParam(const std::string_view & key) const { \ + return !getCookieParam(key); } + template<> std::optional IHttpRequest::getHeaderParam(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 PathElements; - typedef Ice::optional OptionalString; + typedef std::optional OptionalString; typedef std::pair ContentTypeSerializer; class DLL_PUBLIC IHttpRequest { @@ -44,37 +44,37 @@ namespace IceSpider { template T getURLParam(unsigned int) const; template - Ice::optional getBody() const + std::optional getBody() const { return Slicer::DeserializeAnyWith(getDeserializer()); } template - Ice::optional getBodyParam(const Ice::optional & map, const std::string_view & key) const + std::optional getBodyParam(const Ice::optional & map, const std::string_view & key) const { if (!map) { - return Ice::optional(); + return {}; } auto i = map->find(key); if (i == map->end()) { - return Ice::optional(); + return {}; } else { return boost::lexical_cast(i->second); } } - void responseRedirect(const std::string_view & url, const Ice::optional & = 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 & = IceUtil::None, const Ice::optional & = IceUtil::None, - bool = false, Ice::optional = IceUtil::None); + const OptionalString & = {}, const OptionalString & = {}, + bool = false, std::optional = {}); template - void setCookie(const std::string_view &, const T &, const Ice::optional & = IceUtil::None, - const Ice::optional & = IceUtil::None, bool = false, Ice::optional = IceUtil::None); + void setCookie(const std::string_view &, const T &, const OptionalString & = {}, + const OptionalString & = {}, bool = false, std::optional = {}); template - Ice::optional getQueryStringParam(const std::string_view & key) const; + std::optional getQueryStringParam(const std::string_view & key) const; template - Ice::optional getHeaderParam(const std::string_view & key) const; + std::optional getHeaderParam(const std::string_view & key) const; template - Ice::optional getCookieParam(const std::string_view & key) const; + std::optional getCookieParam(const std::string_view & key) const; virtual void response(short, const std::string_view &) const = 0; template 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 + auto operator/(const std::optional & o, const TF & tf) -> decltype(tf()) + { + if (o) return *o; + return tf(); + } +} + template 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 #include #include +#include #include #include #include @@ -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 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 const std::string_view & - findFirstOrElse(const Map &, const std::string & msg) + findFirstOrElse(const Map &) { - throw Ex(msg); + throw Ex(); } template 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(map, msg, ks...); + return findFirstOrElse(map, ks...); } return i->second; } @@ -78,15 +74,18 @@ namespace IceSpider { CgiRequestBase::initialize() { namespace ba = boost::algorithm; - auto path = findFirstOrElse(envmap, "Couldn't determine request path"s, - REDIRECT_URL, - SCRIPT_NAME).substr(1); - if (!path.empty()) { + if (auto path = findFirstOrElse(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::lookup(*i->second); + if (i == envmap.end()) { + throw IceSpider::Http400_BadRequest(); + } + return Slicer::ModelPartForEnum::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 - ostream & operator<<(ostream & s, const Ice::optional & o) { + ostream & operator<<(ostream & s, const std::optional & 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" }); -- cgit v1.2.3