From 148ff028598ae5c99ecf84393bfffe1640cce7df Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Thu, 10 Sep 2026 11:02:00 +0200 Subject: [PATCH] Fix to_string() on eof marker token (#9463) * Rewrite to_string() to use switch{} and handle Token_NONE and identifier without as_string() * Add regression tests for to_string() on tokens without a string form Cover both halves of the recursion that made a whitespace-only file segfault: token::to_string() on the EOF marker and on an instance name, and a parse of input that lexes to zero tokens, which is how the header parser reaches token::as_string() on the EOF marker. Generated with the assistance of an AI coding tool. --------- Co-authored-by: Bruno Postle --- src/ifcparse/parse.cpp | 35 +++++++++++++------ .../tests/test_ifcopenshell_parse.cpp | 34 ++++++++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/src/ifcparse/parse.cpp b/src/ifcparse/parse.cpp index 5917ae89c3..8ab7fe12e5 100644 --- a/src/ifcparse/parse.cpp +++ b/src/ifcparse/parse.cpp @@ -536,19 +536,34 @@ boost::dynamic_bitset<> token::as_binary() { } std::string token::to_string() { - std::string result; - if (type == Token_OPERATOR || type == Token_BOOL) { - result.push_back(value_char); - } else if (type == Token_INT) { - result = std::to_string(value_int); - } else if (type == Token_FLOAT) { + switch (type) { + case Token_OPERATOR: + case Token_BOOL: + return std::string(1, value_char); + + case Token_INT: + return std::to_string(value_int); + + case Token_IDENTIFIER: + return "#" + std::to_string(value_int); + + case Token_FLOAT: { std::ostringstream oss; oss << std::setprecision(15) << value_double; - result = oss.str(); - } else { - return as_string(); + return oss.str(); } - return result; + + case Token_STRING: + case Token_ENUMERATION: + case Token_BINARY: + case Token_KEYWORD: + return as_string(); + + case Token_NONE: + throw invalid_token_exception(start_pos, "", ""); + } + + throw exception("Unknown token type"); } std::string ifcopenshell::encode_spf_string(const std::string& value) { diff --git a/src/ifcparse/tests/test_ifcopenshell_parse.cpp b/src/ifcparse/tests/test_ifcopenshell_parse.cpp index af93cc3a1b..f451bf5e88 100644 --- a/src/ifcparse/tests/test_ifcopenshell_parse.cpp +++ b/src/ifcparse/tests/test_ifcopenshell_parse.cpp @@ -1,8 +1,11 @@ // This file was generated with the assistance of an AI coding tool. #include +#include #include #include +#include +#include #include #include @@ -97,6 +100,37 @@ TEST_CASE("Aggregate inverse updates preserve reference multiplicity", "[ifcpars CHECK(inverse_count(segment_d) == 1); } +TEST_CASE("Tokens without a string representation do not recurse in to_string()", "[ifcparse]") { + // to_string() used to delegate to as_string() for every token type it did + // not handle explicitly, while as_string() builds its exception message + // with to_string(). An EOF marker or an instance name therefore recursed + // between the two until the stack was exhausted. + ifcopenshell::token eof; + REQUIRE(eof.type == ifcopenshell::token::Token_NONE); + CHECK_THROWS_AS(eof.to_string(), ifcopenshell::invalid_token_exception); + CHECK_THROWS_AS(eof.as_string(), ifcopenshell::invalid_token_exception); + + ifcopenshell::token identifier(0, ifcopenshell::token::Token_IDENTIFIER, (int64_t)123); + CHECK(identifier.to_string() == "#123"); + CHECK_THROWS_AS(identifier.as_string(), ifcopenshell::invalid_token_exception); +} + +TEST_CASE("Files that contain no tokens are rejected rather than crashing", "[ifcparse]") { + // The header parser asks the lexer for a keyword before checking for EOF, + // so input that lexes to zero tokens reaches token::as_string() on the EOF + // marker. Parsing must fail cleanly instead of overflowing the stack. + const std::vector inputs{" ", "\r\n\t ", "/* only a comment */"}; + + for (const auto& contents : inputs) { + INFO("input: " << contents); + ifcopenshell::logger log; + std::istringstream input(contents); + ifcopenshell::file file(input, (int)contents.size(), log); + + CHECK(file.good().value() != ifcopenshell::file_open_status::SUCCESS); + } +} + TEST_CASE("Inverse lookups stay consistent across interleaved adds, removals and reads", "[ifcparse]") { ifcopenshell::file file(ifcopenshell::schema_by_name("IFC4")); const auto* point_declaration = file.schema()->declaration_by_name("IfcCartesianPoint");