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.

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 <boost/lexical_cast.hpp>
 #include <time.h>
+#include <stdio.h>
 #include <formatters.h>
 
 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<FILE, decltype(&fclose)>(
+				fmemopen(const_cast<char *>(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<Accept>();
-			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 <boost/test/unit_test.hpp>
+#include <boost/test/included/unit_test.hpp>
 #include <boost/test/data/test_case.hpp>
 
 #include <ihttpRequest.h>
+#include <exceptions.h>
 
 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");