From: Piotr Kosko Date: Tue, 6 Oct 2015 08:49:58 +0000 (+0200) Subject: [Archive] Resolved TODOs X-Git-Tag: submit/tizen/20151026.073646^2^2~47^2~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=16d6bc436d003eafc099e116afe359e304ab4628;p=platform%2Fcore%2Fapi%2Fwebapi-plugins.git [Archive] Resolved TODOs [Feature] Resolved: - permissions of zipped files - in-zip paths problem [Verification] Code compiles without errors. In-zip paths are correct. Correct file permissions are passed while zipping and unzipping. TCT passrate is 100%. Change-Id: Iabd9372b753be740e0011b29f40cc2c86384ddf9 Signed-off-by: Piotr Kosko --- diff --git a/src/archive/archive_callback_data.cc b/src/archive/archive_callback_data.cc index dcb210ff..2efbaefb 100755 --- a/src/archive/archive_callback_data.cc +++ b/src/archive/archive_callback_data.cc @@ -535,7 +535,7 @@ void AddProgressCallback::setBasePath(const std::string& path) { LoggerD("Entered"); m_base_path = path; - m_base_virt_path = filesystem::External::toVirtualPath(m_base_path); + m_base_virt_path = filesystem::External::cutVirtualRoot(m_base_path); std::string::size_type pos = m_base_virt_path.find(filesystem::Path::getSeparator()); if (pos != std::string::npos) { diff --git a/src/archive/archive_utils.cc b/src/archive/archive_utils.cc index 1ce2c703..b0414d55 100755 --- a/src/archive/archive_utils.cc +++ b/src/archive/archive_utils.cc @@ -19,9 +19,6 @@ #include #include "common/logger.h" -//TODO: -//#include - using namespace common; namespace extension { @@ -96,34 +93,6 @@ PlatformResult stringToFileMode(std::string fmString, FileMode* fm) return PlatformResult(ErrorCode::TYPE_MISMATCH_ERR, "Invalid FileMode"); } -// FilePtr fileReferenceToFile(JSContextRef context, JSValueRef fileReference) -// { -// auto g_ctx = GlobalContextManager::getInstance()->getGlobalContext(context); -// -// FilePtr file_ptr; -// try { -// file_ptr = JSFile::getPrivateObject(context, fileReference); -// } catch (const TypeMismatchException &tme) { -// LOGD("Use virtual path."); -// std::string virtual_path = -// JSUtil::JSValueToString(context, fileReference); -// if (!External::isVirtualPath(virtual_path)) { -// LOGE("FileReference can be File object or a virtual path"); -// throw TypeMismatchException( -// "FileReference can be File object or a virtual path"); -// } -// std::string string_path = -// External::fromVirtualPath(virtual_path, g_ctx); -// LOGD("Path: %s", string_path.c_str()); -// -// PathPtr path = Path::create(string_path); -// NodePtr node_ptr = Node::resolve(path); -// file_ptr = FilePtr(new File(node_ptr, File::PermissionList())); -// } -// -// return file_ptr; -// } - void getBasePathAndName(const std::string& filepath, std::string& out_basepath, std::string& out_name) diff --git a/src/archive/archive_utils.h b/src/archive/archive_utils.h index 541822e1..1b9dcb3d 100755 --- a/src/archive/archive_utils.h +++ b/src/archive/archive_utils.h @@ -30,9 +30,6 @@ std::string bytesToReadableString(const size_t num_bytes); common::PlatformResult fileModeToString(FileMode fm, std::string* fm_str); common::PlatformResult stringToFileMode(std::string fmString, FileMode* fm); -//extern Filesystem::FilePtr fileReferenceToFile( -// JSContextRef context, JSValueRef fileReference); - /** * Gets base path and name from full path string, example cases: * full path | base path | name diff --git a/src/archive/filesystem_file.cc b/src/archive/filesystem_file.cc index cbd8de7e..d5e9df51 100755 --- a/src/archive/filesystem_file.cc +++ b/src/archive/filesystem_file.cc @@ -21,6 +21,11 @@ using namespace common; namespace extension { namespace filesystem { +namespace { +const std::string kVirtualRootsDirectory = "/opt/usr/media/"; +const std::string kSlash = "/"; +} + File::File(NodePtr node, const File::PermissionList &parentPermissions, const std::string& original_location) : m_node(node), @@ -65,11 +70,11 @@ const std::string& File::getOriginalFullPath() const return m_original_fullpath; } -std::string External::toVirtualPath(const std::string& path) +std::string External::cutVirtualRoot(const std::string& path) { - //TODO::implement this method - LoggerW("Just STUB"); - return path; + LoggerD("Enter path %s", path.c_str()); + std::string tmp_path = path.substr(kVirtualRootsDirectory.length()); + return tmp_path.substr(tmp_path.find(kSlash)); } } // filesystem diff --git a/src/archive/filesystem_file.h b/src/archive/filesystem_file.h index 26401f29..b764bbdd 100755 --- a/src/archive/filesystem_file.h +++ b/src/archive/filesystem_file.h @@ -58,7 +58,7 @@ private: class External { public: - static std::string toVirtualPath(const std::string& path); + static std::string cutVirtualRoot(const std::string& path); }; diff --git a/src/archive/filesystem_node.cc b/src/archive/filesystem_node.cc index 4e2744b5..918b0c17 100755 --- a/src/archive/filesystem_node.cc +++ b/src/archive/filesystem_node.cc @@ -429,7 +429,6 @@ PlatformResult Node::getModified(std::time_t* time) const return PlatformResult(ErrorCode::NO_ERROR); } -// TODO Optimize it, maybe store a flag indicating that node is a root. PlatformResult Node::getParent(NodePtr* node) const { LoggerD("Enter"); @@ -638,13 +637,6 @@ PlatformResult Node::removeAsDirectory(const PathPtr& path, bool recursive) return PlatformResult(ErrorCode::NO_ERROR); } -std::string Node::toUri(int /*widgetId*/) const -{ - LoggerD("Enter"); - // TODO I believe moving this feature to WrtWrapper would make more sense. - return "file://" + m_path->getFullPath(); -} - bool Node::isSubPath(std::string aDirPath, PathPtr aFilePath) { LoggerD("Enter"); diff --git a/src/archive/filesystem_node.h b/src/archive/filesystem_node.h index 9cc9b522..a8b52995 100755 --- a/src/archive/filesystem_node.h +++ b/src/archive/filesystem_node.h @@ -226,8 +226,6 @@ public: //! - OPT_RECURSIVE - remove node recursively. common::PlatformResult remove(int options); - std::string toUri(int widgetId) const; - static bool isSubPath(std::string aDirPath, PathPtr aFilePath); private: diff --git a/src/archive/un_zip.cc b/src/archive/un_zip.cc index cc5616c2..b961e0e9 100755 --- a/src/archive/un_zip.cc +++ b/src/archive/un_zip.cc @@ -26,6 +26,7 @@ #include "common/logger.h" #include "common/platform_exception.h" +#include "common/tools.h" #include "filesystem_file.h" #include "archive_file.h" @@ -36,6 +37,7 @@ namespace extension { namespace archive { using namespace common; +using common::tools::GetErrorString; UnZip::UnZip(const std::string& filename) : m_zipfile_name(filename), @@ -50,6 +52,13 @@ UnZip::UnZip(const std::string& filename) : UnZip::~UnZip() { LoggerD("Enter"); + for (auto& x: path_access_map) { + LoggerD("Setting permission for path: %s [%d] ", x.first.c_str(), x.second); + if(chmod(x.first.c_str(), x.second) == -1) { + LoggerE("Couldn't set permissions for: [%s] errno: %s", x.first.c_str(), + GetErrorString(errno).c_str()); + } + } close(); } diff --git a/src/archive/un_zip.h b/src/archive/un_zip.h index 187d9c5b..68a30272 100755 --- a/src/archive/un_zip.h +++ b/src/archive/un_zip.h @@ -90,6 +90,13 @@ private: size_t m_default_buffer_size; bool m_is_open; + struct ComparePaths : public std::binary_function { + bool operator() (const std::string& lhs, const std::string& rhs) { + return lhs.length() > rhs.length(); + } + }; + std::map path_access_map; + friend class UnZipExtractRequest; }; diff --git a/src/archive/un_zip_extract_request.cc b/src/archive/un_zip_extract_request.cc index 2498d6e1..197d6b5d 100755 --- a/src/archive/un_zip_extract_request.cc +++ b/src/archive/un_zip_extract_request.cc @@ -95,13 +95,11 @@ void createMissingDirectories(const std::string& path, bool check_first = true) //LoggerD("left_part: [%s] status:%d", left_part.c_str(), status); if(FPS_DIRECTORY != status) { - //TODO investigate 0775 (mode) - filesystem assumed that file should have parent mode + // permissions would be changed after extract process would be finished, + // for now using default 0775 if(mkdir(left_part.c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH) == -1) { LoggerE("Couldn't create new directory: %s errno: %s", left_part.c_str(), GetErrorString(errno).c_str()); - //TODO check why mkdir return -1 but directory is successfully created - // throw UnknownException( - // "Could not create new directory"); } } } @@ -321,6 +319,8 @@ PlatformResult UnZipExtractRequest::handleDirectoryEntry() m_file_info.tmu_date.tm_min, m_file_info.tmu_date.tm_sec); + // change modify date and store permission for later update + storePermissions(); // Directory already exists we only need to update time changeFileAccessAndModifyDate(m_new_dir_path, m_file_info.tmu_date); @@ -341,14 +341,11 @@ PlatformResult UnZipExtractRequest::prepareOutputSubdirectory() "output path is invalid"); } - //Try to create new directory in output directory - //TODO investigate 0775 (mode) - filesystem assumed that file should have parent mode + // permissions would be changed after extract process would be finished, + // for now using default 0775 if(mkdir(m_new_dir_path.c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH) == -1) { LoggerW("couldn't create new directory: %s errno: %s", m_new_dir_path.c_str(), GetErrorString(errno).c_str()); - //TODO check why mkdir return -1 but directory is successfully created - // throw UnknownException( - // "Could not create new directory in extract output directory"); } } @@ -363,8 +360,6 @@ PlatformResult UnZipExtractRequest::prepareOutputSubdirectory() LoggerW("%s exists at output path: [%s], overwrite is set to FALSE", (FPS_DIRECTORY == output_fstatus ? "Directory" : "File"), m_output_filepath.c_str()); - - //Just skip this file - TODO: this should be documented in WIDL return PlatformResult(ErrorCode::INVALID_MODIFICATION_ERR, "file already exists."); } else { if(FPS_DIRECTORY == output_fstatus) { @@ -511,10 +506,23 @@ PlatformResult UnZipExtractRequest::handleFileEntry() m_output_file = NULL; } + // change modify date and store permission for later update + storePermissions(); changeFileAccessAndModifyDate(m_output_filepath, m_file_info.tmu_date); return PlatformResult(ErrorCode::NO_ERROR); } +void UnZipExtractRequest::storePermissions() { + // hold access information for later set + // The high 16 bits of the external file attributes seem to be used for OS-specific permissions + __mode_t mode = m_file_info.external_fa >> 16; + // check if proper permission mode is provided, if not use default 0775 + if (mode == 0) { + mode = S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH; + } + m_owner.path_access_map[m_output_filepath.c_str()] = mode; +} + } //namespace archive } //namespace extension diff --git a/src/archive/un_zip_extract_request.h b/src/archive/un_zip_extract_request.h index 6c12f7bc..ee7fabb7 100755 --- a/src/archive/un_zip_extract_request.h +++ b/src/archive/un_zip_extract_request.h @@ -50,6 +50,7 @@ private: PlatformResult handleDirectoryEntry(); PlatformResult handleFileEntry(); PlatformResult prepareOutputSubdirectory(); + void storePermissions(); //----------------------------------------------------------------------------- //Input request variables