From 2addc7763556ff76809f14e2063d1452aa9d6275 Mon Sep 17 00:00:00 2001 From: Spartan322 Date: Sat, 3 Aug 2024 17:29:33 -0400 Subject: Fix crash for empty/invalid/not-found filepaths Add tests to validate empty string path Add tests to validate non-existent string path Add tests to validate nullptr buffer parser --- src/openvic-dataloader/AbstractSyntaxTree.hpp | 3 ++ src/openvic-dataloader/File.hpp | 9 ++++- src/openvic-dataloader/ParseState.hpp | 4 ++ src/openvic-dataloader/csv/Parser.cpp | 7 +--- src/openvic-dataloader/detail/ParseHandler.hpp | 56 +++++++++++++------------- src/openvic-dataloader/v2script/Parser.cpp | 7 +--- tests/src/csv/Parser.cpp | 24 +++++++++++ tests/src/v2script/Parser.cpp | 24 +++++++++++ 8 files changed, 95 insertions(+), 39 deletions(-) diff --git a/src/openvic-dataloader/AbstractSyntaxTree.hpp b/src/openvic-dataloader/AbstractSyntaxTree.hpp index f9f5796..f2d941b 100644 --- a/src/openvic-dataloader/AbstractSyntaxTree.hpp +++ b/src/openvic-dataloader/AbstractSyntaxTree.hpp @@ -23,6 +23,7 @@ namespace ovdl { struct AbstractSyntaxTree : SymbolIntern { + AbstractSyntaxTree() = default; explicit AbstractSyntaxTree(std::size_t max_elements) : _symbol_interner(max_elements) {} symbol_type intern(const char* str, std::size_t length); @@ -51,6 +52,8 @@ namespace ovdl { using root_node_type = RootNodeT; using node_type = typename file_type::node_type; + BasicAbstractSyntaxTree() = default; + explicit BasicAbstractSyntaxTree(file_type&& file) : AbstractSyntaxTree(file.size() * file.visit_buffer([](auto&& buffer) -> size_t { return sizeof(typename std::decay_t::char_type); })), _file { std::move(file) } {} diff --git a/src/openvic-dataloader/File.hpp b/src/openvic-dataloader/File.hpp index ec25640..cdab377 100644 --- a/src/openvic-dataloader/File.hpp +++ b/src/openvic-dataloader/File.hpp @@ -23,6 +23,7 @@ namespace ovdl { lexy::buffer, lexy::buffer>; + File() = default; explicit File(const char* path); const char* path() const noexcept; @@ -74,6 +75,7 @@ namespace ovdl { decltype(auto) visit_buffer(Visitor&& visitor) { switch (_buffer.index()) { SWITCH_LIST + case 0: return visitor(lexy::buffer<> {}); default: ovdl::detail::unreachable(); } } @@ -82,6 +84,7 @@ namespace ovdl { Return visit_buffer(Visitor&& visitor) { switch (_buffer.index()) { SWITCH_LIST + case 0: return visitor(lexy::buffer<> {}); default: ovdl::detail::unreachable(); } } @@ -90,6 +93,7 @@ namespace ovdl { decltype(auto) visit_buffer(Visitor&& visitor) const { switch (_buffer.index()) { SWITCH_LIST + case 0: return visitor(lexy::buffer<> {}); default: ovdl::detail::unreachable(); } } @@ -98,6 +102,7 @@ namespace ovdl { Return visit_buffer(Visitor&& visitor) const { switch (_buffer.index()) { SWITCH_LIST + case 0: return visitor(lexy::buffer<> {}); default: ovdl::detail::unreachable(); } } @@ -105,7 +110,7 @@ namespace ovdl { #undef SWITCH_LIST protected: - const char* _path; + const char* _path = ""; std::size_t _buffer_size = 0; detail::type_prepend_t _buffer; }; @@ -114,6 +119,8 @@ namespace ovdl { struct BasicFile : File { using node_type = NodeT; + BasicFile() = default; + template explicit BasicFile(const char* path, lexy::buffer&& buffer) : File(path) { diff --git a/src/openvic-dataloader/ParseState.hpp b/src/openvic-dataloader/ParseState.hpp index 806829c..259f2a5 100644 --- a/src/openvic-dataloader/ParseState.hpp +++ b/src/openvic-dataloader/ParseState.hpp @@ -30,6 +30,8 @@ namespace ovdl { using file_type = typename ast_type::file_type; using diagnostic_logger_type = BasicDiagnosticLogger; + ParseState() : _ast {}, _logger { this->ast().file() } {} + ParseState(typename ast_type::file_type&& file, detail::Encoding encoding) : _ast { std::move(file) }, _logger { this->ast().file() }, @@ -69,6 +71,8 @@ namespace ovdl { using file_type = FileT; using diagnostic_logger_type = BasicDiagnosticLogger; + FileParseState() : _file {}, _logger { this->file() } {} + FileParseState(file_type&& file, detail::Encoding encoding) : _file { std::move(file) }, _logger { this->file() }, diff --git a/src/openvic-dataloader/csv/Parser.cpp b/src/openvic-dataloader/csv/Parser.cpp index bbd1be4..e4416b2 100644 --- a/src/openvic-dataloader/csv/Parser.cpp +++ b/src/openvic-dataloader/csv/Parser.cpp @@ -57,9 +57,6 @@ struct Parser::ParseHandler final : detail::BasicFileParseHandler } Parser::error_range get_errors() { - using iterator = typename decltype(std::declval()->children())::iterator; - if (!is_valid()) - return dryad::make_node_range(iterator::from_ptr(nullptr), iterator::from_ptr(nullptr)); return parse_state().logger().get_errors(); } @@ -129,7 +126,7 @@ constexpr void Parser::_run_load_func(detail::LoadCallbackparse_state().logger().template create_log(DiagnosticLogger::DiagnosticKind::error, fmt::runtime(error_message)); + _parse_handler->parse_state().logger().template create_log(DiagnosticLogger::DiagnosticKind::error, fmt::runtime(error_message), _file_path); } if (has_error() && &_error_stream.get() != &detail::cnull) { print_errors_to(_error_stream.get()); @@ -235,7 +232,7 @@ void Parser::print_errors_to(std::basic_ostream& stream) const { dryad::visit_tree( error, [&](const error::BufferError* buffer_error) { - stream << "buffer error: " << this->error(buffer_error) << '\n'; + stream << this->error(buffer_error) << '\n'; }, [&](dryad::child_visitor visitor, const error::AnnotatedError* annotated_error) { stream << this->error(annotated_error) << '\n'; diff --git a/src/openvic-dataloader/detail/ParseHandler.hpp b/src/openvic-dataloader/detail/ParseHandler.hpp index 9666a5b..28222ae 100644 --- a/src/openvic-dataloader/detail/ParseHandler.hpp +++ b/src/openvic-dataloader/detail/ParseHandler.hpp @@ -26,11 +26,11 @@ namespace ovdl::detail { case buffer_is_null: return "Buffer could not be loaded."; case os_error: - return "OS file error."; + return "OS file error for '{}'."; case file_not_found: - return "File not found."; + return "File '{}' not found."; case permission_denied: - return "Read Permission denied."; + return "Read Permission for '{}' denied."; default: return ""; } @@ -78,19 +78,20 @@ namespace ovdl::detail { virtual const char* path_impl() const = 0; template - static constexpr auto generate_state = [](std::optional* state, const char* path, auto&& buffer, Encoding encoding) { + static constexpr auto generate_state = [](State* state, const char* path, auto&& buffer, Encoding encoding) { if (path[0] != '\0') { - state->emplace( + *state = { path, lexy::buffer(std::move(buffer)), - encoding); + encoding + }; return; } - state->emplace(lexy::buffer(std::move(buffer)), encoding); + *state = { lexy::buffer(std::move(buffer)), encoding }; }; template - static void create_state(std::optional* state, const char* path, lexy::buffer&& buffer, std::optional fallback) { + static void create_state(State* state, const char* path, lexy::buffer&& buffer, std::optional fallback) { if (!_system_fallback_encoding.has_value()) { _detect_system_fallback_encoding(); } @@ -121,15 +122,15 @@ namespace ovdl::detail { } if (!is_alone) { - (*state)->logger().info("encoding type could not be distinguished"); + state->logger().info("encoding type could not be distinguished"); } if (is_bad_fallback) { - (*state)->logger().warning("fallback encoding cannot be ascii or utf8"); + state->logger().warning("fallback encoding cannot be ascii or utf8"); } if (encoding == ovdl::detail::Encoding::Unknown) { - (*state)->logger().warning("could not detect encoding"); + state->logger().warning("could not detect encoding"); } } @@ -143,36 +144,37 @@ namespace ovdl::detail { using parse_state_type = ParseState; virtual constexpr bool is_valid_impl() const { - if (!_parse_state) return false; - return _parse_state.value().file().is_valid(); + return _parse_state.file().is_valid(); } constexpr virtual buffer_error load_buffer_impl(lexy::buffer&& buffer, const char* path, std::optional fallback) { - if (buffer.data() == nullptr) return buffer_error::buffer_is_null; + if (buffer.data() == nullptr) { + _parse_state = {}; + return buffer_error::buffer_is_null; + } create_state(&_parse_state, path, std::move(buffer), fallback); return is_valid_impl() ? buffer_error::success : buffer_error::buffer_is_null; } virtual const char* path_impl() const { - if (!_parse_state) return ""; - return _parse_state.value().file().path(); + return _parse_state.file().path(); } parse_state_type& parse_state() { - return _parse_state.value(); + return _parse_state; } const parse_state_type& parse_state() const { - return _parse_state.value(); + return _parse_state; } template constexpr const auto& buffer() const { - return _parse_state.value().file().template get_buffer_as(); + return _parse_state.file().template get_buffer_as(); } protected: - std::optional _parse_state; + parse_state_type _parse_state; }; template @@ -180,8 +182,7 @@ namespace ovdl::detail { using parse_state_type = ParseState; virtual constexpr bool is_valid_impl() const { - if (!_parse_state) return false; - return _parse_state.value().ast().file().is_valid(); + return _parse_state.ast().file().is_valid(); } constexpr virtual buffer_error load_buffer_impl(lexy::buffer&& buffer, const char* path, std::optional fallback) { @@ -191,24 +192,23 @@ namespace ovdl::detail { } virtual const char* path_impl() const { - if (!_parse_state) return ""; - return _parse_state.value().ast().file().path(); + return _parse_state.ast().file().path(); } parse_state_type& parse_state() { - return _parse_state.value(); + return _parse_state; } const parse_state_type& parse_state() const { - return _parse_state.value(); + return _parse_state; } template constexpr const auto& buffer() const { - return _parse_state.value().ast().file().template get_buffer_as(); + return _parse_state.ast().file().template get_buffer_as(); } protected: - std::optional _parse_state; + parse_state_type _parse_state; }; } \ No newline at end of file diff --git a/src/openvic-dataloader/v2script/Parser.cpp b/src/openvic-dataloader/v2script/Parser.cpp index e5234d6..9b36a95 100644 --- a/src/openvic-dataloader/v2script/Parser.cpp +++ b/src/openvic-dataloader/v2script/Parser.cpp @@ -77,9 +77,6 @@ struct Parser::ParseHandler final : detail::BasicStateParseHandler()->children())::iterator; - if (!is_valid()) - return dryad::make_node_range(iterator::from_ptr(nullptr), iterator::from_ptr(nullptr)); return parse_state().logger().get_errors(); } }; @@ -146,7 +143,7 @@ constexpr void Parser::_run_load_func(detail::LoadCallbackparse_state().logger().template create_log(DiagnosticLogger::DiagnosticKind::error, fmt::runtime(error_message)); + _parse_handler->parse_state().logger().template create_log(DiagnosticLogger::DiagnosticKind::error, fmt::runtime(error_message), _file_path); } if (has_error() && &_error_stream.get() != &detail::cnull) { print_errors_to(_error_stream.get()); @@ -356,7 +353,7 @@ void Parser::print_errors_to(std::basic_ostream& stream) const { dryad::visit_tree( error, [&](const error::BufferError* buffer_error) { - stream << "buffer error: " << this->error(buffer_error) << '\n'; + stream << this->error(buffer_error) << '\n'; }, [&](dryad::child_visitor visitor, const error::AnnotatedError* annotated_error) { stream << this->error(annotated_error) << '\n'; diff --git a/tests/src/csv/Parser.cpp b/tests/src/csv/Parser.cpp index fe26726..88a1474 100644 --- a/tests/src/csv/Parser.cpp +++ b/tests/src/csv/Parser.cpp @@ -48,6 +48,14 @@ TEST_CASE("CSV Memory Buffer (begin, end) Parse", "[csv-memory-parse][buffer][be CHECK_PARSE(false); } +TEST_CASE("CSV Buffer nullptr Parse", "[csv-memory-parse][buffer][nullptr]") { + Parser parser(ovdl::detail::cnull); + + parser.load_from_buffer(nullptr, std::size_t { 0 }); + + CHECK_PARSE(true); +} + TEST_CASE("CSV Memory String Parse", "[csv-memory-parse][string]") { Parser parser(ovdl::detail::cnull); @@ -152,6 +160,22 @@ TEST_CASE("CSV File (HasCstr) Handle String Parse", "[csv-file-parse][handle-str CHECK_PARSE(true); } +TEST_CASE("CSV File (const char*) Handle Empty Path String Parse", "[csv-file-parse][handle-string][char-ptr][empty-path]") { + Parser parser(ovdl::detail::cnull); + + parser.load_from_file(""); + + CHECK_OR_RETURN(!parser.get_errors().empty()); +} + +TEST_CASE("CSV File (const char*) Handle Non-existent Path String Parse", "[csv-file-parse][handle-string][char-ptr][nonexistent-path]") { + Parser parser(ovdl::detail::cnull); + + parser.load_from_file("./Idontexist"); + + CHECK_OR_RETURN(!parser.get_errors().empty()); +} + TEST_CASE("CSV Parse", "[csv-parse]") { Parser parser(ovdl::detail::cnull); diff --git a/tests/src/v2script/Parser.cpp b/tests/src/v2script/Parser.cpp index 5ddc49d..856e5fb 100644 --- a/tests/src/v2script/Parser.cpp +++ b/tests/src/v2script/Parser.cpp @@ -54,6 +54,14 @@ TEST_CASE("V2Script Memory Buffer String Simple Parse", "[v2script-memory-simple CHECK_PARSE(); } +TEST_CASE("V2Script Buffer nullptr Simple Parse", "[v2script-memory-simple-parse][buffer][nullptr]") { + Parser parser(ovdl::detail::cnull); + + parser.load_from_buffer(nullptr, std::size_t { 0 }); + + CHECK_PARSE(); +} + TEST_CASE("V2Script File (const char*) Simple Parse", "[v2script-file-simple-parse][char-ptr]") { SetupFile(simple_path); @@ -90,6 +98,22 @@ TEST_CASE("V2Script File (HasCstr) Simple Parse", "[v2script-file-simple-parse][ CHECK_PARSE(); } +TEST_CASE("V2Script File (const char*) Handle Empty Path String Parse", "[v2script-file-parse][handle-string][char-ptr][empty-path]") { + Parser parser(ovdl::detail::cnull); + + parser.load_from_file(""); + + CHECK_OR_RETURN(!parser.get_errors().empty()); +} + +TEST_CASE("V2Script File (const char*) Handle Non-existent Path String Parse", "[v2script-file-parse][handle-string][char-ptr][nonexistent-path]") { + Parser parser(ovdl::detail::cnull); + + parser.load_from_file("./Idontexist"); + + CHECK_OR_RETURN(!parser.get_errors().empty()); +} + TEST_CASE("V2Script Identifier Simple Parse", "[v2script-id-simple-parse]") { Parser parser(ovdl::detail::cnull); -- cgit v1.2.3-56-ga3b1