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

It is possible to call json::find with a json_pointer as argument. This causes runtime UB/crash. #1418

Closed
madmongo1 opened this issue Jan 9, 2019 · 5 comments
Labels
state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated

Comments

@madmongo1
Copy link

  • What is the issue you have?

Issue #1017 raises the ideal of allowing a json_pointer as the argument to find. This seems intuitive to me (in fact I made this error while writing a program). Unfortunately, even though this is not a legal operation (yet) it compiled. Presumably due to implicit conversion rules. I found the bug at runtime.

  • Please describe the steps to reproduce the issue. Can you provide a small but working code example?

Complete example to illustrate:

#include <iostream>
#include <nlohmann/json.hpp>

int main()
{
  using namespace nlohmann;

  // some JSON
  auto jv = R"xxx(
{
  "foo" : "bar",
  "bar" : {
    "baz" : "hello"
  }
}
)xxx"_json;

  // a JSON Pointer
  auto p = "/bar/baz"_json_pointer;
  
  // This is intuitive, compiles and produces the correct output
  std::cout << jv.at(p) << '\n';

  // This is also intuitive, compiles, but causes UB/assertion
  std::cout << *jv.find(p) << '\n';
}
  • What is the expected behavior?

To me:

Either:
a) json::find(json_pointer) produces an iterator referencing the item that would be found by json::at(json_pointer), or
b) compilation error.

  • And what is the actual behavior instead?

The program above compiles, giving the illusion of correctness and then (in debug mode) asserts at runtime.

Various combinations of:
gcc8, apple clang, clang
OSX, Fedora28

  • Did you use a released version of the library or the version from the develop branch?

Released

N/A

@glebbelov
Copy link

I need a non-throwing version of find( json_pointer& ). At the moment I use try { at() } catch but this makes debugging of other throwing points difficult.

@nlohmann
Copy link
Owner

I understand, and this was requested earlier. However, the implementation of JSON Pointers are heavily relying on exceptions in the moment, so this would require some larger refactoring. I'm not saying it won't happen - just that it may take some time. Any help would be greatly appreciated!

@FrancoisChabot
Copy link
Contributor

FrancoisChabot commented Jan 22, 2019

I know this won't really help OP as they have specific needs, but addressing the question as asked:

It's not invoking find() with a json_pointer that' causing the UB. The bug is unconditionally dereferencing the iterator returned by find(), which happens to be jv.end().

However, FWIW, find() compiling when passed a json_pointer comes from json_pointer's convertability to std::string (hello #958). And barring removing that conversion operation, I'd be in favor of adding a static_assert'ed overload to catch these at compilation time if feasible.

@stale
Copy link

stale bot commented Feb 21, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Feb 21, 2019
@stale stale bot closed this as completed Mar 1, 2019
@risa2000
Copy link

risa2000 commented Jul 19, 2023

Just got bitten by the same thing - calling json::find with json_pointer as an argumet. This compiles happily, but seems to return json::cend() for any pointer. I guess triggering static_assert or something during compilation will be helpful.

I thought I had a workaround (by using json::contains instead), but now I am really stuck at how to erase an element referenced by json_pointer. It seems that json::erase can work with anything but json_pointer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated
Projects
None yet
Development

No branches or pull requests

5 participants