diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 4d3c6f4d6f66..d9b1ab6928db 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -62,6 +62,7 @@ BITCOIN_TESTS =\ test/dip0020opcodes_tests.cpp \ test/evo_deterministicmns_tests.cpp \ test/evo_simplifiedmns_tests.cpp \ + test/fs_tests.cpp \ test/getarg_tests.cpp \ test/governance_validators_tests.cpp \ test/hash_tests.cpp \ diff --git a/src/fs.cpp b/src/fs.cpp index c2dfa7b8da44..cfb6066101b4 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -102,4 +102,122 @@ bool FileLock::TryLock() } #endif +std::string get_filesystem_error_message(const fs::filesystem_error& e) +{ +#ifndef WIN32 + return e.what(); +#else + // Convert from Multi Byte to utf-16 + std::string mb_string(e.what()); + int size = MultiByteToWideChar(CP_ACP, 0, mb_string.c_str(), mb_string.size(), nullptr, 0); + + std::wstring utf16_string(size, L'\0'); + MultiByteToWideChar(CP_ACP, 0, mb_string.c_str(), mb_string.size(), &*utf16_string.begin(), size); + // Convert from utf-16 to utf-8 + return std::wstring_convert, wchar_t>().to_bytes(utf16_string); +#endif +} + +#ifdef WIN32 +#ifdef __GLIBCXX__ + +// reference: https://github.com/gcc-mirror/gcc/blob/gcc-7_3_0-release/libstdc%2B%2B-v3/include/std/fstream#L270 + +static std::string openmodeToStr(std::ios_base::openmode mode) +{ + switch (mode & ~std::ios_base::ate) { + case std::ios_base::out: + case std::ios_base::out | std::ios_base::trunc: + return "w"; + case std::ios_base::out | std::ios_base::app: + case std::ios_base::app: + return "a"; + case std::ios_base::in: + return "r"; + case std::ios_base::in | std::ios_base::out: + return "r+"; + case std::ios_base::in | std::ios_base::out | std::ios_base::trunc: + return "w+"; + case std::ios_base::in | std::ios_base::out | std::ios_base::app: + case std::ios_base::in | std::ios_base::app: + return "a+"; + case std::ios_base::out | std::ios_base::binary: + case std::ios_base::out | std::ios_base::trunc | std::ios_base::binary: + return "wb"; + case std::ios_base::out | std::ios_base::app | std::ios_base::binary: + case std::ios_base::app | std::ios_base::binary: + return "ab"; + case std::ios_base::in | std::ios_base::binary: + return "rb"; + case std::ios_base::in | std::ios_base::out | std::ios_base::binary: + return "r+b"; + case std::ios_base::in | std::ios_base::out | std::ios_base::trunc | std::ios_base::binary: + return "w+b"; + case std::ios_base::in | std::ios_base::out | std::ios_base::app | std::ios_base::binary: + case std::ios_base::in | std::ios_base::app | std::ios_base::binary: + return "a+b"; + default: + return std::string(); + } +} + +void ifstream::open(const fs::path& p, std::ios_base::openmode mode) +{ + close(); + m_file = fsbridge::fopen(p, openmodeToStr(mode).c_str()); + if (m_file == nullptr) { + return; + } + m_filebuf = __gnu_cxx::stdio_filebuf(m_file, mode); + rdbuf(&m_filebuf); + if (mode & std::ios_base::ate) { + seekg(0, std::ios_base::end); + } +} + +void ifstream::close() +{ + if (m_file != nullptr) { + m_filebuf.close(); + fclose(m_file); + } + m_file = nullptr; +} + +void ofstream::open(const fs::path& p, std::ios_base::openmode mode) +{ + close(); + m_file = fsbridge::fopen(p, openmodeToStr(mode).c_str()); + if (m_file == nullptr) { + return; + } + m_filebuf = __gnu_cxx::stdio_filebuf(m_file, mode); + rdbuf(&m_filebuf); + if (mode & std::ios_base::ate) { + seekp(0, std::ios_base::end); + } +} + +void ofstream::close() +{ + if (m_file != nullptr) { + m_filebuf.close(); + fclose(m_file); + } + m_file = nullptr; +} +#else // __GLIBCXX__ + +static_assert(sizeof(*fs::path().BOOST_FILESYSTEM_C_STR) == sizeof(wchar_t), + "Warning: This build is using boost::filesystem ofstream and ifstream " + "implementations which will fail to open paths containing multibyte " + "characters. You should delete this static_assert to ignore this warning, " + "or switch to a different C++ standard library like the Microsoft C++ " + "Standard Library (where boost uses non-standard extensions to construct " + "stream objects with wide filenames), or the GNU libstdc++ library (where " + "a more complicated workaround has been implemented above)."); + +#endif // __GLIBCXX__ +#endif // WIN32 + } // fsbridge diff --git a/src/fs.h b/src/fs.h index e3ff51604d92..cddcae5a7313 100644 --- a/src/fs.h +++ b/src/fs.h @@ -7,6 +7,9 @@ #include #include +#if defined WIN32 && defined __GLIBCXX__ +#include +#endif #include #include @@ -39,6 +42,56 @@ namespace fsbridge { void* hFile = (void*)-1; // INVALID_HANDLE_VALUE #endif }; + + std::string get_filesystem_error_message(const fs::filesystem_error& e); + + // GNU libstdc++ specific workaround for opening UTF-8 paths on Windows. + // + // On Windows, it is only possible to reliably access multibyte file paths through + // `wchar_t` APIs, not `char` APIs. But because the C++ standard doesn't + // require ifstream/ofstream `wchar_t` constructors, and the GNU library doesn't + // provide them (in contrast to the Microsoft C++ library, see + // https://stackoverflow.com/questions/821873/how-to-open-an-stdfstream-ofstream-or-ifstream-with-a-unicode-filename/822032#822032), + // Boost is forced to fall back to `char` constructors which may not work properly. + // + // Work around this issue by creating stream objects with `_wfopen` in + // combination with `__gnu_cxx::stdio_filebuf`. This workaround can be removed + // with an upgrade to C++17, where streams can be constructed directly from + // `std::filesystem::path` objects. + +#if defined WIN32 && defined __GLIBCXX__ + class ifstream : public std::istream + { + public: + ifstream() = default; + explicit ifstream(const fs::path& p, std::ios_base::openmode mode = std::ios_base::in) { open(p, mode); } + ~ifstream() { close(); } + void open(const fs::path& p, std::ios_base::openmode mode = std::ios_base::in); + bool is_open() { return m_filebuf.is_open(); } + void close(); + + private: + __gnu_cxx::stdio_filebuf m_filebuf; + FILE* m_file = nullptr; + }; + class ofstream : public std::ostream + { + public: + ofstream() = default; + explicit ofstream(const fs::path& p, std::ios_base::openmode mode = std::ios_base::out) { open(p, mode); } + ~ofstream() { close(); } + void open(const fs::path& p, std::ios_base::openmode mode = std::ios_base::out); + bool is_open() { return m_filebuf.is_open(); } + void close(); + + private: + __gnu_cxx::stdio_filebuf m_filebuf; + FILE* m_file = nullptr; + }; +#else // !(WIN32 && __GLIBCXX__) + typedef fs::ifstream ifstream; + typedef fs::ofstream ofstream; +#endif // WIN32 && __GLIBCXX__ }; #endif // BITCOIN_FS_H diff --git a/src/logging.h b/src/logging.h index f04dde9dabba..55394eb1a854 100644 --- a/src/logging.h +++ b/src/logging.h @@ -175,42 +175,31 @@ std::string SafeStringFormat(const std::string& fmt, const Args&... args) } } -/** Get format string from VA_ARGS for error reporting */ -template std::string FormatStringFromLogArgs(const char *fmt, const Args&... args) { return fmt; } - -static inline void MarkUsed() {} -template static inline void MarkUsed(const T& t, const Args&... args) -{ - (void)t; - MarkUsed(args...); -} - // Be conservative when using LogPrintf/error or other things which // unconditionally log to debug.log! It should not be the case that an inbound // peer can fill up a user's disk with debug.log entries. -#ifdef USE_COVERAGE -#define LogPrintf(...) do { MarkUsed(__VA_ARGS__); } while(0) -#define LogPrint(category, ...) do { MarkUsed(__VA_ARGS__); } while(0) -#else -#define LogPrintf(...) do { \ - if (g_logger->Enabled()) { \ - std::string _log_msg_; /* Unlikely name to avoid shadowing variables */ \ - try { \ - _log_msg_ = tfm::format(__VA_ARGS__); \ - } catch (tinyformat::format_error &e) { \ - /* Original format string will have newline so don't add one here */ \ - _log_msg_ = "Error \"" + std::string(e.what()) + "\" while formatting log message: " + FormatStringFromLogArgs(__VA_ARGS__); \ - } \ - g_logger->LogPrintStr(_log_msg_); \ - } \ -} while(0) - -#define LogPrint(category, ...) do { \ - if (LogAcceptCategory((category))) { \ - LogPrintf(__VA_ARGS__); \ - } \ -} while(0) -#endif // USE_COVERAGE +template +static inline void LogPrintf(const char* fmt, const Args&... args) +{ + if (g_logger->Enabled()) { + std::string log_msg; + try { + log_msg = tfm::format(fmt, args...); + } catch (tinyformat::format_error& fmterr) { + /* Original format string will have newline so don't add one here */ + log_msg = "Error \"" + std::string(fmterr.what()) + "\" while formatting log message: " + fmt; + } + g_logger->LogPrintStr(log_msg); + } +} + +template +static inline void LogPrint(const BCLog::LogFlags& category, const Args&... args) +{ + if (LogAcceptCategory((category))) { + LogPrintf(args...); + } +} #endif // BITCOIN_LOGGING_H diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 93cc353b8d5d..5ebb0ab9ea5e 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -875,7 +875,7 @@ fs::path static GetAutostartFilePath() bool GetStartOnSystemStartup() { - fs::ifstream optionFile(GetAutostartFilePath()); + fsbridge::ifstream optionFile(GetAutostartFilePath()); if (!optionFile.good()) return false; // Scan through file for "Hidden=true": @@ -906,7 +906,7 @@ bool SetStartOnSystemStartup(bool fAutoStart) fs::create_directories(GetAutostartDir()); - fs::ofstream optionFile(GetAutostartFilePath(), std::ios_base::out|std::ios_base::trunc); + fsbridge::ofstream optionFile(GetAutostartFilePath(), std::ios_base::out | std::ios_base::trunc); if (!optionFile.good()) return false; std::string chain = gArgs.GetChainName(); diff --git a/src/rpc/protocol.cpp b/src/rpc/protocol.cpp index 1c47cc6beafa..d57eea569e5e 100644 --- a/src/rpc/protocol.cpp +++ b/src/rpc/protocol.cpp @@ -13,8 +13,6 @@ #include #include -#include - /** * JSON-RPC protocol. Bitcoin speaks version 1.0 for maximum compatibility, * but uses JSON-RPC 1.1/2.0 standards for parts of the 1.0 standard that were @@ -86,9 +84,9 @@ bool GenerateAuthCookie(std::string *cookie_out) /** the umask determines what permissions are used to create this file - * these are set to 077 in init.cpp unless overridden with -sysperms. */ - std::ofstream file; + fsbridge::ofstream file; fs::path filepath_tmp = GetAuthCookieFile(true); - file.open(filepath_tmp.string().c_str()); + file.open(filepath_tmp); if (!file.is_open()) { LogPrintf("Unable to open cookie authentication file %s for writing\n", filepath_tmp.string()); return false; @@ -110,10 +108,10 @@ bool GenerateAuthCookie(std::string *cookie_out) bool GetAuthCookie(std::string *cookie_out) { - std::ifstream file; + fsbridge::ifstream file; std::string cookie; fs::path filepath = GetAuthCookieFile(); - file.open(filepath.string().c_str()); + file.open(filepath); if (!file.is_open()) return false; std::getline(file, cookie); @@ -129,7 +127,7 @@ void DeleteAuthCookie() try { fs::remove(GetAuthCookieFile()); } catch (const fs::filesystem_error& e) { - LogPrintf("%s: Unable to remove random auth cookie file: %s\n", __func__, e.what()); + LogPrintf("%s: Unable to remove random auth cookie file: %s\n", __func__, fsbridge::get_filesystem_error_message(e)); } } diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp new file mode 100644 index 000000000000..6e9aed48d564 --- /dev/null +++ b/src/test/fs_tests.cpp @@ -0,0 +1,56 @@ +// Copyright (c) 2011-2018 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. +// +#include +#include + +#include + +BOOST_FIXTURE_TEST_SUITE(fs_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(fsbridge_fstream) +{ + fs::path tmpfolder = SetDataDir("fsbridge_fstream"); + // tmpfile1 should be the same as tmpfile2 + fs::path tmpfile1 = tmpfolder / "fs_tests_₿_🏃"; + fs::path tmpfile2 = tmpfolder / "fs_tests_₿_🏃"; + { + fsbridge::ofstream file(tmpfile1); + file << "bitcoin"; + } + { + fsbridge::ifstream file(tmpfile2); + std::string input_buffer; + file >> input_buffer; + BOOST_CHECK_EQUAL(input_buffer, "bitcoin"); + } + { + fsbridge::ifstream file(tmpfile1, std::ios_base::in | std::ios_base::ate); + std::string input_buffer; + file >> input_buffer; + BOOST_CHECK_EQUAL(input_buffer, ""); + } + { + fsbridge::ofstream file(tmpfile2, std::ios_base::out | std::ios_base::app); + file << "tests"; + } + { + fsbridge::ifstream file(tmpfile1); + std::string input_buffer; + file >> input_buffer; + BOOST_CHECK_EQUAL(input_buffer, "bitcointests"); + } + { + fsbridge::ofstream file(tmpfile2, std::ios_base::out | std::ios_base::trunc); + file << "bitcoin"; + } + { + fsbridge::ifstream file(tmpfile1); + std::string input_buffer; + file >> input_buffer; + BOOST_CHECK_EQUAL(input_buffer, "bitcoin"); + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util.cpp b/src/util.cpp index 449ee8c8d96d..15bb8fdf2380 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -813,7 +813,7 @@ void ArgsManager::ReadConfigFiles() } const std::string confPath = GetArg("-conf", BITCOIN_CONF_FILENAME); - fs::ifstream stream(GetConfigFile(confPath)); + fsbridge::ifstream stream(GetConfigFile(confPath)); if (stream.good()) { ReadConfigStream(stream); @@ -829,7 +829,7 @@ void ArgsManager::ReadConfigFiles() } for (const std::string& to_include : includeconf) { - fs::ifstream include_config(GetConfigFile(to_include)); + fsbridge::ifstream include_config(GetConfigFile(to_include)); if (include_config.good()) { ReadConfigStream(include_config); LogPrintf("Included configuration file %s\n", to_include.c_str()); diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 7bcdd804dbdf..5e88b489eb37 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -826,7 +826,7 @@ bool BerkeleyDatabase::Backup(const std::string& strDest) LogPrintf("copied %s to %s\n", strFile, pathDest.string()); return true; } catch (const fs::filesystem_error& e) { - LogPrintf("error copying %s to %s - %s\n", strFile, pathDest.string(), e.what()); + LogPrintf("error copying %s to %s - %s\n", strFile, pathDest.string(), fsbridge::get_filesystem_error_message(e)); return false; } } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 8f42a0f9c61f..3ca94f5c6735 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -18,7 +18,6 @@ #include -#include #include #include @@ -519,8 +518,8 @@ UniValue importwallet(const JSONRPCRequest& request) EnsureWalletIsUnlocked(pwallet); - std::ifstream file; - file.open(request.params[0].get_str().c_str(), std::ios::in | std::ios::ate); + fsbridge::ifstream file; + file.open(request.params[0].get_str(), std::ios::in | std::ios::ate); if (!file.is_open()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); } @@ -646,7 +645,7 @@ UniValue importelectrumwallet(const JSONRPCRequest& request) EnsureWalletIsUnlocked(pwallet); - std::ifstream file; + fsbridge::ifstream file; std::string strFileName = request.params[0].get_str(); size_t nDotPos = strFileName.find_last_of("."); if(nDotPos == std::string::npos) @@ -656,7 +655,7 @@ UniValue importelectrumwallet(const JSONRPCRequest& request) if(strFileExt != "json" && strFileExt != "csv") throw JSONRPCError(RPC_INVALID_PARAMETER, "File has wrong extension, should be .json or .csv"); - file.open(strFileName.c_str(), std::ios::in | std::ios::ate); + file.open(strFileName, std::ios::in | std::ios::ate); if (!file.is_open()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open Electrum wallet export file"); @@ -887,8 +886,8 @@ UniValue dumpwallet(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, filepath.string() + " already exists. If you are sure this is what you want, move it out of the way first"); } - std::ofstream file; - file.open(filepath.string().c_str()); + fsbridge::ofstream file; + file.open(filepath); if (!file.is_open()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b5afe2901275..6e2f697b80d6 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4951,7 +4951,7 @@ bool CWallet::Verify(const WalletLocation& location, bool salvage_wallet, std::s return false; } } catch (const fs::filesystem_error& e) { - error_string = strprintf("Error loading wallet %s. %s", location.GetName(), e.what()); + error_string = strprintf("Error loading wallet %s. %s", location.GetName(), fsbridge::get_filesystem_error_message(e)); return false; } @@ -5399,7 +5399,7 @@ bool CWallet::AutoBackupWallet(const fs::path& wallet_path, std::string& strBack fs::copy_file(sourceFile, backupFile); LogPrintf("Creating backup of %s -> %s\n", sourceFile.string(), backupFile.string()); } catch(fs::filesystem_error &error) { - strBackupWarningRet = strprintf(_("Failed to create backup, error: %s"), error.what()); + strBackupWarningRet = strprintf(_("Failed to create backup, error: %s"), fsbridge::get_filesystem_error_message(error)); LogPrintf("%s\n", strBackupWarningRet); nWalletBackups = -1; return false; @@ -5438,7 +5438,7 @@ bool CWallet::AutoBackupWallet(const fs::path& wallet_path, std::string& strBack fs::remove(file.second); LogPrintf("Old backup deleted: %s\n", file.second); } catch(fs::filesystem_error &error) { - strBackupWarningRet = strprintf(_("Failed to delete backup, error: %s"), error.what()); + strBackupWarningRet = strprintf(_("Failed to delete backup, error: %s"), fsbridge::get_filesystem_error_message(error)); LogPrintf("%s\n", strBackupWarningRet); return false; } diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index cfdcea0f9897..6c6d01a22f9b 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -28,6 +28,7 @@ ("src/util.cpp", "strprintf(COPYRIGHT_HOLDERS, COPYRIGHT_HOLDERS_SUBSTITUTION)"), ("src/wallet/wallet.h", "WalletLogPrintf(std::string fmt, Params... parameters)"), ("src/wallet/wallet.h", "LogPrintf((\"%s \" + fmt).c_str(), GetDisplayName(), parameters...)"), + ("src/logging.h", "LogPrintf(const char* fmt, const Args&... args)"), ] def parse_function_calls(function_name, source_code):