fix(mime): ignore locales when parsing qvalues

[Tachi107]

Qvalues are now parsed manually in str_to_qvalue() instead of using the locale-dependant std::strtof.

Using a custom function allowed me to add some more checks to the function; while std::strtof was previously accepting all floating point numbers, str_to_qvalues() makes sure that the supplied input conforms to the HTTP standard

A rapid "conformance check" reveals the following results:

$ cat qvalues
0
0.0
0.00
0.000
0.1
0.12
0.123
1
1.0
1.00
1.000
0.
00.0
0.0.0
.0
.
1.
1.001
.1
0.1234
a
0.a
1.a
a.1
0.00a
0.000a

std::strtof():

$ for value in $(cat qvalues); do
      curl localhost:9080 --header "Accept: text/html;q=$value"
  done
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Unfinished Media Type parameter
Hello World
Invalid quality factor
Hello World
Hello World
Hello World
Hello World
Invalid quality factor
Unfinished Media Type parameter
Unfinished Media Type parameter
Invalid quality factor
Unfinished Media Type parameter
Unfinished Media Type parameter

str_to_qvalue():

$ for value in $(cat qvalues); do
      curl localhost:9080 --header "Accept: text/html;q=$value"
  done
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Hello World
Invalid quality factor
Invalid quality factor
Invalid quality factor
Invalid quality factor
Invalid quality factor
Invalid quality factor
Invalid quality factor
Invalid quality factor
Invalid quality factor
Invalid quality factor
Invalid quality factor
Invalid quality factor
Invalid quality factor
Unfinished Media Type parameter
Unfinished Media Type parameter

str_to_qvalue() is far stricter, making MIME handling as a whole more reliable. It also makes responses more accurate, as in the new approach "Invalid quality factor" is sent when the old one wrongly sends "Unfinished Media Type parameter".

Fixes #1076

Please note that I'm not that familiar with how the Accept header works, so this might be completely wrong!

@rems4e could you confirm that this patch fixes your issue? Thanks

~TODO: tests~

[Tachi107]

In addition to verifying taht str_to_qvalue() rejects malformed input, also verify that it correctly parses stuff like:

Thanks for mentioning, I can confirm that it does.

This can possibly have odd rounding errors, as its not a perfect binary fraction.

Suggestion: Parse the number as if it were an integer, then convert to float at the very end of the function.

I'm not sure I understood you correctly, could you please check if my latest change is ok?

Please add unit tests

I simply added a call to std::locale::global(std::locale("")) in the mime test to set the user-preferred locale when parsing, and it should be enough as valid_parsing_test already checks that qvalues are parsed correctly. In fact, setting the locale while reverting this fix results in failed tests (this is only effective in systems where the preferred locale uses , as the decimal separator, but specifying a specific locale may be worse- this makes it possible to run tests with different locales without recompiling)

[Tachi107]

From my new commit:

I added some tests that verify that the HttpError exeption message is the expected one, i.e. "Invalid quality factor". This is done with GoogleTest's new ThrowsMessage matchers. Unfortunately they are not fully documented, but most information about them can be found in this comment: https://github.com/google/googletest/issues/952#issuecomment-638968347

This (new?) EXPECT_THAT syntax seems to be encouraged by GoogleTest, and I find it quite nice: https://google.github.io/googletest/faq.html#why-does-googletest-support-expect_eqnull-ptr-and-assert_eqnull-ptr-but-not-expect_nenull-ptr-and-assert_nenull-ptr

EXPECT_THAT is part of GMock, and the ThrowsMessage matcher is only available in GoogleTest 1.11.0 and newer. ~This means that the debian/control file in the debian branch needs to be updated.~ Done in https://github.com/pistacheio/pistache/commit/6b461d4aefdea0f416472907e69981ad9b64eb58

[dennisjenkins75]

EXPECT_THAT(fut, Matcher()) is indeed the newer syntax (instead of EXPECT_FOO(fut)). I hope to review this PR in full soon. Just busy w/ work (where I also use Google test extensively).