Make parse_many safer. (simdjson#1137)

lemire · web-flow · commit 5b10c38e4319 · 2020-08-20T22:22:46.000-04:00
diff --git a/doc/basics.md b/doc/basics.md
@@ -621,7 +621,7 @@ Unlike `parser.parse`, both `parser.load_many(filename)` and `parser.parse_many(
 document at a time.
 
 1. When calling `parser.load_many(filename)`, the file's content is loaded up in a memory buffer owned by the `parser`'s instance. Thus the file can be safely deleted after calling `parser.load_many(filename)` as the parser instance owns all of the data.
-2. When calling  `parser.parse_many(string)`, no copy is made of the provided string input. The provided memory buffer may be accessed each time a JSON document is parsed.  Calling `parser.parse_many(string)` on a  temporary string buffer (e.g., `docs = parser.parse_many("[1,2,3]"_padded)`) is unsafe because the  `document_stream` instance needs access to the buffer to return the JSON documents. In constrast, calling `doc = parser.parse("[1,2,3]"_padded)` is safe because `parser.parse` eagerly parses the input.
+2. When calling  `parser.parse_many(string)`, no copy is made of the provided string input. The provided memory buffer may be accessed each time a JSON document is parsed.  Calling `parser.parse_many(string)` on a  temporary string buffer (e.g., `docs = parser.parse_many("[1,2,3]"_padded)`) is unsafe (and will not compile) because the  `document_stream` instance needs access to the buffer to return the JSON documents. In constrast, calling `doc = parser.parse("[1,2,3]"_padded)` is safe because `parser.parse` eagerly parses the input.
 
 
 Both `load_many` and `parse_many` take an optional parameter `size_t batch_size` which defines the window processing size. It is set by default to a large value (`1000000` corresponding to 1 MB). None of your JSON documents should exceed this window size, or else you will get  the error `simdjson::CAPACITY`. You cannot set this window size larger than 4 GB: you will get  the error `simdjson::CAPACITY`. The smaller the window size is, the less memory the function will use. Setting the window size too small (e.g., less than 100 kB) may also impact performance negatively. Leaving it to 1 MB is expected to be a good choice, unless you have some larger documents.
diff --git a/doc/basics_doxygen.md b/doc/basics_doxygen.md
@@ -602,7 +602,7 @@ Unlike `parser.parse`, both `parser.load_many(filename)` and `parser.parse_many(
 document at a time.
 
 1. When calling `parser.load_many(filename)`, the file's content is loaded up in a memory buffer owned by the `parser`'s instance. Thus the file can be safely deleted after calling `parser.load_many(filename)` as the parser instance owns all of the data.
-2. When calling  `parser.parse_many(string)`, no copy is made of the provided string input. The provided memory buffer may be accessed each time a JSON document is parsed.  Calling `parser.parse_many(string)` on a  temporary string buffer (e.g., `docs = parser.parse_many("[1,2,3]"_padded)`) is unsafe because the  `document_stream` instance needs access to the buffer to return the JSON documents. In constrast, calling `doc = parser.parse("[1,2,3]"_padded)` is safe because `parser.parse` eagerly parses the input.
+2. When calling  `parser.parse_many(string)`, no copy is made of the provided string input. The provided memory buffer may be accessed each time a JSON document is parsed.  Calling `parser.parse_many(string)` on a  temporary string buffer (e.g., `docs = parser.parse_many("[1,2,3]"_padded)`) is unsafe  (and will not compile) because the  `document_stream` instance needs access to the buffer to return the JSON documents. In constrast, calling `doc = parser.parse("[1,2,3]"_padded)` is safe because `parser.parse` eagerly parses the input.
 
 Both `load_many` and `parse_many` take an optional parameter `size_t batch_size` which defines the window processing size. It is set by default to a large value (`1000000` corresponding to 1 MB). None of your JSON documents should exceed this window size, or else you will get  the error `simdjson::CAPACITY`. You cannot set this window size larger than 4 GB: you will get  the error `simdjson::CAPACITY`. The smaller the window size is, the less memory the function will use. Setting the window size too small (e.g., less than 100 kB) may also impact performance negatively. Leaving it to 1 MB is expected to be a good choice, unless you have some larger documents.
 
diff --git a/include/simdjson/dom/parser.h b/include/simdjson/dom/parser.h
@@ -233,7 +233,7 @@ class parser {
    * returned.
    * 
    * The caller is responsabile to ensure that the input string data remains unchanged and is
-   * not deleted during the loop. In particular, the following is unsafe:
+   * not deleted during the loop. In particular, the following is unsafe and will not compile:
    * 
    *   auto docs = parser.parse_many("[\"temporary data\"]"_padded);
    *   // here the string "[\"temporary data\"]" may no longer exist in memory
@@ -314,9 +314,11 @@ class parser {
   inline simdjson_result<document_stream> parse_many(const char *buf, size_t len, size_t batch_size = DEFAULT_BATCH_SIZE) noexcept;
   /** @overload parse_many(const uint8_t *buf, size_t len, size_t batch_size) */
   inline simdjson_result<document_stream> parse_many(const std::string &s, size_t batch_size = DEFAULT_BATCH_SIZE) noexcept;
+  inline simdjson_result<document_stream> parse_many(const std::string &&s, size_t batch_size) = delete;// unsafe
   /** @overload parse_many(const uint8_t *buf, size_t len, size_t batch_size) */
   inline simdjson_result<document_stream> parse_many(const padded_string &s, size_t batch_size = DEFAULT_BATCH_SIZE) noexcept;
-
+  inline simdjson_result<document_stream> parse_many(const padded_string &&s, size_t batch_size) = delete;// unsafe
+  
   /** @private We do not want to allow implicit conversion from C string to std::string. */
   simdjson_result<document_stream> parse_many(const char *buf, size_t batch_size = DEFAULT_BATCH_SIZE) noexcept = delete;
 
diff --git a/tests/compilation_failure_tests/CMakeLists.txt b/tests/compilation_failure_tests/CMakeLists.txt
@@ -5,7 +5,7 @@
 # the macro COMPILATION_TEST_USE_FAILING_CODE set to 0 or 1.
 #
 
-# adds a compilation test. two targets are created, one expected to
+# adds a compilation test. Two targets are created, one expected to
 # succeed compilation and one that is expected to fail.
 function(add_dual_compile_test TEST_NAME)
   add_cpp_test(${TEST_NAME}_should_compile SOURCES ${TEST_NAME}.cpp COMPILE_ONLY)
@@ -22,4 +22,5 @@ if (SIMDJSON_EXCEPTIONS)
   add_dual_compile_test(dangling_parser_parse_uchar)
   add_dual_compile_test(dangling_parser_parse_stdstring)
   add_dual_compile_test(dangling_parser_parse_padstring)
+  add_dual_compile_test(unsafe_parse_many)
 endif()
diff --git a/tests/compilation_failure_tests/unsafe_parse_many.cpp b/tests/compilation_failure_tests/unsafe_parse_many.cpp
@@ -0,0 +1,47 @@
+#include <string>
+#include <vector>
+#include <iostream>
+
+#include "simdjson.h"
+
+bool single_document() {
+    std::cout << "Running " << __func__ << std::endl;
+    simdjson::dom::parser parser;
+    simdjson::dom::document_stream stream;
+
+#if COMPILATION_TEST_USE_FAILING_CODE
+    auto error = parser.parse_many(json).get(R"({"hello": "world"})"_padded);
+#else 
+    auto json = R"({"hello": "world"})"_padded;
+    auto error = parser.parse_many(json).get(stream);
+#endif 
+    if(error) {
+        std::cerr << error << std::endl;
+        return false;
+    }
+    size_t count = 0;
+    for (auto doc : stream) {
+        if(doc.error()) {
+          std::cerr << "Unexpected error: " << doc.error() << std::endl;
+          return false;
+        }
+        std::string expected = R"({"hello":"world"})";
+        simdjson::dom::element this_document;
+        error = doc.get(this_document);
+        if(error) {
+          std::cerr << error << std::endl;
+          return false;
+        }
+        std::string answer = simdjson::minify(this_document);
+        if(answer != expected) {
+          std::cout << this_document << std::endl;
+          return false;
+        }
+        count += 1;
+    }
+    return count == 1;
+}
+
+int main() {
+    return single_document() ? EXIT_SUCCESS : EXIT_FAILURE;
+}
