nlohmann / json

JSON for Modern C++
https://json.nlohmann.me
MIT License
41.33k stars 6.58k forks source link

Exception SIGSEGV - Segmentation violation signal on file parsing (v3.11.2, linux, doctest) #4193

Closed obewan closed 8 months ago

obewan commented 8 months ago

Description

Using a valid json file, when using parsing, it got a SIGSEGV exception, on linux, with the v3.11.2. Here the Json file: { "a": [
{ "b": [ { "c": [ 0.10000000149011612, 0.10000000149011612,
0.0864868313074112 ] } ] } ], "params": { "d": 0.0010000000474974513 }, "version": "1.0.0" }

Here the line that crash: using json = nlohmann::json; std::ifstream file(the_file); //<< OK if (!file.is_open()) { return; } //<< OK if (!json::accept(file)) { return; ] //<< OK nlohmann::json json_model = json::parse(file); //<< CRASH

Reproduction steps

I got a testModel.json file: { "a": [
{ "b": [ { "c": [ 0.10000000149011612, 0.10000000149011612,
0.0864868313074112 ] } ] } ], "params": { "d": 0.0010000000474974513 }, "version": "1.0.0" }

And an import function that contain: std::ifstream file(path_in); if (!file.is_open()) { throw ImportExportJSONException("Failed to open file for reading."); } if (!json::accept(file)) { throw ImportExportJSONException("JSON parsing error: invalid JSON file."); } try { nlohmann::json json_model = json::parse(file); } catch (...) { throw ImportExportJSONException("JSON parsing error"); }

And a doctest test that crash with:

SUBCASE("Test importModel function") { ImportExportJSON importExportJSON; CHECK_NOTHROW(importExportJSON.importModel(modelJsonFile)); } Saying: Exception: "JSON parsing error" Exception (crash=true): SIGSEGV - Segmentation violation signal

Expected vs. actual results

Expected parsing file without crash. The json dump to file works fine on the other hand.

Minimal code example

using json = nlohmann::json;
std::ifstream file(the_file);  //<< OK
if (!file.is_open()) { return;  }  //<< OK
if (!json::accept(file)) { return; ] //<< OK
nlohmann::json json_model = json::parse(file); //<< CRASH

Error messages

SIGSEGV - Segmentation violation signal

Compiler and operating system

gcc version 10.2.1 20210110 (Debian 10.2.1-6)

Library version

3.11.2

Validation

obewan commented 8 months ago

Ok, I finally resolve this issue with several steps:

  1. I add some lines for cmake in my CMakeLists.txt of my test project (you may not use doctest and some different path):

    add_library(nlohmann-json INTERFACE)
    target_sources(nlohmann-json INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/../../json/include/json.hpp)
    add_library(lib_doctest INTERFACE)
    target_sources(lib_doctest INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}/include/doctest.h)
    set(LIBRARY_HEADERS_DIR include)
    target_include_directories(lib_doctest INTERFACE  "${CMAKE_CURRENT_SOURCE_DIR}/${LIBRARY_HEADERS_DIR}")
    file(GLOB SOURCES "*.cpp")
    add_executable(tester ${SOURCES}) 
    target_link_libraries(tester PRIVATE ${LIBRARY_NAME} lib_doctest nlohmann-json)

    So then there will no more SIGSEGV exception, but still a json parsing error, so step 2:

  2. I remove the json::accept(file) test, so the file descriptor is still at the beginning of the file, not at the end (or else you will have to reset its pos, which is another possibility). But even then, there's a step 3:

  3. Add a relative path "./" to the filename (was without path), so even if before it read it, now it read it and parse it: std::string modelJsonFile = "./testModel.json"; //must add "./"

And now it's all good. Quite tricky though, it isn't indicated clearly in the doc.

nlohmann commented 8 months ago

What is missing in the doc?

obewan commented 8 months ago

We could have more info about the cmake requirement. For the accept(file) I recognize it's logic to let the file at the end of file, though we could have been advice, and for the relative path, I didn't see that requirement except on another issue (https://github.com/nlohmann/json/issues/2066). Great json lib though.

gregmarr commented 8 months ago

for the relative path, I didn't see that requirement except on another issue

That's not a requirement of the library, that's a requirement of your compiler's std::ifstream implementation.

nlohmann commented 8 months ago