diff --git a/src/common/config/config_file.cpp b/src/common/config/config_file.cpp index 017f7d99a9c..74e081165dc 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); @@ -756,8 +759,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 @@ -765,7 +770,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) @@ -773,7 +778,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); @@ -796,12 +801,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(); } } } @@ -814,7 +819,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); @@ -855,7 +860,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 { @@ -962,4 +967,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 cb0e9bdf07d..939aaadc83e 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; @@ -126,7 +130,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}; @@ -140,11 +144,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 index 6542b65574f..de59158d9fd 100644 --- a/src/common/tests/CommonFixtures.h +++ b/src/common/tests/CommonFixtures.h @@ -2,37 +2,26 @@ #define COMMON_FIXTURES #include "boost/test/unit_test.hpp" +#include "CommonUtils.h" + #include -#include namespace TestsUtils { namespace fs = std::filesystem; -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; -} - struct TempPathFixture { fs::path tempPathFX; TempPathFixture() { - tempPathFX = fs::temp_directory_path() / (generateRandomString(10) + "_common_test.tmp"); + auto tempDir = fs::temp_directory_path(); + // Resolve symlink (/var on macos) + tempDir = fs::canonical(tempDir); + + tempPathFX = tempDir / (generateRandomString(10) + "_common_test.tmp"); } ~TempPathFixture() 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/ConfigFileTest.cpp b/src/common/tests/ConfigFileTest.cpp index 1727324a151..27f29846907 100644 --- a/src/common/tests/ConfigFileTest.cpp +++ b/src/common/tests/ConfigFileTest.cpp @@ -27,6 +27,53 @@ BOOST_FIXTURE_TEST_CASE(IncludeInUserSessionBug, TestsUtils::TempPathFixture) BOOST_CHECK_THROW(ConfigFile file({}, text.data(), ConfigFile::DENY_INCLUDE), Firebird::Exception); } +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 2f78a013aa0..8b323c2841e 100644 --- a/src/common/utils_proto.h +++ b/src/common/utils_proto.h @@ -335,6 +335,29 @@ namespace fb_utils }; bool isLoopbackAddress(const std::string_view address); + + 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