From 30376c3d4af3f0fa8d1c89e2ff38f332fe1995bf Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Wed, 16 May 2018 08:20:09 +0100 Subject: Cut 3 string_view (accept parser) Complete rewrite of the accept header parser to be string_view compatible and far more standards compliant, including checking for invalid requests. Bolsters tests over accept header parser considerably. --- icespider/core/ihttpRequest.cpp | 51 ++++++++++++++++++++++++++------------ icespider/core/ihttpRequest.h | 2 +- icespider/unittests/testAccept.cpp | 48 +++++++++++++++++++++-------------- icespider/unittests/testApp.cpp | 2 +- 4 files changed, 66 insertions(+), 37 deletions(-) diff --git a/icespider/core/ihttpRequest.cpp b/icespider/core/ihttpRequest.cpp index 21506e5..ea94fb2 100644 --- a/icespider/core/ihttpRequest.cpp +++ b/icespider/core/ihttpRequest.cpp @@ -5,6 +5,7 @@ #include "xwwwFormUrlEncoded.h" #include #include +#include #include namespace IceSpider { @@ -34,29 +35,47 @@ namespace IceSpider { } Accepted - IHttpRequest::parseAccept(const std::string & acceptHdr) + IHttpRequest::parseAccept(const std::string_view & acceptHdr) { - auto accept = acceptHdr.c_str(); + auto accept = std::unique_ptr( + fmemopen(const_cast(acceptHdr.data()), acceptHdr.length(), "r"), &fclose); Accepted accepts; - accepts.reserve(5); - char grp[BUFSIZ], type[BUFSIZ]; - float pri = 0.0f; - int chars, v; - while ((v = sscanf(accept, " %[^ /] / %[^ ;,] %n , %n", grp, type, &chars, &chars)) == 2) { - accept += chars; - chars = 0; + accepts.reserve(acceptHdr.length() / 15); + int grpstart, grpend, typestart, typeend; + auto reset = [&]() { + grpstart = grpend = typestart = typeend = 0; + return ftell(accept.get()); + }; + + for (auto off = reset(); + fscanf(accept.get(), " %n%*[^ /]%n / %n%*[^ ;,]%n", &grpstart, &grpend, &typestart, &typeend) == 0; + off = reset()) { + if (grpend <= grpstart || typestart <= grpend || typeend <= typestart) { + throw Http400_BadRequest(); + } auto a = std::make_shared(); - if ((v = sscanf(accept, " ; q = %f %n , %n", &pri, &chars, &chars)) == 1) { - a->q = pri; + if (auto g = acceptHdr.substr(typestart + off, typeend - typestart); g != "*") { + a->type.emplace(g); + } + if (auto t = acceptHdr.substr(grpstart + off, grpend - grpstart); t != "*") { + a->group.emplace(t); } - if (strcmp(grp, "*")) { - a->group.emplace(grp); + else if (a->type) { + throw Http400_BadRequest(); } - if (strcmp(type, "*")) { - a->type.emplace(type); + if (fscanf(accept.get(), " ; q = %f ", &a->q) == 1) { + if (a->q <= 0.0f || a->q > 1.0f) { + throw Http400_BadRequest(); + } } - accept += chars; accepts.push_back(a); + if (fscanf(accept.get(), " ,") != 0) { + break; + } + } + + 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 accepts; diff --git a/icespider/core/ihttpRequest.h b/icespider/core/ihttpRequest.h index be3a0e0..fdc3b5d 100644 --- a/icespider/core/ihttpRequest.h +++ b/icespider/core/ihttpRequest.h @@ -32,7 +32,7 @@ namespace IceSpider { virtual OptionalString getHeaderParam(const std::string_view &) const = 0; virtual OptionalString getCookieParam(const std::string_view &) const = 0; virtual OptionalString getEnv(const std::string_view &) const = 0; - static Accepted parseAccept(const std::string &); + static Accepted parseAccept(const std::string_view &); virtual Slicer::DeserializerPtr getDeserializer() const; virtual ContentTypeSerializer getSerializer(const IRouteHandler *) const; virtual std::istream & getInputStream() const = 0; diff --git a/icespider/unittests/testAccept.cpp b/icespider/unittests/testAccept.cpp index fb5705b..2cabe5e 100644 --- a/icespider/unittests/testAccept.cpp +++ b/icespider/unittests/testAccept.cpp @@ -1,8 +1,9 @@ #define BOOST_TEST_MODULE TestAccept -#include +#include #include #include +#include auto parse = IceSpider::IHttpRequest::parseAccept; using namespace boost::unit_test::data; @@ -18,19 +19,27 @@ namespace std { } } -BOOST_DATA_TEST_CASE(empty, make({ - "", - " ", +BOOST_TEST_DECORATOR(* boost::unit_test::timeout(1)) +BOOST_DATA_TEST_CASE(bad_requests, make({ + "", // Can't specify nothing + " ", // This is still nothing + " group ", + " ; ", + " / ", + " ^ ", + " * / plain ", + " text / plain ; q = 0.0 ", + " text / plain ; q = 1.1 ", }), a) { - BOOST_REQUIRE(parse(a).empty()); + BOOST_CHECK_THROW(parse(a), IceSpider::Http400_BadRequest); } BOOST_DATA_TEST_CASE(texthtml, make({ "text/html", - "image/png;q=0, text/html", - "image/png;q=1.0, text/html;q=1.1", - "image/png;q=1.0, text/html;q=1.1, something/else;q=1.1", + "image/png;q=0.1, text/html", + "image/png;q=0.9, text/html;q=1.0", + "image/png;q=0.9, text/html;q=1.0, something/else;q=1.0", }), a) { auto front = parse(a).front(); @@ -42,9 +51,9 @@ BOOST_DATA_TEST_CASE(texthtml, make({ BOOST_DATA_TEST_CASE(textany, make({ "text/*", - "image/png;q=0, text/*", - "image/png;q=1.0, text/*;q=1.1", - "image/png;q=1.0, text/*;q=1.1, something/else;q=1.1", + "image/png;q=0.1, text/*", + "image/png;q=0.9, text/*;q=1.0", + "image/png;q=0.9, text/*;q=1.0, something/else;q=1.0", }), a) { auto front = parse(a).front(); @@ -54,23 +63,24 @@ BOOST_DATA_TEST_CASE(textany, make({ } BOOST_DATA_TEST_CASE(anyhtml, make({ - "*/html", - "image/png;q=0, */html", - "image/png;q=1.0, */html;q=1.1", - "image/png;q=1.0, */html;q=1.1, something/else;q=1.1", + "any/html", + "image/png;q=0.1, any/html", + "image/png;q=0.9, any/html;q=1.0", + "image/png;q=0.9, any/html;q=1.0, something/else;q=1.0", + " image / png ; q = 0.9 , any / html ; q = 1.0 , something / else ; q = 1.0", }), a) { auto front = parse(a).front(); - BOOST_REQUIRE(!front->group); + BOOST_REQUIRE(front->group); BOOST_REQUIRE(front->type); BOOST_REQUIRE_EQUAL(*front->type, "html"); } BOOST_DATA_TEST_CASE(anyany, make({ "*/*", - "image/png;q=0, */*", - "image/png;q=1.0, */*;q=1.1", - "image/png;q=1.0, */*;q=1.1, something/else;q=1.1", + "image/png;q=0.1, */*", + "image/png;q=0.9, */*;q=1.0", + "image/png;q=0.9, */*;q=1.0, something/else;q=1.0", }), a) { auto front = parse(a).front(); diff --git a/icespider/unittests/testApp.cpp b/icespider/unittests/testApp.cpp index 918a056..3bfcd8d 100644 --- a/icespider/unittests/testApp.cpp +++ b/icespider/unittests/testApp.cpp @@ -411,7 +411,7 @@ BOOST_AUTO_TEST_CASE( testCallIndexAcceptNotSupported ) BOOST_AUTO_TEST_CASE( testCallIndexComplexAccept ) { TestRequest requestChoice(this, HttpMethod::GET, "/"); - requestChoice.hdr["Accept"] = "something/special ; q = 20, application/json, application/xml;q=1.1"; + requestChoice.hdr["Accept"] = "something/special ; q = 0.9, application/json ; q = 0.8, application/xml;q=1.0"; process(&requestChoice); auto h = requestChoice.getResponseHeaders(); BOOST_REQUIRE_EQUAL(h["Status"], "200 OK"); -- cgit v1.2.3