diff --git a/src/common/config/config_file.cpp b/src/common/config/config_file.cpp index 24572dfc13c..a187d3afa84 100644 --- a/src/common/config/config_file.cpp +++ b/src/common/config/config_file.cpp @@ -117,6 +117,8 @@ class MainStream : public ConfigFile::Stream class TextStream : public ConfigFile::Stream { public: + inline static constexpr const char* STREAM_NAME = "Passed text"; + explicit TextStream(const char* configText) : s(configText), l(0) { @@ -275,7 +277,7 @@ ConfigFile::Stream::~Stream() * Parse line, taking quotes into account */ -ConfigFile::LineType ConfigFile::parseLine(const char* fileName, const String& inputPar, Parameter& par) +ConfigFile::LineType ConfigFile::parseLine(const StreamName fileName, const String& inputPar, Parameter& par) { int inString = 0; String::size_type valStart = 0; @@ -449,7 +451,7 @@ void ConfigFile::adjustMacroReplacePositions(const String& value, const String& to += getDirSeparatorLength(value, to); } -bool ConfigFile::macroParse(String& value, const char* fileName) const +bool ConfigFile::macroParse(String& value, const StreamName fileName) const { String::size_type pos = 0; String::size_type subFrom; @@ -506,7 +508,7 @@ bool ConfigFile::macroParse(String& value, const char* fileName) const * Find macro value */ -bool ConfigFile::translate(const char* fileName, const String& from, String& to) const +bool ConfigFile::translate(const StreamName fileName, const String& from, String& to) const { if (from == "root") { @@ -518,19 +520,20 @@ bool ConfigFile::translate(const char* fileName, const String& from, String& to) } else if (from == "this") { - if (!fileName) + if (!fileName.has_value()) { return false; } - PathName tempPath(fileName); + const char* fileNameData = fileName.value_or(""); + PathName tempPath(fileNameData); #ifdef UNIX if (PathUtils::isSymLink(tempPath)) { // If $(this) is a symlink, expand it. TEXT temp[MAXPATHLEN]; - const int n = readlink(fileName, temp, sizeof(temp)); + const int n = readlink(fileNameData, temp, sizeof(temp)); if (n != -1) { @@ -539,7 +542,7 @@ bool ConfigFile::translate(const char* fileName, const String& from, String& to) if (PathUtils::isRelative(tempPath)) { PathName parent; - PathUtils::splitLastComponent(parent, tempPath, fileName); + PathUtils::splitLastComponent(parent, tempPath, fileNameData); PathUtils::concatPath(tempPath, parent, temp); } } @@ -640,9 +643,9 @@ const ConfigFile::Parameter* ConfigFile::findParameter(const KeyType& name, cons * Take into an account fault line */ -void ConfigFile::badLine(const char* fileName, const String& line) +void ConfigFile::badLine(const StreamName fileName, const String& line) { - (Arg::Gds(isc_conf_line) << (fileName ? fileName : "Passed text") << line).raise(); + (Arg::Gds(isc_conf_line) << fileName.value_or(TextStream::STREAM_NAME) << line).raise(); } /****************************************************************************** @@ -655,7 +658,7 @@ void ConfigFile::parse(Stream* stream) String inputLine; Parameter* previous = NULL; unsigned int line; - const char* streamName = stream->getFileName(); + const StreamName streamName = stream->getFileName(); parameters.setSortMode(FB_ARRAY_SORT_MANUAL); @@ -753,8 +756,10 @@ void ConfigFile::parse(Stream* stream) * Parse include operator */ -void ConfigFile::include(const char* currentFileName, const PathName& parPath) +void ConfigFile::include(const StreamName currentFileName, const PathName& parPath) { + const auto fileNameForError = currentFileName.value_or(TextStream::STREAM_NAME); + #ifdef DEBUG_INCLUDES fprintf(stderr, "include into %s file(s) %s\n", currentFileName, parPath.c_str()); #endif @@ -762,7 +767,7 @@ void ConfigFile::include(const char* currentFileName, const PathName& parPath) AutoSetRestore depth(&includeLimit, includeLimit + 1); if (includeLimit > INCLUDE_LIMIT) { - (Arg::Gds(isc_conf_include) << currentFileName << parPath << Arg::Gds(isc_include_depth)).raise(); + (Arg::Gds(isc_conf_include) << fileNameForError << parPath << Arg::Gds(isc_include_depth)).raise(); } // for relative paths first of all prepend with current path (i.e. path of current conf file) @@ -770,7 +775,7 @@ void ConfigFile::include(const char* currentFileName, const PathName& parPath) if (PathUtils::isRelative(parPath)) { PathName dummy; - PathUtils::splitLastComponent(path, dummy, currentFileName); + PathUtils::splitLastComponent(path, dummy, currentFileName.value_or("")); } PathUtils::concatPath(path, path, parPath); @@ -793,12 +798,12 @@ void ConfigFile::include(const char* currentFileName, const PathName& parPath) } // analyze components for wildcards - if (!wildCards(currentFileName, pathPrefix, components)) + if (!wildCards(pathPrefix, components)) { // no matches found - check for presence of wild symbols in path if (!hadWildCards) { - (Arg::Gds(isc_conf_include) << currentFileName << parPath << Arg::Gds(isc_include_miss)).raise(); + (Arg::Gds(isc_conf_include) << fileNameForError << parPath << Arg::Gds(isc_include_miss)).raise(); } } } @@ -811,7 +816,7 @@ void ConfigFile::include(const char* currentFileName, const PathName& parPath) * - returns true if some match was found */ -bool ConfigFile::wildCards(const char* currentFileName, const PathName& pathPrefix, FilesArray& components) +bool ConfigFile::wildCards(const PathName& pathPrefix, FilesArray& components) { // Any change in directory can cause config change PathName prefix(pathPrefix); @@ -852,7 +857,7 @@ bool ConfigFile::wildCards(const char* currentFileName, const PathName& pathPref if (mustBeDir) // should be directory { - found = wildCards(currentFileName, name, components) || found; + found = wildCards(name, components) || found; } else { @@ -959,4 +964,3 @@ bool ConfigFile::Parameter::asBoolean() const value.equalsNoCase("yes") || value.equalsNoCase("y"); } - diff --git a/src/common/config/config_file.h b/src/common/config/config_file.h index b10669419fc..ceffb51a6a8 100644 --- a/src/common/config/config_file.h +++ b/src/common/config/config_file.h @@ -28,6 +28,8 @@ #include "../common/classes/objects_array.h" #include "../common/classes/fb_string.h" #include "../common/classes/auto.h" +#include "../common/utils_proto.h" + /** Since the original (isc.cpp) code wasn't able to provide powerful and @@ -50,6 +52,8 @@ class ConfigCache; class ConfigFile : public Firebird::AutoStorage, public Firebird::RefCounted { + using StreamName = fb_utils::SafePointer; + public: // flags for config file static inline constexpr USHORT HAS_SUB_CONF = 0x01; @@ -125,7 +129,7 @@ class ConfigFile : public Firebird::AutoStorage, public Firebird::RefCounted } // Substitute macro values in a string - bool macroParse(String& value, const char* fileName) const; + bool macroParse(String& value, const StreamName fileName) const; private: enum LineType {LINE_BAD, LINE_REGULAR, LINE_START_SUB, LINE_END_SUB, LINE_INCLUDE}; @@ -139,11 +143,11 @@ class ConfigFile : public Firebird::AutoStorage, public Firebird::RefCounted // utilities bool getLine(Stream* stream, String&, unsigned int&); void parse(Stream* stream); - LineType parseLine(const char* fileName, const String& input, Parameter& par); - bool translate(const char* fileName, const String& from, String& to) const; - [[noreturn]] void badLine(const char* fileName, const String& line); - void include(const char* currentFileName, const Firebird::PathName& path); - bool wildCards(const char* currentFileName, const Firebird::PathName& pathPrefix, FilesArray& components); + LineType parseLine(const StreamName fileName, const String& input, Parameter& par); + bool translate(const StreamName fileName, const String& from, String& to) const; + [[noreturn]] void badLine(const StreamName fileName, const String& line); + void include(const StreamName currentFileName, const Firebird::PathName& path); + bool wildCards(const Firebird::PathName& pathPrefix, FilesArray& components); bool substituteStandardDir(const String& from, String& to) const; void adjustMacroReplacePositions(const String& value, const String& macro, String::size_type& from, String::size_type& to) const; unsigned getDirSeparatorLength(const String& value, String::size_type subFrom) const; diff --git a/src/common/tests/CommonFixtures.h b/src/common/tests/CommonFixtures.h new file mode 100644 index 00000000000..8956f9dcbc3 --- /dev/null +++ b/src/common/tests/CommonFixtures.h @@ -0,0 +1,32 @@ +#ifndef COMMON_FIXTURES +#define COMMON_FIXTURES +#include "boost/test/unit_test.hpp" + +#include "CommonUtils.h" + +#include + +namespace TestsUtils +{ + + namespace fs = std::filesystem; + + struct TempPathFixture + { + fs::path tempPathFX; + + TempPathFixture() + { + tempPathFX = fs::temp_directory_path() / (generateRandomString(10) + "_common_test.tmp"); + } + + ~TempPathFixture() + { + if (fs::exists(tempPathFX)) + fs::remove(tempPathFX); + } + }; + +} + +#endif diff --git a/src/common/tests/CommonUtils.h b/src/common/tests/CommonUtils.h new file mode 100644 index 00000000000..50f633b062b --- /dev/null +++ b/src/common/tests/CommonUtils.h @@ -0,0 +1,83 @@ +#ifndef TEST_COMMON_UTILS +#define TEST_COMMON_UTILS + +#include "firebird.h" +#include "fb_exception.h" + +#include "boost/test/unit_test.hpp" + +#include +#include +#include + +namespace TestsUtils +{ + inline std::string generateRandomString(std::size_t length) + { + std::random_device rd; + std::mt19937 generator(rd()); + + std::uniform_int_distribution<> distribution(0, 9); + + std::string randomString; + for (std::size_t i = 0; i < length; ++i) + { + randomString += '0' + distribution(generator); + } + + return randomString; + } + + // Use std::string because it works better with BOOST_TEST + inline std::string getErrorMessage(const Firebird::status_exception& ex) + { + const ISC_STATUS* status = ex.value(); + + std::string buffer; + TEXT temp[BUFFER_LARGE]; + while (fb_interpret(temp, sizeof(temp), &status)) + { + buffer += temp; + buffer += " "; + } + + if (!buffer.empty()) + buffer.resize(buffer.length() - 1); + + return buffer; + } + + // Wrapper to pass const char array as template argument + template + struct ConstexprString + { + char value[N]; + + constexpr ConstexprString(const char (&str)[N]) + { + std::copy_n(str, N, value); + } + + constexpr operator std::string_view() const + { + return std::string_view(value, N - 1); // Exclude '\0' + } + }; + + inline bool checkErrorMessage(const Firebird::status_exception& ex, const std::string_view expected) + { + const auto message = getErrorMessage(ex); + BOOST_TEST_INFO(std::string("Expected exception: ") + expected.data()); + BOOST_TEST_INFO("Caught exception: " + message); // Space for alignment + return message == expected; + } + + template + inline bool checkErrorMessage(const Firebird::status_exception& ex) + { + return checkErrorMessage(ex, static_cast(Expecter)); + } + +} + +#endif diff --git a/src/common/tests/ConfigFileTests.cpp b/src/common/tests/ConfigFileTests.cpp new file mode 100644 index 00000000000..9c3f589849e --- /dev/null +++ b/src/common/tests/ConfigFileTests.cpp @@ -0,0 +1,64 @@ +#include "boost/test/unit_test.hpp" +#include "../common/config/config_file.h" + +#include "CommonFixtures.h" +#include "CommonUtils.h" + +#include + +using namespace Firebird; + +BOOST_AUTO_TEST_SUITE(CommonClassesSuite) +BOOST_AUTO_TEST_SUITE(ConfigFileTests) + + +BOOST_AUTO_TEST_CASE(InvalidIncludeInTextStreamBug) +{ + MemoryPool& pool = *getDefaultMemoryPool(); + + const std::string_view text = R"( +database +{ +enabled = true +} +include /a/b/c/d/f.d + )"; + + + // Should be an exception, not a segfault + BOOST_CHECK_EXCEPTION(ConfigFile file({}, text.data(), 0), Firebird::status_exception, + TestsUtils::checkErrorMessage<"Invalid include operator in Passed text for File to include not found">); +} + +BOOST_FIXTURE_TEST_CASE(RecursiveInclude, TestsUtils::TempPathFixture) +{ + MemoryPool& pool = *getDefaultMemoryPool(); + const auto pathStr = tempPathFX.string(); + + std::string text = R"( +database +{ +enabled = true +} +include )"; + + text += pathStr; + + std::ofstream out(pathStr); + out << "include " + pathStr; + out.close(); + + Firebird::string error; + error.printf("Invalid include operator in %s for <%s> Include depth too big", pathStr.data(), pathStr.data()); + + // Should be an exception, not a segfault + BOOST_CHECK_EXCEPTION(ConfigFile file({}, text.data(), 0), Firebird::status_exception, + [&error](const Firebird::status_exception& ex) + { + return TestsUtils::checkErrorMessage(ex, error.data()); + }); +} + + +BOOST_AUTO_TEST_SUITE_END() // AutoPtrFunctionalTests +BOOST_AUTO_TEST_SUITE_END() // CommonClassesSuite diff --git a/src/common/utils_proto.h b/src/common/utils_proto.h index 06fbffc15a5..07d62e0bd3c 100644 --- a/src/common/utils_proto.h +++ b/src/common/utils_proto.h @@ -333,6 +333,29 @@ namespace fb_utils private: int reason; }; + + template + requires(!std::is_pointer_v) + class SafePointer + { + public: + constexpr SafePointer(T* value) : + m_value(value) + { } + + constexpr T* value_or(T* defaultValue) const noexcept + { + return m_value != nullptr ? m_value : defaultValue; + } + + constexpr bool has_value() const noexcept + { + return m_value != nullptr; + } + + private: + T* m_value; + }; } // namespace fb_utils #endif // INCLUDE_UTILS_PROTO_H