Skip to content

Commit 71acb52

Browse files
Woazboatmmd-osm
authored andcommitted
Stricter accept header parsing. Throw on malformed input, don't throw on no acceptable media types
1 parent ff88a7e commit 71acb52

File tree

2 files changed

+84
-15
lines changed

2 files changed

+84
-15
lines changed

src/choose_formatter.cpp

+25-13
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <string>
3131
#include <vector>
3232
#include <algorithm>
33+
#include <charconv>
3334

3435
#include <fmt/core.h>
3536

@@ -45,10 +46,6 @@ AcceptHeader::AcceptHeader(const std::string &header) {
4546

4647
for (const auto &acceptedType : acceptedTypes)
4748
mapping[acceptedType.mimeType] = acceptedType.q;
48-
49-
if (mapping.empty()) {
50-
throw http::bad_request("Accept header could not be parsed.");
51-
}
5249
}
5350

5451
[[nodiscard]] bool AcceptHeader::is_acceptable(mime::type mt) const {
@@ -91,13 +88,16 @@ std::vector<AcceptHeader::AcceptElement> AcceptHeader::parse(const std::string &
9188
// Split by comma to get individual media types
9289
auto items = split_trim(data, ',');
9390

91+
if (items.empty())
92+
throw http::bad_request("Invalid empty accept header");
93+
9494
for (const auto &item : items) {
9595
// Split each item by semicolon to separate media type from
9696
// parameters
9797
auto elems = split_trim(item, ';');
9898

9999
if (elems.empty()) {
100-
continue;
100+
throw http::bad_request("Malformed accept header");
101101
}
102102

103103
// Treat Accept: * as Accept: */*
@@ -106,8 +106,13 @@ std::vector<AcceptHeader::AcceptElement> AcceptHeader::parse(const std::string &
106106

107107
// Split the media type into type and subtype
108108
auto mime_parts = split_trim(elems[0], '/');
109-
if (mime_parts.size() != 2 || mime_parts[1].empty()) {
110-
continue;
109+
if (mime_parts.size() != 2 || mime_parts[0].empty() || mime_parts[1].empty()) {
110+
throw http::bad_request("Invalid accept header media type");
111+
}
112+
113+
// */subtype is not allowed
114+
if (mime_parts[0] == "*" && mime_parts[1] != "*") {
115+
throw http::bad_request("Invalid wildcard in accept header media type");
111116
}
112117

113118
// figure out the mime::type from the string
@@ -125,14 +130,21 @@ std::vector<AcceptHeader::AcceptElement> AcceptHeader::parse(const std::string &
125130
// Parse parameters
126131
for (size_t i = 1; i < elems.size(); ++i) {
127132
auto param_parts = split_trim(elems[i], '=');
128-
if (param_parts.size() != 2)
129-
continue;
133+
if (param_parts.size() != 2) {
134+
throw http::bad_request("Malformed parameter in accept header");
135+
}
130136

131137
if (param_parts[0] == "q") {
132-
try {
133-
acceptElement.q = std::stod(std::string{param_parts[1]});
134-
} catch (const std::exception &) {
135-
acceptElement.q = 0.0;
138+
auto [ptr, ec] = std::from_chars(param_parts[1].begin(),
139+
param_parts[1].end(),
140+
acceptElement.q,
141+
std::chars_format::fixed);
142+
if (ec != std::errc() ||
143+
ptr != param_parts[1].end() ||
144+
!std::isfinite(acceptElement.q) ||
145+
acceptElement.q < 0 ||
146+
acceptElement.q > 1) {
147+
throw http::bad_request("Invalid q parameter value in accept header");
136148
}
137149
} else {
138150
acceptElement.params[std::string{param_parts[0]}] = std::string{param_parts[1]};

test/test_http.cpp

+59-2
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,66 @@ TEST_CASE("http_check_accept_header_parsing", "[http]") {
109109
CHECK(header.is_acceptable(mime::type::text_plain) == true);
110110
}
111111

112+
SECTION("test: wildcard header") {
113+
AcceptHeader header{"*/*"};
114+
CHECK(header.is_acceptable(mime::type::any_type) == true);
115+
CHECK(header.is_acceptable(mime::type::application_json) == true);
116+
CHECK(header.is_acceptable(mime::type::application_xml) == true);
117+
CHECK(header.is_acceptable(mime::type::text_plain) == true);
118+
}
119+
120+
SECTION("test: bug-compatible wildcard header ") {
121+
AcceptHeader header{"*"};
122+
CHECK(header.is_acceptable(mime::type::any_type) == true);
123+
CHECK(header.is_acceptable(mime::type::application_json) == true);
124+
CHECK(header.is_acceptable(mime::type::application_xml) == true);
125+
CHECK(header.is_acceptable(mime::type::text_plain) == true);
126+
}
127+
112128
SECTION("test: not supported mime types") {
113-
REQUIRE_THROWS_AS(AcceptHeader{"audio/*; q=0.2, audio/basic"}, http::bad_request);
114-
REQUIRE_THROWS_AS(AcceptHeader{"text, text/html"}, http::bad_request);
129+
CHECK_NOTHROW(AcceptHeader{"audio/*; q=0.2, audio/basic"});
130+
CHECK_NOTHROW(AcceptHeader{"text/html"});
131+
}
132+
133+
SECTION("test: invalid accept header format") {
134+
CHECK_THROWS_AS(AcceptHeader{""}, http::bad_request);
135+
CHECK_THROWS_AS(AcceptHeader{"/"}, http::bad_request);
136+
CHECK_THROWS_AS(AcceptHeader{"*/"}, http::bad_request);
137+
CHECK_THROWS_AS(AcceptHeader{"foo/"}, http::bad_request);
138+
CHECK_THROWS_AS(AcceptHeader{"/*"}, http::bad_request);
139+
CHECK_THROWS_AS(AcceptHeader{"/foo"}, http::bad_request);
140+
CHECK_THROWS_AS(AcceptHeader{"*/foo"}, http::bad_request);
141+
CHECK_THROWS_AS(AcceptHeader{"text"}, http::bad_request);
142+
}
143+
144+
SECTION("test: accept header params") {
145+
CHECK_NOTHROW(AcceptHeader{"application/xml;q=0.5"});
146+
CHECK_NOTHROW(AcceptHeader{"application/xml;baz=abc;bat=123"});
147+
CHECK_NOTHROW(AcceptHeader{"application/xml;baz=abc;bat=123, application/json; param1=1; param2=2"});
148+
CHECK_NOTHROW(AcceptHeader{"foo/bar;q=0.5; accept-extension-param1=abcd123; exptaram=%653"});
149+
CHECK_NOTHROW(AcceptHeader{"text/html, application/xhtml+xml, application/xml;q=0.9, image/webp, */*;q=0.8"});
150+
CHECK_NOTHROW(AcceptHeader{"text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7"});
151+
CHECK_NOTHROW(AcceptHeader{"text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/png,image/svg+xml,*/*;q=0.8"});
152+
}
153+
154+
SECTION("test: invalid accept header q value") {
155+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=foobar"}, http::bad_request);
156+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q="}, http::bad_request);
157+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=123456"}, http::bad_request);
158+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=-123456"}, http::bad_request);
159+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=1.1"}, http::bad_request);
160+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=-0.1"}, http::bad_request);
161+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=NAN"}, http::bad_request);
162+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=-NAN"}, http::bad_request);
163+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=INF"}, http::bad_request);
164+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=INFINITY"}, http::bad_request);
165+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=-INF"}, http::bad_request);
166+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=-INFINITY"}, http::bad_request);
167+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=0x1"}, http::bad_request);
168+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=0x0"}, http::bad_request);
169+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=0b1"}, http::bad_request);
170+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=0.5abc"}, http::bad_request);
171+
CHECK_THROWS_AS(AcceptHeader{"application/xml;q=abc0.5"}, http::bad_request);
115172
}
116173

117174
SECTION("test: application/json") {

0 commit comments

Comments
 (0)