Segmentation fault toml::json_formatter for empty document.

[proydakov]

Hi, Mark Gillard.

Many thx for hight quality C++ TOML parser library. It looks I found a corner case. See description below.

Environment

master

Compiler:
AppleClang12 BigSur 11.3

C++ standard mode (e.g. 17, 20, 'latest'):
20

Target arch (e.g. x64):
x64

Library configuration overrides:
Default.

Relevant compilation flags:
-fno-exceptions

Describe the bug

Process 18945 stopped

• thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x41)
  frame #0: 0x0000000100035bbf toml_demo`toml::v2::json_formatter::print(this=0x00007ffeefbff530) at toml_json_formatter.h:91:47
  88
  89 void print()
  90 {
  -> 91 switch (auto source_type = base::source().type())
  92 {
  93 case node_type::table:
  94 print(reinterpret_cast<const table>(&base::source()));
  Target 0: (toml_demo) stopped.

Steps to reproduce (or a small repro code sample)

Compile code:

auto config = toml::parse_file( FNAME ); <-- no real TOML file here.
std::cout << toml::json_formatter{ config } << "\n";

Additional information

I think it is better to print empty json in this case: {}
What do you think about it?

Best regards, Proydakov Evgeny.

[marzer]

Thanks for the report!

I think it is better to print empty json in this case

Yup, definitely. The fact that it doesn't already behave that way is an oversight on my part.

[marzer]

@proydakov can you clarify something for me? You said you're using -fno-exceptions, which means that the return value for toml::parse_file is not a toml::table (the root document), but is in fact a toml::parse_result, which you need to check for success/failure before attempting to do anything with.

Trying to read the root TOML table from a parse_result that contains a failure result is an error, because a toml::parse_result is a discriminated union of a toml::table and a toml::parse_error; it only models one or the other at any given time.

I can special-case this in the formatter classes but you should also be aware of this behaviour if you're working without exceptions.

See also: Working without exceptions

[marzer]

Ok, so a follow-up. I've checked the code-paths for blank documents in the JSON formatter and they do actually already print {}, as you'd expect. The issue you're experiencing is the one I describe above. You can verify this yourself:

#define TOML_EXCEPTIONS 0
#include <toml++/toml.h>

using namespace std::string_view_literals;

int main() {

// this will print '{}', because the returned toml::parse_result contains a successful parse result
// (an blank TOML document is valid TOML)
std::cout << toml::json_formatter{ toml::parse(""sv) } << "\n";

// this will crash because the returned toml::parse_result contains a failed parse result
// (a file not existing is an error, not the same as an blank document!)
std::cout << toml::json_formatter{ toml::parse_file("this_file_does_not_exist.toml"sv) } << "\n";

}

I can add some special-cases for the formatters so they know how to handle toml::parse_result (rather than rely on the implicit conversion to toml::table), though in the case of an error result they'll just print the error reason, I think. It will be malformed JSON, but it will be something that can at least appear in logs and diagnostics.

[proydakov]

I just copied example from Reamde::'Basic usage'. Inserted a non-existent file name and got a crash when trying to output the document in JSON, but TOML formatter works fine. See full code example below:

  #define TOML_EXCEPTIONS 0
  
  #include <toml++/toml.h>
  
  #include <fstream>
  #include <iostream>
  #include <string_view>
    
  int main()
  {
      auto config = toml::parse_file( "this_file_does_not_exist.toml" );
  
      // get key-value pairs
      std::string_view library_name = config["library"]["name"].value_or(std::string_view(""));
      if (config["library"]["authors"].is_array())
      {
          std::cout << "len: " << config["library"]["authors"].as_array()->size() << "\n";
      }
      std::string_view library_author = config["library"]["authors"][0].value_or(std::string_view(""));
      int64_t depends_on_cpp_version = config["dependencies"]["cpp"].value_or(0);
  
      std::cout << "library_name: " << library_name << "\n";
      std::cout << "library_author: " << library_author << "\n";
      std::cout << "depends_on_cpp_version: " << depends_on_cpp_version << "\n";
  
      std::cout << "\nRANGLE LOOP:\n\n";
  
      // iterate & visit over the data
      for (auto const& [k, v] : config)
      {
          v.visit([](auto& node) noexcept
          {
              std::cout << node << "\n";
          });
      }
  
      std::cout << "\nTOML:\n\n";
  
      // re-serialize as TOML
      std::cout << config << "\n";
  
      std::cout << "\nJSON:\n\n";
  
      std::cout << toml::json_formatter{ config } << "\n";
  
      return 0;
  }

I thought that maybe it was worth not to segfault, but to output an empty document.
I will add a result parsing check to my project code.

evgeny.proydakov@MacBook-Pro-i5-gen10 build % ./toml_demo 
library_name: 
library_author: 
depends_on_cpp_version: 0

RANGLE LOOP:


TOML:

File could not be opened for reading
	(error occurred at line 0, column 0 of 'this_file_does_not_exist.toml')

JSON:

zsh: segmentation fault  ./toml_demo

[marzer]

Inserted a non-existent file name and got a crash when trying to output the document in JSON, but TOML formatter works fine

@proydakov I pushed a fix for this at about the same time you wrote your reply; are you still experiencing this behaviour with the new version in master? I was unable to reproduce it after applying the fix.

I thought that maybe it was worth not to segfault, but to output an empty document.

Naturally I don't want users to segfault, but outputting an empty TOML or JSON isn't ideal either because it implies the source document was valid and simply empty; a missing file isn't valid, so the output shouldn't be either. I've chosen instead to simply emit the error message in this situation.

[proydakov]

Ok. Let me check new revision.

[proydakov]

Looks good. Many thx @marzer

evgeny.proydakov@MacBook-Pro-i5-gen10 build % ./toml_demo
library_name: 
library_author: 
depends_on_cpp_version: 0

RANGLE LOOP:


TOML:

File could not be opened for reading
	(error occurred at line 0, column 0 of 'this_file_does_not_exist.toml')

JSON:

File could not be opened for reading
	(error occurred at line 0, column 0 of 'this_file_does_not_exist.toml')

[marzer]

Ah, great news. Thanks for the report!

[uyplayer]

Looks good. Many thx @marzer

evgeny.proydakov@MacBook-Pro-i5-gen10 build % ./toml_demo
library_name: 
library_author: 
depends_on_cpp_version: 0

RANGLE LOOP:


TOML:

File could not be opened for reading
	(error occurred at line 0, column 0 of 'this_file_does_not_exist.toml')

JSON:

File could not be opened for reading
	(error occurred at line 0, column 0 of 'this_file_does_not_exist.toml')

i get same error , how to fix them ?

[marzer]

@uyplayer if you got the error message above then you're likely trying to open a file that does not exist.

If that's not the case, I have no idea? You've provided very little information. Please open an issue in that case.