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

array_index possible out of range #2205

Closed
t-b opened this issue Jun 21, 2020 · 2 comments
Closed

array_index possible out of range #2205

t-b opened this issue Jun 21, 2020 · 2 comments
Labels
kind: question solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@t-b
Copy link
Contributor

t-b commented Jun 21, 2020

Due to my work on #2203 I stumpled upon:

}
else
{
const auto idx = json_pointer::array_index(last_path);
if (JSON_HEDLEY_UNLIKELY(static_cast<size_type>(idx) > parent.size()))
{
// avoid undefined behavior
JSON_THROW(out_of_range::create(401, "array index " + std::to_string(idx) + " is out of range"));
}
// default case: insert add offset

and I'm really not understanding why it is if (JSON_HEDLEY_UNLIKELY(static_cast<size_type>(idx) > parent.size())) and not if (JSON_HEDLEY_UNLIKELY(static_cast<size_type>(idx) >= parent.size())). For zero-based indizes we need to be smaller than the size or?

@nlohmann
Copy link
Owner

Good question. From the JSON Patch specification (https://tools.ietf.org/html/rfc6902#section-4.1):

The specified index MUST NOT be greater than the number of elements in the array. If the "-" character is used to index the end of the array (see [RFC6901]), this has the effect of appending the value to the array.

I interpret these sentences that (1) passing the size of the array as index is allowed (as that index is not greater than the number of elements) and that (2) that value could be interpreted just like -, meaning we would append to the array.

So this code:

#include <iostream>
#include "json.hpp"

using json = nlohmann::json;

int main() {
    json j = {0, 1, 2};
    json add = {{{"op", "add"}, {"path", "/3"}, {"value", 99}}};
    
    auto result = j.patch(add);
    
    std::cout << result << std::endl;
}

prints

[0,1,2,99]

And [{"op":"add","path":"/3","value":99}] is treated the same as [{"op":"add","path":"/-","value":99}].

Do you agree?

@nlohmann nlohmann added kind: question solution: proposed fix a fix for the issue has been proposed and waits for confirmation labels Jun 22, 2020
@t-b
Copy link
Contributor Author

t-b commented Jun 22, 2020

@nlohmann Yes, I agree.

Just for future reference: In your example (parent.begin() + static_cast<difference_type>(idx)) == parent.end() holds and parent.end() can be passed to insert.

@t-b t-b closed this as completed Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: question solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

2 participants