Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

traits/nlohmann-json using json::parse with exceptions without catch, an uncaught exception is generated when handling bad input. #293

Open
1 task done
lijl44 opened this issue May 30, 2023 · 6 comments

Comments

@lijl44
Copy link

lijl44 commented May 30, 2023

What happened?

There is the following functions in the include/jwt-cpp/traits/nlohmann-json/traits.h header file.

static bool parse(json& val, std::string str) {
			val = json::parse(str.begin(), str.end());
			return true;
		}

This function uses json::parse with exceptions, an uncaught exception is generated when handling bad input.

Could you please refer to the following nlohmann-json official example? Thank you.

// parse with exceptions
try
{
    json j = json::parse(text);
}
catch (json::parse_error& e)
{
    std::cout << e.what() << std::endl;
}

// parse without exceptions
json j = json::parse(text, nullptr, false);

if (j.is_discarded())
{
    std::cout << "the input is invalid JSON" << std::endl;
}
else
{
    std::cout << "the input is valid JSON: " << j << std::endl;
}

How To Reproduce?

No response

Version

0.6.0

What OS are you seeing the problem on?

Linux

What compiler are you seeing the problem on?

GCC

Relevant log output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@lijl44 lijl44 added the bug label May 30, 2023
@prince-chrismc
Copy link
Collaborator

This library is not exception free so could you elaborate on what the advantages of this would be?

This would be suppressing a useful error case for users with malformed token as well. I am curious what pros you see to this?

@lijl44
Copy link
Author

lijl44 commented May 31, 2023

In the expected case, jwt-cpp throws an invalid_json_exception exception when parsing json fails.
struct invalid_json_exception : public std::runtime_error

In fact, I failed when I tried to catch std::runtime_error because a nlohmann::json::parse_error exception was thrown instead.
class parse_error : public exception
class exception : public std::exception

Obviously, the actual exception thrown does not match the expected one.
I think a change to this would be beneficial to make jwt-cpp's behavior consistent across json libs, thanks.

@Thalhammer
Copy link
Owner

I am curious what pros you see to this?

I think the bigger issue is that using the error_code overloads might still throw exceptions in this case.

@prince-chrismc
Copy link
Collaborator

Yes but the traits could easily be modified.

However we'd need to do this for every single json library where some might not support this. Then how do we expose that?

I think as a header-only library we do not care how the JSON libraries are compiled or setup. https://github.com/nlohmann/json/blob/a0c1318830519eac027a31edec1a99ce1ae5670e/docs/mkdocs/docs/home/exceptions.md?plain=1#L29 could easily be setup with the current implementation to have the desired effect?

I work on a package manager so in my head if you need to set something on a dependency it's not the responsibility of the upstream projects to do that for you.

Especially for the "user provided override" there's no obvious we could handle that? 🤔

@lijl44 lijl44 closed this as completed May 31, 2023
@lijl44
Copy link
Author

lijl44 commented May 31, 2023

I agree with you, in fact I'm just giving feedback since nlohmann-json is a commonly used json lib.

@prince-chrismc
Copy link
Collaborator

Oh this is a good discussion let's keep it open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants