Add file system exceptions

Change-Id: If0510554ef7136875feb8aa6f01fac9a1122e4db
Signed-off-by: Kyungwook Tak <k.tak@samsung.com>

diff --git a/src/framework/common/exception.h b/src/framework/common/exception.h
index 83e7d42..01e2202 100644 (file)
--- a/src/framework/common/exception.h
+++ b/src/framework/common/exception.h
@@ -77,11 +77,13 @@ public:
        }
 };
 
-using DbFailed      = DerivedException<CSR_ERROR_DB>;
-using InvalidParam  = DerivedException<CSR_ERROR_INVALID_PARAMETER>;
-using InternalError = DerivedException<CSR_ERROR_SERVER>;
-using EngineError   = DerivedException<CSR_ERROR_ENGINE_INTERNAL>;
-using SocketError   = DerivedException<CSR_ERROR_SOCKET>;
+using DbFailed        = DerivedException<CSR_ERROR_DB>;
+using InvalidParam    = DerivedException<CSR_ERROR_INVALID_PARAMETER>;
+using InternalError   = DerivedException<CSR_ERROR_SERVER>;
+using EngineError     = DerivedException<CSR_ERROR_ENGINE_INTERNAL>;
+using SocketError     = DerivedException<CSR_ERROR_SOCKET>;
+using FileDoNotExist  = DerivedException<CSR_ERROR_FILE_DO_NOT_EXIST>;
+using FileSystemError = DerivedException<CSR_ERROR_FILE_SYSTEM>;
 
 } // namespace Csr
 

diff --git a/src/framework/service/file-system.cpp b/src/framework/service/file-system.cpp
index edd2213..ee14137 100644 (file)
--- a/src/framework/service/file-system.cpp
+++ b/src/framework/service/file-system.cpp
@@ -145,11 +145,9 @@ FilePtr File::create(const std::string &fpath, time_t modifiedSince)
 {
        auto statptr = getStat(fpath);
        if (statptr == nullptr) {
-               // target doesn't exist
-               return nullptr;
+               ThrowExc(FileDoNotExist, "file not exist: " << fpath);
        } else if (!S_ISREG(statptr->st_mode)) {
-               INFO("File type is not regular: " << fpath);
-               return nullptr;
+               ThrowExc(FileSystemError, "file type is not reguler: " << fpath);
        } else if (modifiedSince != -1 && statptr->st_mtim.tv_sec < modifiedSince) {
                DEBUG("file[" << fpath << "] isn't modified since[" << modifiedSince << "]");
                return nullptr;
@@ -166,15 +164,12 @@ FsVisitor::DirPtr FsVisitor::openDir(const std::string &dir)
 FsVisitorPtr FsVisitor::create(const std::string &dirpath, time_t modifiedSince)
 {
        auto statptr = getStat(dirpath);
-       if (statptr == nullptr) {
-               // target doesn't exist
-               return nullptr;
-       } else if (!S_ISDIR(statptr->st_mode)) {
-               INFO("File type is not directory: " << dirpath);
-               return nullptr;
-       } else {
+       if (statptr == nullptr)
+               ThrowExc(FileDoNotExist, "directory not exist: " << dirpath);
+       else if (!S_ISDIR(statptr->st_mode))
+               ThrowExc(FileSystemError, "file type is not directory: " << dirpath);
+       else
                return FsVisitorPtr(new FsVisitor(dirpath, modifiedSince));
-       }
 }
 
 FsVisitor::FsVisitor(const std::string &dirpath, time_t modifiedSince) :
@@ -199,9 +194,8 @@ FilePtr FsVisitor::next()
        while (readdir_r(m_dirptr.get(), m_entryBuf, &result) == 0) {
                if (result == nullptr) { // end of dir stream
                        m_dirs.pop();
-                       while (!m_dirs.empty() && !(m_dirptr = openDir(m_dirs.front()))) {
+                       while (!m_dirs.empty() && !(m_dirptr = openDir(m_dirs.front())))
                                m_dirs.pop();
-                       }
 
                        if (m_dirs.empty())
                                return nullptr;
@@ -213,18 +207,22 @@ FilePtr FsVisitor::next()
                std::string filepath(result->d_name);
 
                if (result->d_type == DT_DIR) {
-                       if (filepath.compare(".") == 0 || filepath.compare("..") == 0)
-                               continue;
-                       else
+                       if (filepath.compare(".") != 0 && filepath.compare("..") != 0)
                                m_dirs.emplace(
                                        dir + ((filepath.back() == '/') ? filepath : (filepath + '/')));
                } else if (result->d_type == DT_REG) {
-                       auto fileptr = File::create(dir + filepath, m_since);
-
-                       if (!fileptr)
-                               continue;
-                       else
-                               return fileptr;
+                       try {
+                               auto fileptr = File::create(dir + filepath, m_since);
+
+                               if (fileptr)
+                                       return fileptr;
+                       } catch (const FileDoNotExist &e) {
+                               WARN("file not exist: " << dir << filepath <<
+                                        " msg: " << e.what());
+                       } catch (const FileSystemError &e) {
+                               WARN("file type is not regular...? can it be happened?"
+                                        " :" << dir << filepath << " msg: " << e.what());
+                       }
                }
        }
 

diff --git a/src/framework/service/file-system.h b/src/framework/service/file-system.h
index 7c46093..bd28e18 100644 (file)
--- a/src/framework/service/file-system.h
+++ b/src/framework/service/file-system.h
@@ -45,6 +45,7 @@ public:
 
        bool remove();
 
+       // throws FileNotExist and FileSystemError
        static FilePtr create(const std::string &fpath, time_t modifiedSince = -1);
 
 private:
@@ -72,6 +73,7 @@ public:
 
        FilePtr next();
 
+       // throws FileNotExist and FileSystemError
        static FsVisitorPtr create(const std::string &dirpath, time_t modifiedSince = -1);
 
 private:

diff --git a/src/framework/service/logic.cpp b/src/framework/service/logic.cpp
index bd66427..fe025af 100644 (file)
--- a/src/framework/service/logic.cpp
+++ b/src/framework/service/logic.cpp
@@ -179,11 +179,18 @@ RawBuffer Logic::scanFile(const CsContext &context, const std::string &filepath)
                return scanFileHelper(context, filepath);
 
        DEBUG("Scan history exist on file: " << filepath);
-       if (File::create(filepath, static_cast<time_t>(history->ts))) {
-               DEBUG("file[" << filepath << "] is modified since the detected time. "
-                         "let's remove history and re-scan");
-               m_db->deleteDetectedMalware(filepath);
-               return scanFileHelper(context, filepath);
+
+       try {
+               if (File::create(filepath, static_cast<time_t>(history->ts))) {
+                       DEBUG("file[" << filepath << "] is modified since the detected time. "
+                                 "let's remove history and re-scan");
+                       m_db->deleteDetectedMalware(filepath);
+                       return scanFileHelper(context, filepath);
+               }
+       } catch (const FileSystemError &e) {
+               ERROR("file doesn't exist or type isn't regular: " << filepath <<
+                         " " << e.what());
+               return BinaryQueue::Serialize(CSR_ERROR_INVALID_PARAMETER, CsDetected()).pop();
        }
 
        DEBUG("file[" << filepath << "] isn't modified since the detected time. "
@@ -202,16 +209,22 @@ RawBuffer Logic::scanFile(const CsContext &context, const std::string &filepath)
                m_db->setDetectedMalwareIgnored(filepath, true);
                break;
 
-       case CSR_CS_REMOVE: {
-               auto file = File::create(filepath);
-               if (!file && !file->remove()) {
-                       ERROR("Failed to remove filepath: " << filepath);
-                       return BinaryQueue::Serialize(CSR_ERROR_REMOVE_FAILED, CsDetected()).pop();
+       case CSR_CS_REMOVE:
+               try {
+                       auto file = File::create(filepath);
+                       if (!file && !file->remove()) {
+                               ERROR("Failed to remove filepath: " << filepath);
+                               return BinaryQueue::Serialize(CSR_ERROR_REMOVE_FAILED, CsDetected()).pop();
+                       }
+               } catch (const FileDoNotExist &) {
+                       WARN("File already removed... : " << filepath);
+               } catch (const FileSystemError &) {
+                       WARN("File type is changed... it's considered as different file "
+                                "in same path: " << filepath);
                }
 
                m_db->deleteDetectedMalware(filepath);
                break;
-       }
 
        case CSR_CS_SKIP:
                break;
@@ -229,12 +242,26 @@ RawBuffer Logic::scanFile(const CsContext &context, const std::string &filepath)
 RawBuffer Logic::getScannableFiles(const std::string &dir)
 {
        auto lastScanTime = m_db->getLastScanTime(dir, m_csDataVersion);
-       auto visitor = FsVisitor::create(dir, lastScanTime);
 
        StrSet fileset;
-       while (auto file = visitor->next()) {
-               DEBUG("In dir[" << dir << "], Scannable file[" << file->getPath() << "]");
-               fileset.insert(file->getPath());
+       try {
+               auto visitor = FsVisitor::create(dir, lastScanTime);
+
+               if (visitor == nullptr)
+                       return BinaryQueue::Serialize(CSR_ERROR_INVALID_PARAMETER, StrSet()).pop();
+
+               while (auto file = visitor->next()) {
+                       DEBUG("In dir[" << dir << "], Scannable file[" << file->getPath() << "]");
+                       fileset.insert(file->getPath());
+               }
+       } catch (const FileDoNotExist &) {
+               WARN("Directory isn't exist: " << dir << " return success with empty file set "
+                        "to skip it softly.");
+               return BinaryQueue::Serialize(CSR_ERROR_NONE, StrSet()).pop();
+       } catch (const FileSystemError &) {
+               WARN("Directory isn't directory... file type changed: " << dir << " return "
+                        "empty file set to skip it softly.");
+               return BinaryQueue::Serialize(CSR_ERROR_NONE, StrSet()).pop();
        }
 
        if (lastScanTime != -1) {
@@ -255,24 +282,36 @@ RawBuffer Logic::judgeStatus(const std::string &filepath, csr_cs_action_e action
                return BinaryQueue::Serialize(CSR_ERROR_INVALID_PARAMETER).pop();
        }
 
-       if (File::create(filepath, static_cast<time_t>(history->ts))) {
-               ERROR("Target modified since db delta inserted. name: " << filepath);
-               m_db->deleteDetectedMalware(filepath);
-               // TODO: is it okay to just refresh db and return success?
-               return BinaryQueue::Serialize(CSR_ERROR_NONE).pop();
+       try {
+               if (File::create(filepath, static_cast<time_t>(history->ts))) {
+                       ERROR("Target modified since db delta inserted. name: " << filepath);
+                       m_db->deleteDetectedMalware(filepath);
+                       // TODO: is it okay to just refresh db and return success?
+                       return BinaryQueue::Serialize(CSR_ERROR_NONE).pop();
+               }
+       } catch (const FileSystemError &e) {
+               ERROR("file doesn't exist or type isn't regular: " << filepath <<
+                         " " << e.what());
+               return BinaryQueue::Serialize(CSR_ERROR_INVALID_PARAMETER).pop();
        }
 
        switch (action) {
-       case CSR_CS_ACTION_REMOVE: {
-               auto file = File::create(filepath);
-               if (!file && !file->remove()) {
-                       ERROR("Failed to remove filepath: " << filepath);
-                       return BinaryQueue::Serialize(CSR_ERROR_REMOVE_FAILED).pop();
+       case CSR_CS_ACTION_REMOVE:
+               try {
+                       auto file = File::create(filepath);
+                       if (!file && !file->remove()) {
+                               ERROR("Failed to remove filepath: " << filepath);
+                               return BinaryQueue::Serialize(CSR_ERROR_REMOVE_FAILED).pop();
+                       }
+               } catch (const FileDoNotExist &) {
+                       WARN("File already removed... : " << filepath);
+               } catch (const FileSystemError &) {
+                       WARN("File type is changed... it's considered as different file "
+                                "in same path: " << filepath);
                }
 
                m_db->deleteDetectedMalware(filepath);
                break;
-       }
 
        case CSR_CS_ACTION_IGNORE:
                m_db->setDetectedMalwareIgnored(filepath, true);

diff --git a/src/include/csr/error.h b/src/include/csr/error.h
index 15e90a2..58cd9fc 100644 (file)
--- a/src/include/csr/error.h
+++ b/src/include/csr/error.h
@@ -48,6 +48,8 @@ typedef enum {
        CSR_ERROR_NO_TASK               = TIZEN_ERROR_CSR | 0x04,        /**< No Task exists*/
        CSR_ERROR_DB                    = TIZEN_ERROR_CSR | 0x05,        /**< DB transaction error */
        CSR_ERROR_REMOVE_FAILED         = TIZEN_ERROR_CSR | 0x06,        /**< Removing file or application is failed */
+       CSR_ERROR_FILE_DO_NOT_EXIST     = TIZEN_ERROR_CSR | 0x07,        /**< File not exist */
+       CSR_ERROR_FILE_SYSTEM           = TIZEN_ERROR_CSR | 0x08,        /**< File type is invalid */
        CSR_ERROR_ENGINE_PERMISSION     = TIZEN_ERROR_CSR | 0x11,        /**< Insufficient permission of engine */
        CSR_ERROR_ENGINE_NOT_EXIST      = TIZEN_ERROR_CSR | 0x12,        /**< No engine exists*/
        CSR_ERROR_ENGINE_DIASABLED      = TIZEN_ERROR_CSR | 0x13,        /**< Engine is in disabled state*/

diff --git a/test/internals/CMakeLists.txt b/test/internals/CMakeLists.txt
index 3bafdc9..6987496 100644 (file)
--- a/test/internals/CMakeLists.txt
+++ b/test/internals/CMakeLists.txt
@@ -34,6 +34,8 @@ SET(${TARGET_CSR_INTERNAL_TEST}_SRCS
        ${CSR_TEST_SRC_PATH}/colour_log_formatter.cpp
        ${CSR_TEST_SRC_PATH}/test-common.cpp
 
+       ${CSR_FW_SRC_PATH}/common/exception.cpp
+
        ${CSR_FW_SRC_PATH}/db/connection.cpp
        ${CSR_FW_SRC_PATH}/db/statement.cpp
        ${CSR_FW_SRC_PATH}/db/manager.cpp

diff --git a/test/internals/test-file-system.cpp b/test/internals/test-file-system.cpp
index 379dd7e..518c653 100644 (file)
--- a/test/internals/test-file-system.cpp
+++ b/test/internals/test-file-system.cpp
@@ -20,6 +20,7 @@
  * @brief       filesystem class unit test
  */
 #include <service/file-system.h>
+#include <common/exception.h>
 
 #include <string>
 #include <iostream>
@@ -227,7 +228,7 @@ BOOST_AUTO_TEST_CASE(file_visitor_positive_modified)
 
 BOOST_AUTO_TEST_CASE(file_visitor_negative_non_existing)
 {
-       CHECK_IS_NULL(File::create(TEST_DIR "/non_existing_file"));
+       BOOST_REQUIRE_THROW(File::create(TEST_DIR "/non_existing_file"), FileDoNotExist);
 }
 
 BOOST_AUTO_TEST_CASE(directory_visitor_positive_existing)