From e6a1684c6ddf55f58e6b385f86bd028f67141aff Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Mon, 6 Sep 2021 19:00:05 +0100 Subject: Non-allocating, non-FILE based, view based Accept parser --- icespider/common/http.ice | 9 ----- icespider/core/ihttpRequest.cpp | 76 +++++++++++++++++++++----------------- icespider/core/ihttpRequest.h | 7 +++- icespider/core/irouteHandler.cpp | 4 +- icespider/core/irouteHandler.h | 2 +- icespider/unittests/testAccept.cpp | 28 +++++++------- icespider/unittests/testPerf.cpp | 19 ++++++++++ 7 files changed, 84 insertions(+), 61 deletions(-) diff --git a/icespider/common/http.ice b/icespider/common/http.ice index 35c518d..4ec6fdd 100644 --- a/icespider/common/http.ice +++ b/icespider/common/http.ice @@ -24,15 +24,6 @@ module IceSpider { string type; }; - ["slicer:ignore"] - local class Accept { - optional(0) string group; - optional(1) string type; - float q = 1.0; - }; - ["slicer:ignore"] - local sequence Accepted; - ["slicer:json:object", "cpp:type:std::map>"] local dictionary StringMap; diff --git a/icespider/core/ihttpRequest.cpp b/icespider/core/ihttpRequest.cpp index 025509f..9459f60 100644 --- a/icespider/core/ihttpRequest.cpp +++ b/icespider/core/ihttpRequest.cpp @@ -35,57 +35,65 @@ namespace IceSpider { } Accepted - IHttpRequest::parseAccept(const std::string_view & acceptHdr) + IHttpRequest::parseAccept(std::string_view acceptHdr) { - if (acceptHdr.empty() - || std::find_if_not(acceptHdr.begin(), acceptHdr.end(), std::iswspace) == acceptHdr.end()) { + remove_leading(acceptHdr, ' '); + if (acceptHdr.empty()) { throw Http400_BadRequest(); } - auto accept = std::unique_ptr( - fmemopen(const_cast(acceptHdr.data()), acceptHdr.length(), "r"), &fclose); Accepted accepts; - accepts.reserve(acceptHdr.length() / 15); - int grpstart, grpend, typestart, typeend; - auto reset = [&]() { - grpstart = grpend = typestart = typeend = 0; - return ftell(accept.get()); - }; + accepts.reserve(std::count(acceptHdr.begin(), acceptHdr.end(), ',') + 1); - for (auto off = reset(); - // NOLINTNEXTLINE(hicpp-vararg) - fscanf(accept.get(), " %n%*[^ /]%n / %n%*[^ ;,]%n", &grpstart, &grpend, &typestart, &typeend) == 0; - off = reset()) { - if (grpend <= grpstart || typestart <= grpend || typeend <= typestart) { + auto upto = [](std::string_view & in, const std::string_view term, bool req) { + remove_leading(in, ' '); + const auto end = in.find_first_of(term); + if (req && end == std::string_view::npos) { + throw Http400_BadRequest(); + } + auto val = in.substr(0, end); + remove_trailing(val, ' '); + if (val.empty()) { throw Http400_BadRequest(); } - auto a = std::make_shared(); - if (auto g = acceptHdr.substr(typestart + off, typeend - typestart); g != "*") { - a->type.emplace(g); + if (end != std::string_view::npos) { + const auto tc = in[end]; + in.remove_prefix(end + 1); + return std::make_pair(val, tc); + } + else { + in.remove_prefix(in.length()); + return std::make_pair(val, '\n'); + } + }; + + while (!acceptHdr.empty()) { + const auto group = upto(acceptHdr, "/", true).first; + const auto [type, tc] = upto(acceptHdr, ";,", false); + Accept a; + if (type != "*") { + a.type.emplace(type); } - if (auto t = acceptHdr.substr(grpstart + off, grpend - grpstart); t != "*") { - a->group.emplace(t); + if (group != "*") { + a.group.emplace(group); } - else if (a->type) { + else if (a.type) { throw Http400_BadRequest(); } - // NOLINTNEXTLINE(hicpp-vararg) - if (fscanf(accept.get(), " ; q = %f ", &a->q) == 1) { - if (a->q <= 0.0F || a->q > 1.0F) { + if (tc == ';') { + if (upto(acceptHdr, "=", true).first != "q") { + throw Http400_BadRequest(); + } + const auto qs = upto(acceptHdr, ",", false).first; + a.q = std::atof(std::string(qs).c_str()); + if (a.q <= 0.0F || a.q > 1.0F) { throw Http400_BadRequest(); } } - accepts.push_back(a); - // NOLINTNEXTLINE(hicpp-vararg) - if (fscanf(accept.get(), " ,") != 0) { - break; - } + accepts.push_back(std::move(a)); } - if (accepts.empty()) { - throw Http400_BadRequest(); - } std::stable_sort(accepts.begin(), accepts.end(), [](const auto & a, const auto & b) { - return a->q > b->q; + return a.q > b.q; }); return accepts; } diff --git a/icespider/core/ihttpRequest.h b/icespider/core/ihttpRequest.h index b4bab30..a54eae3 100644 --- a/icespider/core/ihttpRequest.h +++ b/icespider/core/ihttpRequest.h @@ -15,6 +15,11 @@ namespace IceSpider { class Core; class IRouteHandler; + struct Accept { + std::optional group, type; + float q {1.0F}; + }; + using Accepted = std::vector; using PathElements = std::vector; using OptionalString = std::optional; using ContentTypeSerializer = std::pair; @@ -34,7 +39,7 @@ namespace IceSpider { [[nodiscard]] virtual OptionalString getCookieParam(const std::string_view &) const = 0; [[nodiscard]] virtual OptionalString getEnv(const std::string_view &) const = 0; [[nodiscard]] virtual bool isSecure() const = 0; - [[nodiscard]] static Accepted parseAccept(const std::string_view &); + [[nodiscard]] static Accepted parseAccept(std::string_view); [[nodiscard]] virtual Slicer::DeserializerPtr getDeserializer() const; [[nodiscard]] virtual ContentTypeSerializer getSerializer(const IRouteHandler *) const; [[nodiscard]] virtual std::istream & getInputStream() const = 0; diff --git a/icespider/core/irouteHandler.cpp b/icespider/core/irouteHandler.cpp index b968397..fc49aff 100644 --- a/icespider/core/irouteHandler.cpp +++ b/icespider/core/irouteHandler.cpp @@ -27,10 +27,10 @@ namespace IceSpider { } ContentTypeSerializer - IRouteHandler::getSerializer(const AcceptPtr & a, std::ostream & strm) const + IRouteHandler::getSerializer(const Accept & a, std::ostream & strm) const { for (const auto & rs : routeSerializers) { - if ((!a->group || rs.first.group == a->group) && (!a->type || rs.first.type == a->type)) { + if ((!a.group || rs.first.group == a.group) && (!a.type || rs.first.type == a.type)) { return {rs.first, rs.second->create(strm)}; } } diff --git a/icespider/core/irouteHandler.h b/icespider/core/irouteHandler.h index 7889031..6781151 100644 --- a/icespider/core/irouteHandler.h +++ b/icespider/core/irouteHandler.h @@ -23,7 +23,7 @@ namespace IceSpider { virtual ~IRouteHandler() = default; virtual void execute(IHttpRequest * request) const = 0; - virtual ContentTypeSerializer getSerializer(const AcceptPtr &, std::ostream &) const; + virtual ContentTypeSerializer getSerializer(const Accept &, std::ostream &) const; virtual ContentTypeSerializer defaultSerializer(std::ostream &) const; const HttpMethod method; diff --git a/icespider/unittests/testAccept.cpp b/icespider/unittests/testAccept.cpp index 91935fc..eb59bb3 100644 --- a/icespider/unittests/testAccept.cpp +++ b/icespider/unittests/testAccept.cpp @@ -46,10 +46,10 @@ BOOST_DATA_TEST_CASE(texthtml, a) { auto front = parse(a).front(); - BOOST_REQUIRE(front->group); - BOOST_REQUIRE_EQUAL(*front->group, "text"); - BOOST_REQUIRE(front->type); - BOOST_REQUIRE_EQUAL(*front->type, "html"); + BOOST_REQUIRE(front.group); + BOOST_REQUIRE_EQUAL(*front.group, "text"); + BOOST_REQUIRE(front.type); + BOOST_REQUIRE_EQUAL(*front.type, "html"); } BOOST_DATA_TEST_CASE(textany, @@ -62,9 +62,9 @@ BOOST_DATA_TEST_CASE(textany, a) { auto front = parse(a).front(); - BOOST_REQUIRE(front->group); - BOOST_REQUIRE_EQUAL(*front->group, "text"); - BOOST_REQUIRE(!front->type); + BOOST_REQUIRE(front.group); + BOOST_REQUIRE_EQUAL(*front.group, "text"); + BOOST_REQUIRE(!front.type); } BOOST_DATA_TEST_CASE(anyhtml, @@ -78,9 +78,9 @@ BOOST_DATA_TEST_CASE(anyhtml, a) { auto front = parse(a).front(); - BOOST_REQUIRE(front->group); - BOOST_REQUIRE(front->type); - BOOST_REQUIRE_EQUAL(*front->type, "html"); + BOOST_REQUIRE(front.group); + BOOST_REQUIRE(front.type); + BOOST_REQUIRE_EQUAL(*front.type, "html"); } BOOST_DATA_TEST_CASE(anyany, @@ -93,8 +93,8 @@ BOOST_DATA_TEST_CASE(anyany, a) { auto front = parse(a).front(); - BOOST_REQUIRE(!front->group); - BOOST_REQUIRE(!front->type); + BOOST_REQUIRE(!front.group); + BOOST_REQUIRE(!front.type); } BOOST_DATA_TEST_CASE(q1, @@ -107,8 +107,8 @@ BOOST_DATA_TEST_CASE(q1, { auto all = parse(a); for (const auto & accept : all) { - BOOST_TEST_CONTEXT(*accept) { - BOOST_CHECK_CLOSE(accept->q, 1.0, 0.1); + BOOST_TEST_CONTEXT(accept) { + BOOST_CHECK_CLOSE(accept.q, 1.0, 0.1); } } } diff --git a/icespider/unittests/testPerf.cpp b/icespider/unittests/testPerf.cpp index 1958525..5b82213 100644 --- a/icespider/unittests/testPerf.cpp +++ b/icespider/unittests/testPerf.cpp @@ -4,6 +4,8 @@ #include #include +#define BENCHMARK_CAPTURE_LITERAL(Name, Value) BENCHMARK_CAPTURE(Name, Value, Value); + class TestRequest : public IceSpider::CgiRequestBase { public: TestRequest(IceSpider::Core * c, char ** env) : IceSpider::CgiRequestBase(c, env) @@ -109,4 +111,21 @@ BENCHMARK_F(CoreFixture, get_cookie_param)(benchmark::State & state) } } +static void +AcceptParse(benchmark::State & state, const std::string_view accept) +{ + for (auto _ : state) { + benchmark::DoNotOptimize(IceSpider::IHttpRequest::parseAccept(accept)); + } +} +BENCHMARK_CAPTURE_LITERAL(AcceptParse, "*/*"); +BENCHMARK_CAPTURE_LITERAL(AcceptParse, "any/html"); +BENCHMARK_CAPTURE_LITERAL(AcceptParse, "image/png, */*"); +BENCHMARK_CAPTURE_LITERAL(AcceptParse, "image/png;q=0.1, */*"); +BENCHMARK_CAPTURE_LITERAL(AcceptParse, "image/png;q=0.1, any/html"); +BENCHMARK_CAPTURE_LITERAL(AcceptParse, "image/png;q=0.9, any/html;q=1.0, something/else;q=1.0"); +BENCHMARK_CAPTURE_LITERAL(AcceptParse, "image/png;q=0.9, */*;q=1.0"); +BENCHMARK_CAPTURE_LITERAL(AcceptParse, "text/*"); +BENCHMARK_CAPTURE_LITERAL(AcceptParse, "text/html"); + BENCHMARK_MAIN(); -- cgit v1.2.3