🚑 fix for #894

src/json.hpp

@@ -732,7 +732,7 @@ struct external_constructor<value_t::array>
         j.m_type = value_t::array;
         j.m_value = value_t::array;
         j.m_value.array->reserve(arr.size());
-        for (bool x : arr)
+        for (const bool x : arr)
         {
             j.m_value.array->push_back(x);
         }
@@ -2759,7 +2759,7 @@ scan_number_done:
     {
         // escape control characters
         std::string result;
-        for (auto c : token_string)
+        for (const auto c : token_string)
         {
             if ('\x00' <= c and c <= '\x1F')
             {
@@ -7011,6 +7011,27 @@ class json_pointer
         return to_string();
     }
 
+    /*!
+    @param[in] s  reference token to be converted into an array index
+
+    @return integer representation of @a s
+
+    @throw out_of_range.404 if string @a s could not be converted to an integer
+    */
+    static int array_index(const std::string& s)
+    {
+        size_t processed_chars = 0;
+        const int res = std::stoi(s, &processed_chars);
+
+        // check if the string was completely read
+        if (JSON_UNLIKELY(processed_chars != s.size()))
+        {
+            JSON_THROW(detail::out_of_range::create(404, "unresolved reference token '" + s + "'"));
+        }
+
+        return res;
+    }
+
   private:
     /*!
     @brief remove and return last reference pointer
@@ -7046,7 +7067,6 @@ class json_pointer
         return result;
     }
 
-
     /*!
     @brief create and return a reference to the pointed to value
 
@@ -12236,7 +12256,7 @@ class basic_json
             JSON_THROW(type_error::create(312, "cannot use update() with " + std::string(j.type_name())));
         }
 
-        for (auto it = j.begin(); it != j.end(); ++it)
+        for (auto it = j.cbegin(); it != j.cend(); ++it)
         {
             m_value.object->operator[](it.key()) = it.value();
         }
@@ -13981,7 +14001,7 @@ class basic_json
                         }
                         else
                         {
-                            const auto idx = std::stoi(last_path);
+                            const auto idx = json_pointer::array_index(last_path);
                             if (JSON_UNLIKELY(static_cast<size_type>(idx) > parent.size()))
                             {
                                 // avoid undefined behavior
@@ -14029,7 +14049,7 @@ class basic_json
             else if (parent.is_array())
             {
                 // note erase performs range check
-                parent.erase(static_cast<size_type>(std::stoi(last_path)));
+                parent.erase(static_cast<size_type>(json_pointer::array_index(last_path)));
             }
         };
 
@@ -14120,12 +14140,17 @@ class basic_json
 
                 case patch_operations::copy:
                 {
-                    const std::string from_path = get_value("copy", "from", true);
-                    const json_pointer from_ptr(from_path);
+                  const std::string from_path = get_value("copy", "from", true);
+                  const json_pointer from_ptr(from_path);
 
-                    // the "from" location must exist - use at()
-                    result[ptr] = result.at(from_ptr);
-                    break;
+                  // the "from" location must exist - use at()
+                  basic_json v = result.at(from_ptr);
+
+                  // The copy is functionally identical to an "add"
+                  // operation at the target location using the value
+                  // specified in the "from" member.
+                  operation_add(ptr, v);
+                  break;
                 }
 
                 case patch_operations::test:
@@ -14266,7 +14291,7 @@ class basic_json
                 case value_t::object:
                 {
                     // first pass: traverse this object's elements
-                    for (auto it = source.begin(); it != source.end(); ++it)
+                    for (auto it = source.cbegin(); it != source.cend(); ++it)
                     {
                         // escape the key name to be used in a JSON patch
                         const auto key = json_pointer::escape(it.key());
@@ -14288,7 +14313,7 @@ class basic_json
                     }
 
                     // second pass: traverse other object's elements
-                    for (auto it = target.begin(); it != target.end(); ++it)
+                    for (auto it = target.cbegin(); it != target.cend(); ++it)
                     {
                         if (source.find(it.key()) == source.end())
                         {
@@ -14381,7 +14406,7 @@ json_pointer::get_and_create(NLOHMANN_BASIC_JSON_TPL& j) const
                 // create an entry in the array
                 JSON_TRY
                 {
-                    result = &result->operator[](static_cast<size_type>(std::stoi(reference_token)));
+                    result = &result->operator[](static_cast<size_type>(array_index(reference_token)));
                 }
                 JSON_CATCH(std::invalid_argument&)
                 {
@@ -14458,7 +14483,7 @@ json_pointer::get_unchecked(NLOHMANN_BASIC_JSON_TPL* ptr) const
                     JSON_TRY
                     {
                         ptr = &ptr->operator[](
-                            static_cast<size_type>(std::stoi(reference_token)));
+                            static_cast<size_type>(array_index(reference_token)));
                     }
                     JSON_CATCH(std::invalid_argument&)
                     {
@@ -14513,7 +14538,7 @@ json_pointer::get_checked(NLOHMANN_BASIC_JSON_TPL* ptr) const
                 // note: at performs range check
                 JSON_TRY
                 {
-                    ptr = &ptr->at(static_cast<size_type>(std::stoi(reference_token)));
+                    ptr = &ptr->at(static_cast<size_type>(array_index(reference_token)));
                 }
                 JSON_CATCH(std::invalid_argument&)
                 {
@@ -14568,7 +14593,7 @@ json_pointer::get_unchecked(const NLOHMANN_BASIC_JSON_TPL* ptr) const
                 JSON_TRY
                 {
                     ptr = &ptr->operator[](
-                        static_cast<size_type>(std::stoi(reference_token)));
+                        static_cast<size_type>(array_index(reference_token)));
                 }
                 JSON_CATCH(std::invalid_argument&)
                 {
@@ -14622,7 +14647,7 @@ json_pointer::get_checked(const NLOHMANN_BASIC_JSON_TPL* ptr) const
                 // note: at performs range check
                 JSON_TRY
                 {
-                    ptr = &ptr->at(static_cast<size_type>(std::stoi(reference_token)));
+                    ptr = &ptr->at(static_cast<size_type>(array_index(reference_token)));
                 }
                 JSON_CATCH(std::invalid_argument&)
                 {

test/data/json-patch-tests/README.md

+JSON Patch Tests
+================
+
+These are test cases for implementations of [IETF JSON Patch (RFC6902)](http://tools.ietf.org/html/rfc6902).
+
+Some implementations can be found at [jsonpatch.com](http://jsonpatch.com).
+
+
+Test Format
+-----------
+
+Each test file is a JSON document that contains an array of test records. A
+test record is an object with the following members:
+
+- doc: The JSON document to test against
+- patch: The patch(es) to apply
+- expected: The expected resulting document, OR
+- error: A string describing an expected error
+- comment: A string describing the test
+- disabled: True if the test should be skipped
+
+All fields except 'doc' and 'patch' are optional. Test records consisting only
+of a comment are also OK.
+
+
+Files
+-----
+
+- tests.json: the main test file
+- spec_tests.json: tests from the RFC6902 spec
+
+
+Writing Tests
+-------------
+
+All tests should have a descriptive comment.  Tests should be as
+simple as possible - just what's required to test a specific piece of
+behavior.  If you want to test interacting behaviors, create tests for
+each behavior as well as the interaction.
+
+If an 'error' member is specified, the error text should describe the
+error the implementation should raise - *not* what's being tested.
+Implementation error strings will vary, but the suggested error should
+be easily matched to the implementation error string.  Try to avoid
+creating error tests that might pass because an incorrect error was
+reported.
+
+Please feel free to contribute!
+
+
+Credits
+-------
+
+The seed test set was adapted from Byron Ruth's
+[jsonpatch-js](https://github.com/bruth/jsonpatch-js/blob/master/test.js) and
+extended by [Mike McCabe](https://github.com/mikemccabe).
+
+
+License
+-------
+
+   Copyright 2014 The Authors
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+

test/data/json-patch-tests/spec_tests.json

+[
+  {
+    "comment": "4.1. add with missing object",
+    "doc": { "q": { "bar": 2 } },
+    "patch": [ {"op": "add", "path": "/a/b", "value": 1} ],
+    "error":
+       "path /a does not exist -- missing objects are not created recursively"
+  },
+
+  {
+    "comment": "A.1.  Adding an Object Member",
+    "doc": {
+  "foo": "bar"
+},
+    "patch": [
+  { "op": "add", "path": "/baz", "value": "qux" }
+],
+    "expected": {
+  "baz": "qux",
+  "foo": "bar"
+}
+  },
+
+  {
+    "comment": "A.2.  Adding an Array Element",
+    "doc": {
+  "foo": [ "bar", "baz" ]
+},
+    "patch": [
+  { "op": "add", "path": "/foo/1", "value": "qux" }
+],
+    "expected": {
+  "foo": [ "bar", "qux", "baz" ]
+}
+  },
+
+  {
+    "comment": "A.3.  Removing an Object Member",
+    "doc": {
+  "baz": "qux",
+  "foo": "bar"
+},
+    "patch": [
+  { "op": "remove", "path": "/baz" }
+],
+    "expected": {
+  "foo": "bar"
+}
+  },
+
+  {
+    "comment": "A.4.  Removing an Array Element",
+    "doc": {
+  "foo": [ "bar", "qux", "baz" ]
+},
+    "patch": [
+  { "op": "remove", "path": "/foo/1" }
+],
+    "expected": {
+  "foo": [ "bar", "baz" ]
+}
+  },
+
+  {
+    "comment": "A.5.  Replacing a Value",
+    "doc": {
+  "baz": "qux",
+  "foo": "bar"
+},
+    "patch": [
+  { "op": "replace", "path": "/baz", "value": "boo" }
+],
+    "expected": {
+  "baz": "boo",
+  "foo": "bar"
+}
+  },
+
+  {
+    "comment": "A.6.  Moving a Value",
+    "doc": {
+  "foo": {
+    "bar": "baz",
+    "waldo": "fred"
+  },
+  "qux": {
+    "corge": "grault"
+  }
+},
+    "patch": [
+  { "op": "move", "from": "/foo/waldo", "path": "/qux/thud" }
+],
+    "expected": {
+  "foo": {
+    "bar": "baz"
+  },
+  "qux": {
+    "corge": "grault",
+    "thud": "fred"
+  }
+}
+  },
+
+  {
+    "comment": "A.7.  Moving an Array Element",
+    "doc": {
+  "foo": [ "all", "grass", "cows", "eat" ]
+},
+    "patch": [
+  { "op": "move", "from": "/foo/1", "path": "/foo/3" }
+],
+    "expected": {
+  "foo": [ "all", "cows", "eat", "grass" ]
+}
+
+  },
+
+  {
+    "comment": "A.8.  Testing a Value: Success",
+    "doc": {
+  "baz": "qux",
+  "foo": [ "a", 2, "c" ]
+},
+    "patch": [
+  { "op": "test", "path": "/baz", "value": "qux" },
+  { "op": "test", "path": "/foo/1", "value": 2 }
+],
+    "expected": {
+     "baz": "qux",
+     "foo": [ "a", 2, "c" ]
+    }
+  },
+
+  {
+    "comment": "A.9.  Testing a Value: Error",
+    "doc": {
+  "baz": "qux"
+},
+    "patch": [
+  { "op": "test", "path": "/baz", "value": "bar" }
+],
+    "error": "string not equivalent"
+  },
+
+  {
+    "comment": "A.10.  Adding a nested Member Object",
+    "doc": {
+  "foo": "bar"
+},
+    "patch": [
+  { "op": "add", "path": "/child", "value": { "grandchild": { } } }
+],
+    "expected": {
+  "foo": "bar",
+  "child": {
+    "grandchild": {
+    }
+  }
+}
+  },
+
+  {
+    "comment": "A.11.  Ignoring Unrecognized Elements",
+    "doc": {
+  "foo":"bar"
+},
+    "patch": [
+  { "op": "add", "path": "/baz", "value": "qux", "xyz": 123 }
+],
+    "expected": {
+  "foo":"bar",
+  "baz":"qux"
+}
+  },
+
+ {
+    "comment": "A.12.  Adding to a Non-existent Target",
+    "doc": {
+  "foo": "bar"
+},
+    "patch": [
+  { "op": "add", "path": "/baz/bat", "value": "qux" }
+],
+    "error": "add to a non-existent target"
+  },
+
+ {
+    "comment": "A.13 Invalid JSON Patch Document",
+    "doc": {
+     "foo": "bar"
+    },
+    "patch": [
+  { "op": "add", "path": "/baz", "value": "qux", "op": "remove" }
+],
+    "error": "operation has two 'op' members",
+    "disabled": true
+  },
+
+  {
+    "comment": "A.14. ~ Escape Ordering",
+    "doc": {
+       "/": 9,
+       "~1": 10
+    },
+    "patch": [{"op": "test", "path": "/~01", "value": 10}],
+    "expected": {
+       "/": 9,
+       "~1": 10
+    }
+  },
+
+  {
+    "comment": "A.15. Comparing Strings and Numbers",
+    "doc": {
+       "/": 9,
+       "~1": 10
+    },
+    "patch": [{"op": "test", "path": "/~01", "value": "10"}],
+    "error": "number is not equal to string"
+  },
+
+  {
+    "comment": "A.16. Adding an Array Value",
+    "doc": {
+       "foo": ["bar"]
+    },
+    "patch": [{ "op": "add", "path": "/foo/-", "value": ["abc", "def"] }],
+    "expected": {
+      "foo": ["bar", ["abc", "def"]]
+    }
+  }
+
+]

test/src/unit-json_patch.cpp

@@ -31,6 +31,8 @@ SOFTWARE.
 #include "json.hpp"
 using nlohmann::json;
 
+#include <fstream>
+
 TEST_CASE("JSON patch")
 {
     SECTION("examples from RFC 6902")
@@ -1250,4 +1252,42 @@ TEST_CASE("JSON patch")
                               R"( [{"op": "test", "path": "/foo", "value": "bar"}] )"_json));
         }
     }
+
+    SECTION("Tests from github.com/json-patch/json-patch-tests")
+    {
+        for (auto filename :
+                {"test/data/json-patch-tests/spec_tests.json",
+                 "test/data/json-patch-tests/tests.json"
+                })
+        {
+            CAPTURE(filename);
+            std::ifstream f(filename);
+            json suite = json::parse(f);
+
+            for (const auto& test : suite)
+            {
+                CAPTURE(test.value("comment", ""))
+
+                // skip tests marked as disabled
+                if (test.value("disabled", false))
+                {
+                    continue;
+                }
+
+                const auto& doc = test["doc"];
+                const auto& patch = test["patch"];
+
+                if (test.count("error") == 0)
+                {
+                    // if an expected value is given, use it; use doc otherwise
+                    const auto& expected = test.value("expected", doc);
+                    CHECK(doc.patch(patch) == expected);
+                }
+                else
+                {
+                    CHECK_THROWS(doc.patch(patch));
+                }
+            }
+        }
+    }
 }

test/src/unit-regression.cpp

@@ -1322,4 +1322,32 @@ TEST_CASE("regression tests")
         j = ar;
         ar = j;
     }
+
+    SECTION("issue #894 - invalid RFC6902 copy operation succeeds")
+    {
+        auto model = R"({
+            "one": {
+                "two": {
+                    "three": "hello",
+                    "four": 42
+                }
+            }
+        })"_json;
+
+        CHECK_THROWS_AS(model.patch(R"([{"op": "move",
+                         "from": "/one/two/three",
+                         "path": "/a/b/c"}])"_json), json::out_of_range);
+        CHECK_THROWS_WITH(model.patch(R"([{"op": "move",
+                         "from": "/one/two/three",
+                         "path": "/a/b/c"}])"_json),
+                          "[json.exception.out_of_range.403] key 'a' not found");
+
+        CHECK_THROWS_AS(model.patch(R"([{"op": "copy",
+                                 "from": "/one/two/three",
+                                 "path": "/a/b/c"}])"_json), json::out_of_range);
+        CHECK_THROWS_WITH(model.patch(R"([{"op": "copy",
+                                 "from": "/one/two/three",
+                                 "path": "/a/b/c"}])"_json),
+                          "[json.exception.out_of_range.403] key 'a' not found");
+    }
 }