Thalhammer / jwt-cpp

A header only library for creating and validating json web tokens in c++
https://thalhammer.github.io/jwt-cpp/
MIT License
828 stars 229 forks source link

Update tests to include jsoncpp trait #320

Closed prince-chrismc closed 6 months ago

prince-chrismc commented 6 months ago

@cjserio I believe there is a small bug 🐛 with the behavior regarding integers (NumbericDatetime from RFC 7519) being mistaken for number (floats, etc). This is something most other libraries have with the translation from Javascript to C++ but I am not sure how it works for JSONCPP so I would appreciate any help looking into this 🙏

cjserio commented 6 months ago

Hmm the only error i see in the test is:

/home/runner/work/jwt-cpp/jwt-cpp/tests/traits/OspJsoncppTest.cpp:15: Failure Expected equality of these values: integer.get_type() Which is: 4-byte object <02-00 00-00> jwt::json::type::integer Which is: 4-byte object <01-00 00-00>

Which looks like the test failing is:

const auto integer = jwt::basic_claim<jwt::traits::open_source_parsers_jsoncpp>(159816816); ASSERT_EQ(integer.get_type(), jwt::json::type::integer);

Is that what you're referring to?

prince-chrismc commented 6 months ago

yes that exactly it! Thanks for being so quick ❤️

jwt::json::type::integer == 1
jwt::json::type::number == 2
cjserio commented 6 months ago

Facepalm for JsonCPP:

bool Value::isDouble() const {
  return type() == intValue || type() == uintValue || type() == realValue;
}

bool Value::isNumeric() const { return isDouble(); }

I think I just need to be more strict about the order I check these in...Where should I push a fix for this?

cjserio commented 6 months ago

Actually, here's the patch if you want to just apply it. Otherwise let me know what to branch off of and where to put the fix:

git diff
diff --git a/include/jwt-cpp/traits/open-source-parsers-jsoncpp/traits.h b/include/jwt-cpp/traits/open-source-parsers-jsoncpp/traits.h
index 1d4c0c7..3598a7d 100644
--- a/include/jwt-cpp/traits/open-source-parsers-jsoncpp/traits.h
+++ b/include/jwt-cpp/traits/open-source-parsers-jsoncpp/traits.h
@@ -66,10 +66,10 @@ namespace jwt {
                                        return type::array;
                                else if (val.isString())
                                        return type::string;
+                else if (val.isInt())
+                    return type::integer;
                                else if (val.isNumeric())
                                        return type::number;
-                               else if (val.isInt())
-                                       return type::integer;
                                else if (val.isBool())
                                        return type::boolean;
                                else if (val.isObject())
prince-chrismc commented 6 months ago

Perfect that as exactly it https://github.com/Thalhammer/jwt-cpp/actions/runs/7290583494/job/19867701199 👏