From a58974d77346d65e2ec48f83b0db53ea44cfeea5 Mon Sep 17 00:00:00 2001 From: Willi Goesgens Date: Mon, 23 Feb 2015 12:26:49 +0100 Subject: [PATCH] Improve error handling - directory creation - zip extraction --- UnitTests/Basics/files-test.cpp | 4 +- arangod/VocBase/collection.cpp | 10 +++-- arangod/VocBase/server.cpp | 40 +++++++++++------ arangosh/V8Client/arangodump.cpp | 6 ++- lib/Basics/files.cpp | 54 +++++++++------------- lib/Basics/files.h | 8 ++-- lib/Basics/tri-zip.cpp | 77 +++++++++++++++++++++++++------- lib/Basics/tri-zip.h | 3 +- lib/V8/v8-utils.cpp | 10 ++--- 9 files changed, 132 insertions(+), 80 deletions(-) diff --git a/UnitTests/Basics/files-test.cpp b/UnitTests/Basics/files-test.cpp index b34589eb9f..d2c0b480dc 100644 --- a/UnitTests/Basics/files-test.cpp +++ b/UnitTests/Basics/files-test.cpp @@ -40,13 +40,15 @@ using namespace triagens::basics; struct CFilesSetup { CFilesSetup () : _directory(TRI_UNKNOWN_MEM_ZONE) { + long systemError; + std::string errorMessage; BOOST_TEST_MESSAGE("setup files"); _directory.appendText("/tmp/arangotest-"); _directory.appendInteger((uint64_t) TRI_microtime()); _directory.appendInteger((uint32_t) TRI_UInt32Random()); - TRI_CreateDirectory(_directory.c_str()); + TRI_CreateDirectory(_directory.c_str(), systemError, errorMessage); } ~CFilesSetup () { diff --git a/arangod/VocBase/collection.cpp b/arangod/VocBase/collection.cpp index 7721bdf13c..309ee732cd 100644 --- a/arangod/VocBase/collection.cpp +++ b/arangod/VocBase/collection.cpp @@ -1053,13 +1053,17 @@ TRI_collection_t* TRI_CreateCollection (TRI_vocbase_t* vocbase, } // create directory - int res = TRI_CreateDirectory(filename); + std::string errorMessage; + long systemError; + int res = TRI_CreateDirectory(filename, systemError, errorMessage); if (res != TRI_ERROR_NO_ERROR) { - LOG_ERROR("cannot create collection '%s' in directory '%s': %s", + LOG_ERROR("cannot create collection '%s' in directory '%s': %s - %ld - %s", parameters->_name, path, - TRI_errno_string(res)); + TRI_errno_string(res), + systemError, + errorMessage.c_str()); TRI_FreeString(TRI_CORE_MEM_ZONE, filename); diff --git a/arangod/VocBase/server.cpp b/arangod/VocBase/server.cpp index 862a90f65d..189ed1e3e4 100644 --- a/arangod/VocBase/server.cpp +++ b/arangod/VocBase/server.cpp @@ -440,16 +440,17 @@ static int CreateBaseApplicationDirectory (char const* basePath, if (path != nullptr) { if (! TRI_IsDirectory(path)) { - res = TRI_CreateDirectory(path); + std::string errorMessage; + long systemError; + res = TRI_CreateDirectory(path, systemError, errorMessage); if (res == TRI_ERROR_NO_ERROR) { LOG_INFO("created base application directory '%s'", path); } else { - LOG_ERROR("unable to create base application directory '%s': %s", - path, - TRI_errno_string(res)); + LOG_ERROR("unable to create base application directory %s", + errorMessage.c_str()); } } @@ -474,7 +475,9 @@ static int CreateApplicationDirectory (char const* name, if (path != nullptr) { if (! TRI_IsDirectory(path)) { - res = TRI_CreateDirectory(path); + long systemError; + std::string errorMessage; + res = TRI_CreateDirectory(path, systemError, errorMessage); if (res == TRI_ERROR_NO_ERROR) { if (triagens::wal::LogfileManager::instance()->isInRecovery()) { @@ -492,13 +495,13 @@ static int CreateApplicationDirectory (char const* name, LOG_INFO("unable to create application directory '%s' for database '%s': %s", path, name, - TRI_errno_string(res)); + errorMessage.c_str()); } else { LOG_ERROR("unable to create application directory '%s' for database '%s': %s", path, name, - TRI_errno_string(res)); + errorMessage.c_str()); } } @@ -1178,10 +1181,15 @@ static int CreateDatabaseDirectory (TRI_server_t* server, return TRI_ERROR_OUT_OF_MEMORY; } + std::string errorMessage; + long systemError; + + res = TRI_CreateDirectory(file, systemError, errorMessage); - res = TRI_CreateDirectory(file); - - if (res != TRI_ERROR_NO_ERROR) { + if (res != TRI_ERROR_NO_ERROR){ + if (res != TRI_ERROR_FILE_EXISTS) { + LOG_ERROR("failed to create database directory: %s", errorMessage.c_str()); + } TRI_FreeString(TRI_CORE_MEM_ZONE, file); TRI_FreeString(TRI_CORE_MEM_ZONE, dname); @@ -1800,12 +1808,14 @@ int TRI_StartServer (TRI_server_t* server, // ............................................................................. if (! TRI_IsDirectory(server->_databasePath)) { - res = TRI_CreateDirectory(server->_databasePath); + long systemError; + std::string errorMessage; + res = TRI_CreateDirectory(server->_databasePath, systemError, errorMessage); if (res != TRI_ERROR_NO_ERROR) { LOG_ERROR("unable to create database directory '%s': %s", server->_databasePath, - TRI_errno_string(res)); + errorMessage.c_str()); return TRI_ERROR_ARANGO_DATADIR_NOT_WRITABLE; } @@ -1851,12 +1861,14 @@ int TRI_StartServer (TRI_server_t* server, return TRI_ERROR_BAD_PARAMETER; } - res = TRI_CreateDirectory(server->_appPath); + long systemError; + std::string errorMessage; + res = TRI_CreateDirectory(server->_appPath, systemError, errorMessage); if (res != TRI_ERROR_NO_ERROR) { LOG_ERROR("unable to create --javascript.app-path directory '%s': %s", server->_appPath, - TRI_errno_string(res)); + errorMessage.c_str()); return res; } } diff --git a/arangosh/V8Client/arangodump.cpp b/arangosh/V8Client/arangodump.cpp index b5ae8fc831..22f6458f91 100644 --- a/arangosh/V8Client/arangodump.cpp +++ b/arangosh/V8Client/arangodump.cpp @@ -1385,10 +1385,12 @@ int main (int argc, char* argv[]) { } if (! isDirectory) { - int res = TRI_CreateDirectory(OutputDirectory.c_str()); + long systemError; + std::string errorMessage; + int res = TRI_CreateDirectory(OutputDirectory.c_str(),systemError, errorMessage); if (res != TRI_ERROR_NO_ERROR) { - cerr << "unable to create output directory '" << OutputDirectory << "': " << string(TRI_errno_string(res)) << endl; + cerr << "unable to create output directory '" << OutputDirectory << "': " << errorMessage << endl; TRI_EXIT_FUNCTION(EXIT_FAILURE, nullptr); } } diff --git a/lib/Basics/files.cpp b/lib/Basics/files.cpp index a7f7a4d5b3..ab361435f0 100644 --- a/lib/Basics/files.cpp +++ b/lib/Basics/files.cpp @@ -513,14 +513,13 @@ int TRI_MTimeFile (char const* path, int64_t* mtime) { //////////////////////////////////////////////////////////////////////////////// int TRI_CreateRecursiveDirectory (char const* path, - long *systemError, - std::string *systemErrorStr) { + long &systemError, + std::string &systemErrorStr) { TRI_ERRORBUF; char* copy; char* p; char* s; int res; - int m; res = TRI_ERROR_NO_ERROR; p = s = copy = TRI_DuplicateString(path); @@ -530,19 +529,13 @@ int TRI_CreateRecursiveDirectory (char const* path, if (p - s > 0) { *p = '\0'; - m = TRI_MKDIR(copy, 0777); - TRI_SYSTEM_ERROR(); - *p = TRI_DIR_SEPARATOR_CHAR; - s = p + 1; - - if (m != 0 && errno != EEXIST) { - res = errno; - if (systemErrorStr != nullptr) { - *systemErrorStr = std::string("Failed to create directory [") + path + "] " + TRI_GET_ERRORBUF; - } - if (systemError != nullptr) { - *systemError = errno; - } + res = TRI_CreateDirectory(copy, systemError, systemErrorStr); + if ((res == TRI_ERROR_FILE_EXISTS) || + (res == TRI_ERROR_NO_ERROR)) { + *p = TRI_DIR_SEPARATOR_CHAR; + s = p + 1; + } + else { break; } } @@ -550,12 +543,10 @@ int TRI_CreateRecursiveDirectory (char const* path, p++; } - if (res == TRI_ERROR_NO_ERROR && (p - s > 0)) { - m = TRI_MKDIR(copy, 0777); - - if (m != 0 && errno != EEXIST) { - res = errno; - } + if (((res == TRI_ERROR_FILE_EXISTS) || + (res == TRI_ERROR_NO_ERROR)) + && (p - s > 0)) { + res = TRI_CreateDirectory(copy, systemError, systemErrorStr); } TRI_Free(TRI_CORE_MEM_ZONE, copy); @@ -566,10 +557,9 @@ int TRI_CreateRecursiveDirectory (char const* path, //////////////////////////////////////////////////////////////////////////////// /// @brief creates a directory //////////////////////////////////////////////////////////////////////////////// - int TRI_CreateDirectory (char const* path, - long *systemError, - std::string *systemErrorStr) { + long &systemError, + std::string &systemErrorStr) { TRI_ERRORBUF; int res; @@ -583,9 +573,7 @@ int TRI_CreateDirectory (char const* path, TRI_SYSTEM_ERROR(); res = errno; if (res != TRI_ERROR_NO_ERROR) { - if (systemErrorStr != nullptr) { - *systemErrorStr = std::string("Failed to create directory [") + path + "] " + TRI_GET_ERRORBUF; - } + systemErrorStr = std::string("Failed to create directory [") + path + "] " + TRI_GET_ERRORBUF; #ifndef _WIN32 if (res == ENOENT) { res = TRI_ERROR_FILE_NOT_FOUND; @@ -607,13 +595,11 @@ int TRI_CreateDirectory (char const* path, res = TRI_ERROR_FORBIDDEN; } #endif - if (systemError != nullptr) { - *systemError = errno; - } else { - // an unknown error type. will be translated into system error below res = TRI_ERROR_NO_ERROR; } + systemError = errno; + // an unknown error type. will be translated into system error below } // if errno doesn't indicate an error, return a system error @@ -2034,6 +2020,8 @@ int TRI_GetTempName (char const* directory, char* dir; char* temp; int tries; + int long systemError = 0; + std::string errorMessage; temp = TRI_GetUserTempPath(); @@ -2049,7 +2037,7 @@ int TRI_GetTempName (char const* directory, // remove trailing PATH_SEPARATOR RemoveTrailingSeparator(dir); - TRI_CreateRecursiveDirectory(dir); + TRI_CreateRecursiveDirectory(dir, systemError, errorMessage); /// TODO: errormessage! if (! TRI_IsDirectory(dir)) { TRI_Free(TRI_CORE_MEM_ZONE, dir); diff --git a/lib/Basics/files.h b/lib/Basics/files.h index 6d4a23591a..cc92b826f6 100644 --- a/lib/Basics/files.h +++ b/lib/Basics/files.h @@ -97,16 +97,16 @@ int TRI_MTimeFile (char const* path, int64_t* mtime); //////////////////////////////////////////////////////////////////////////////// int TRI_CreateRecursiveDirectory (char const* path, - long *systemError = nullptr, - std::string *systemErrorStr = nullptr); + long &systemError, + std::string &systemErrorStr); //////////////////////////////////////////////////////////////////////////////// /// @brief creates a directory //////////////////////////////////////////////////////////////////////////////// int TRI_CreateDirectory (char const* path, - long *systemError = nullptr, - std::string *systemErrorStr = nullptr); + long &systemError, + std::string &systemErrorStr); //////////////////////////////////////////////////////////////////////////////// /// @brief removes an empty directory diff --git a/lib/Basics/tri-zip.cpp b/lib/Basics/tri-zip.cpp index a8c04fe9e1..a3833c2fa4 100644 --- a/lib/Basics/tri-zip.cpp +++ b/lib/Basics/tri-zip.cpp @@ -53,16 +53,21 @@ static int ExtractCurrentFile (unzFile uf, const char* outPath, const bool skipPaths, const bool overwrite, - const char* password) { + const char* password, + std::string &errorMessage) { char filenameInZip[256]; char* filenameWithoutPath; char* fullPath; char* p; FILE *fout; unz_file_info64 fileInfo; + long systemError; + int err; filenameInZip[0] = '\0'; - if (unzGetCurrentFileInfo64(uf, &fileInfo, filenameInZip, sizeof(filenameInZip), NULL, 0, NULL, 0) != UNZ_OK) { + err = unzGetCurrentFileInfo64(uf, &fileInfo, filenameInZip, sizeof(filenameInZip), NULL, 0, NULL, 0); + if(err != UNZ_OK) { + errorMessage = std::string("Failed to get file info for ") + filenameInZip + ": " + std::to_string(err); return TRI_ERROR_INTERNAL; } @@ -96,7 +101,12 @@ static int ExtractCurrentFile (unzFile uf, if (*filenameWithoutPath == '\0') { if (! skipPaths) { fullPath = TRI_Concatenate2File(outPath, filenameInZip); - TRI_CreateRecursiveDirectory(fullPath); + err = TRI_CreateRecursiveDirectory(fullPath, systemError, errorMessage); + if ((err != TRI_ERROR_NO_ERROR) && + (err != TRI_ERROR_FILE_EXISTS) ) { + TRI_Free(TRI_CORE_MEM_ZONE, fullPath); + return err; + } TRI_Free(TRI_CORE_MEM_ZONE, fullPath); } } @@ -112,7 +122,9 @@ static int ExtractCurrentFile (unzFile uf, writeFilename = filenameWithoutPath; } - if (unzOpenCurrentFilePassword(uf, password) != UNZ_OK) { + err = unzOpenCurrentFilePassword(uf, password); + if (err != UNZ_OK) { + errorMessage = "failed to authenticate the password in the zip: " + std::to_string(err); return TRI_ERROR_INTERNAL; } @@ -121,6 +133,7 @@ static int ExtractCurrentFile (unzFile uf, if (! overwrite && TRI_ExistsFile(fullPath)) { TRI_Free(TRI_CORE_MEM_ZONE, fullPath); + errorMessage = std::string("not allowed to overwrite file ") + fullPath; return TRI_ERROR_CANNOT_OVERWRITE_FILE; } @@ -128,13 +141,18 @@ static int ExtractCurrentFile (unzFile uf, fout = fopen(fullPath, "wb"); // cannot write to outfile. this may be due to the target directory missing - if (fout == NULL && ! skipPaths && filenameWithoutPath != (char*) filenameInZip) { + if (fout == nullptr && ! skipPaths && filenameWithoutPath != (char*) filenameInZip) { char c = *(filenameWithoutPath - 1); *(filenameWithoutPath - 1) = '\0'; // create target directory recursively char* d = TRI_Concatenate2File(outPath, filenameInZip); - TRI_CreateRecursiveDirectory(d); + err = TRI_CreateRecursiveDirectory(d, systemError, errorMessage); + if ((err != TRI_ERROR_NO_ERROR) && + (err != TRI_ERROR_FILE_EXISTS) ) { + TRI_Free(TRI_CORE_MEM_ZONE, d); + return err; + } TRI_Free(TRI_CORE_MEM_ZONE, d); *(filenameWithoutPath - 1) = c; @@ -148,15 +166,22 @@ static int ExtractCurrentFile (unzFile uf, // strip filename so we only have the directory name char* dir = TRI_Dirname(d); TRI_Free(TRI_CORE_MEM_ZONE, d); - TRI_CreateRecursiveDirectory(dir); + err = TRI_CreateRecursiveDirectory(dir, systemError, errorMessage); + if ((err != TRI_ERROR_NO_ERROR) && + (err != TRI_ERROR_FILE_EXISTS) ) { + TRI_Free(TRI_CORE_MEM_ZONE, d); + TRI_Free(TRI_CORE_MEM_ZONE, dir); + return err; + } + TRI_Free(TRI_CORE_MEM_ZONE, d); TRI_Free(TRI_CORE_MEM_ZONE, dir); // try again fout = fopen(fullPath, "wb"); } - TRI_Free(TRI_CORE_MEM_ZONE, fullPath); - if (fout == NULL) { + errorMessage = std::string("failed to open file for writing: ") + fullPath + " - " + strerror(errno); + TRI_Free(TRI_CORE_MEM_ZONE, fullPath); return TRI_ERROR_CANNOT_WRITE_FILE; } @@ -164,15 +189,18 @@ static int ExtractCurrentFile (unzFile uf, int result = unzReadCurrentFile(uf, buffer, (unsigned int) bufferSize); if (result < 0) { + errorMessage = std::string("failed to write file ") + fullPath + " - " + strerror(errno); fclose(fout); - + TRI_Free(TRI_CORE_MEM_ZONE, fullPath); return TRI_ERROR_CANNOT_WRITE_FILE; } if (result > 0) { if (fwrite(buffer, result, 1, fout) != 1) { + + errorMessage = std::string("failed to write file ") + fullPath + " - " + strerror(errno); + TRI_Free(TRI_CORE_MEM_ZONE, fullPath); fclose(fout); - return TRI_set_errno(TRI_ERROR_SYS_ERROR); } } @@ -181,6 +209,7 @@ static int ExtractCurrentFile (unzFile uf, break; } } + TRI_Free(TRI_CORE_MEM_ZONE, fullPath); fclose(fout); } @@ -200,24 +229,33 @@ static int UnzipFile (unzFile uf, const char* outPath, const bool skipPaths, const bool overwrite, - const char* password) { + const char* password, + std::string &errorMessage) { unz_global_info64 gi; uLong i; int res = TRI_ERROR_NO_ERROR; + int err; - if (unzGetGlobalInfo64(uf, &gi) != UNZ_OK) { + err = unzGetGlobalInfo64(uf, &gi); + if(err!= UNZ_OK) { + errorMessage = "Failed to get info: " + std::to_string(err); return TRI_ERROR_INTERNAL; } for (i = 0; i < gi.number_entry; i++) { - res = ExtractCurrentFile(uf, buffer, bufferSize, outPath, skipPaths, overwrite, password); + res = ExtractCurrentFile(uf, buffer, bufferSize, outPath, skipPaths, overwrite, password, errorMessage); if (res != TRI_ERROR_NO_ERROR) { break; } if ((i + 1) < gi.number_entry) { - if (unzGoToNextFile(uf) != UNZ_OK) { + err = unzGoToNextFile(uf); + if (err == UNZ_END_OF_LIST_OF_FILE) { + break; + } + else if (err != UNZ_OK) { + errorMessage = "Failed to jump to next file: " + std::to_string(err); res = TRI_ERROR_INTERNAL; break; } @@ -386,7 +424,8 @@ int TRI_UnzipFile (const char* filename, const char* outPath, const bool skipPaths, const bool overwrite, - const char* password) { + const char* password, + std::string &errorMessage) { unzFile uf; #ifdef USEWIN32IOAPI zlib_filefunc64_def ffunc; @@ -409,8 +448,12 @@ int TRI_UnzipFile (const char* filename, #else uf = unzOpen64(filename); #endif + if (uf == nullptr) { + errorMessage = std::string("unable to open zip file ") + filename; + return TRI_ERROR_INTERNAL; + } - res = UnzipFile(uf, buffer, bufferSize, outPath, skipPaths, overwrite, password); + res = UnzipFile(uf, buffer, bufferSize, outPath, skipPaths, overwrite, password, errorMessage); unzClose(uf); diff --git a/lib/Basics/tri-zip.h b/lib/Basics/tri-zip.h index f59401034c..89bed802a1 100644 --- a/lib/Basics/tri-zip.h +++ b/lib/Basics/tri-zip.h @@ -58,7 +58,8 @@ int TRI_UnzipFile (const char*, const char*, const bool, const bool, - const char*); + const char*, + std::string &errorMessage); #endif diff --git a/lib/V8/v8-utils.cpp b/lib/V8/v8-utils.cpp index f42123bf02..a7b7028673 100644 --- a/lib/V8/v8-utils.cpp +++ b/lib/V8/v8-utils.cpp @@ -1400,9 +1400,9 @@ static void JS_MakeDirectory (const v8::FunctionCallbackInfo& args) { if (*name == nullptr) { TRI_V8_THROW_TYPE_ERROR(" must be a string"); } - + long systemError = 0; std::string systemErrorStr; - int res = TRI_CreateDirectory(*name, nullptr, &systemErrorStr); + int res = TRI_CreateDirectory(*name, systemError, systemErrorStr); if (res != TRI_ERROR_NO_ERROR) { TRI_V8_THROW_EXCEPTION_MESSAGE(res, systemErrorStr); @@ -1452,14 +1452,14 @@ static void JS_UnzipFile (const v8::FunctionCallbackInfo& args) { password = TRI_ObjectToString(args[4]); p = password.c_str(); } - - int res = TRI_UnzipFile(filename.c_str(), outPath.c_str(), skipPaths, overwrite, p); + std::string errMsg; + int res = TRI_UnzipFile(filename.c_str(), outPath.c_str(), skipPaths, overwrite, p, errMsg); if (res == TRI_ERROR_NO_ERROR) { TRI_V8_RETURN_TRUE(); } - TRI_V8_THROW_EXCEPTION(res); + TRI_V8_THROW_EXCEPTION_MESSAGE(res, errMsg); } ////////////////////////////////////////////////////////////////////////////////