Use absolute paths for archives and filesystems (#437)

Author: andiwand

src/odr/exceptions.cpp

@@ -126,4 +126,7 @@ DecryptionFailed::DecryptionFailed()
 NotEncryptedError::NotEncryptedError()
     : std::runtime_error("not encrypted error") {}
 
+InvalidPath::InvalidPath(const std::string &message)
+    : std::runtime_error("invalid path: " + message) {}
+
 } // namespace odr

src/odr/exceptions.hpp

@@ -215,4 +215,9 @@ struct NotEncryptedError : public std::runtime_error {
   explicit NotEncryptedError();
 };
 
+/// @brief Invalid path
+struct InvalidPath : public std::runtime_error {
+  explicit InvalidPath(const std::string &message);
+};
+
 } // namespace odr

src/odr/internal/cfb/cfb_archive.cpp

@@ -34,7 +34,6 @@ std::shared_ptr<abstract::Filesystem> CfbArchive::as_filesystem() const {
 
 void CfbArchive::save(std::ostream &out) const {
   (void)out;
-
   throw UnsupportedOperation();
 }
 

src/odr/internal/common/filesystem.cpp

@@ -19,7 +19,11 @@ class SystemFileWalker final : public abstract::FileWalker {
 public:
   SystemFileWalker(Path root, const Path &path)
       : m_root{std::move(root)},
-        m_iterator{std::filesystem::recursive_directory_iterator(path)} {}
+        m_iterator{std::filesystem::recursive_directory_iterator(path)} {
+    if (path.relative()) {
+      throw InvalidPath("SystemFileWalker: path must be absolute");
+    }
+  }
 
   [[nodiscard]] std::unique_ptr<FileWalker> clone() const final {
     return std::make_unique<SystemFileWalker>(*this);
@@ -69,40 +73,77 @@ Path SystemFilesystem::to_system_path_(const Path &path) const {
 }
 
 bool SystemFilesystem::exists(const Path &path) const {
+  if (path.relative()) {
+    throw InvalidPath("SystemFilesystem::exists: path must be absolute");
+  }
+
   return std::filesystem::exists(to_system_path_(path));
 }
 
 bool SystemFilesystem::is_file(const Path &path) const {
+  if (path.relative()) {
+    throw InvalidPath("SystemFilesystem::is_file: path must be absolute");
+  }
+
   return std::filesystem::is_regular_file(to_system_path_(path));
 }
 
 bool SystemFilesystem::is_directory(const Path &path) const {
+  if (path.relative()) {
+    throw InvalidPath("SystemFilesystem::is_directory: path must be absolute");
+  }
+
   return std::filesystem::is_directory(to_system_path_(path));
 }
 
 std::unique_ptr<abstract::FileWalker>
 SystemFilesystem::file_walker(const Path &path) const {
+  if (path.relative()) {
+    throw InvalidPath("SystemFilesystem::file_walker: path must be absolute");
+  }
+
   return std::make_unique<SystemFileWalker>(m_root, to_system_path_(path));
 }
 
 std::shared_ptr<abstract::File> SystemFilesystem::open(const Path &path) const {
+  if (path.relative()) {
+    throw InvalidPath("SystemFilesystem::open: path must be absolute");
+  }
+
   return std::make_unique<DiskFile>(to_system_path_(path));
 }
 
 std::unique_ptr<std::ostream> SystemFilesystem::create_file(const Path &path) {
+  if (path.relative()) {
+    throw InvalidPath("SystemFilesystem::create_file: path must be absolute");
+  }
+
   return std::make_unique<std::ofstream>(
       util::file::create(to_system_path_(path).string()));
 }
 
 bool SystemFilesystem::create_directory(const Path &path) {
+  if (path.relative()) {
+    throw InvalidPath(
+        "SystemFilesystem::create_directory: path must be absolute");
+  }
+
   return std::filesystem::create_directory(to_system_path_(path));
 }
 
 bool SystemFilesystem::remove(const Path &path) {
+  if (path.relative()) {
+    throw InvalidPath("SystemFilesystem::remove: path must be absolute");
+  }
+
   return std::filesystem::remove(to_system_path_(path));
 }
 
 bool SystemFilesystem::copy(const Path &from, const Path &to) {
+  if (from.relative() || to.relative()) {
+    throw InvalidPath("SystemFilesystem::copy: paths must be absolute");
+  }
+
   std::error_code error_code;
   std::filesystem::copy(to_system_path_(from), to_system_path_(to), error_code);
   if (error_code) {
@@ -113,6 +154,10 @@ bool SystemFilesystem::copy(const Path &from, const Path &to) {
 
 std::shared_ptr<abstract::File>
 SystemFilesystem::copy(const abstract::File &from, const Path &to) {
+  if (to.relative()) {
+    throw InvalidPath("SystemFilesystem::copy: path must be absolute");
+  }
+
   auto istream = from.stream();
   auto ostream = create_file(to_system_path_(to));
 
@@ -123,10 +168,18 @@ SystemFilesystem::copy(const abstract::File &from, const Path &to) {
 
 std::shared_ptr<abstract::File>
 SystemFilesystem::copy(std::shared_ptr<abstract::File> from, const Path &to) {
+  if (to.relative()) {
+    throw InvalidPath("SystemFilesystem::copy: path must be absolute");
+  }
+
   return copy(*from, to_system_path_(to));
 }
 
 bool SystemFilesystem::move(const Path &from, const Path &to) {
+  if (from.relative() || to.relative()) {
+    throw InvalidPath("SystemFilesystem::move: paths must be absolute");
+  }
+
   std::error_code error_code;
   std::filesystem::rename(to_system_path_(from), to_system_path_(to),
                           error_code);
@@ -142,6 +195,10 @@ class VirtualFileWalker final : public abstract::FileWalker {
   VirtualFileWalker(
       const Path &root,
       const std::map<Path, std::shared_ptr<abstract::File>> &files) {
+    if (root.relative()) {
+      throw InvalidPath("VirtualFileWalker: path must be absolute");
+    }
+
     for (auto &&f : files) {
       if (f.first.ancestor_of(root)) {
         m_files[f.first] = f.second;
@@ -195,10 +252,18 @@ class VirtualFileWalker final : public abstract::FileWalker {
 } // namespace
 
 bool VirtualFilesystem::exists(const Path &path) const {
+  if (path.relative()) {
+    throw InvalidPath("VirtualFilesystem::exists: path must be absolute");
+  }
+
   return m_files.find(path) != std::end(m_files);
 }
 
 bool VirtualFilesystem::is_file(const Path &path) const {
+  if (path.relative()) {
+    throw InvalidPath("VirtualFilesystem::is_file: path must be absolute");
+  }
+
   auto file_it = m_files.find(path);
   if (file_it == std::end(m_files)) {
     return false;
@@ -207,6 +272,10 @@ bool VirtualFilesystem::is_file(const Path &path) const {
 }
 
 bool VirtualFilesystem::is_directory(const Path &path) const {
+  if (path.relative()) {
+    throw InvalidPath("VirtualFilesystem::is_directory: path must be absolute");
+  }
+
   auto file_it = m_files.find(path);
   if (file_it == std::end(m_files)) {
     return false;
@@ -216,11 +285,19 @@ bool VirtualFilesystem::is_directory(const Path &path) const {
 
 std::unique_ptr<abstract::FileWalker>
 VirtualFilesystem::file_walker(const Path &path) const {
-  return std::make_unique<VirtualFileWalker>(std::move(path), m_files);
+  if (path.relative()) {
+    throw InvalidPath("VirtualFilesystem::file_walker: path must be absolute");
+  }
+
+  return std::make_unique<VirtualFileWalker>(path, m_files);
 }
 
 std::shared_ptr<abstract::File>
 VirtualFilesystem::open(const Path &path) const {
+  if (path.relative()) {
+    throw InvalidPath("VirtualFilesystem::open: path must be absolute");
+  }
+
   auto file_it = m_files.find(path);
   if (file_it == std::end(m_files)) {
     return {};
@@ -234,6 +311,11 @@ VirtualFilesystem::create_file(const Path & /*path*/) {
 }
 
 bool VirtualFilesystem::create_directory(const Path &path) {
+  if (path.relative()) {
+    throw InvalidPath(
+        "VirtualFilesystem::create_directory: path must be absolute");
+  }
+
   if (exists(path)) {
     return false;
   }
@@ -242,6 +324,10 @@ bool VirtualFilesystem::create_directory(const Path &path) {
 }
 
 bool VirtualFilesystem::remove(const Path &path) {
+  if (path.relative()) {
+    throw InvalidPath("VirtualFilesystem::remove: path must be absolute");
+  }
+
   auto file_it = m_files.find(path);
   if (file_it == std::end(m_files)) {
     return false;
@@ -251,6 +337,10 @@ bool VirtualFilesystem::remove(const Path &path) {
 }
 
 bool VirtualFilesystem::copy(const Path &from, const Path &to) {
+  if (from.relative() || to.relative()) {
+    throw InvalidPath("VirtualFilesystem::copy: paths must be absolute");
+  }
+
   // TODO what about directories?
 
   auto from_it = m_files.find(from);
@@ -271,6 +361,10 @@ VirtualFilesystem::copy(const abstract::File & /*from*/, const Path & /*to*/) {
 
 std::shared_ptr<abstract::File>
 VirtualFilesystem::copy(std::shared_ptr<abstract::File> from, const Path &to) {
+  if (to.relative()) {
+    throw InvalidPath("VirtualFilesystem::copy: path must be absolute");
+  }
+
   if (exists(to)) {
     return {};
   }
@@ -279,6 +373,10 @@ VirtualFilesystem::copy(std::shared_ptr<abstract::File> from, const Path &to) {
 }
 
 bool VirtualFilesystem::move(const Path &from, const Path &to) {
+  if (from.relative() || to.relative()) {
+    throw InvalidPath("VirtualFilesystem::move: paths must be absolute");
+  }
+
   if (!copy(from, to)) {
     return false;
   }

src/odr/internal/common/path.cpp

@@ -186,7 +186,11 @@ Path Path::join(const Path &b) const {
 }
 
 Path Path::rebase(const Path &b) const {
-  Path result = Path(m_absolute ? "/" : "");
+  if (m_absolute != b.m_absolute) {
+    throw std::invalid_argument("cannot rebase absolute and relative path");
+  }
+
+  Path result = Path("");
   auto common_root = this->common_root(b);
 
   Path sub_a;
@@ -217,7 +221,7 @@ Path Path::rebase(const Path &b) const {
     result.join_("..");
   }
 
-  for (auto part : sub_a) {
+  for (const auto &part : sub_a) {
     result.join_(part);
   }
 
@@ -251,7 +255,7 @@ Path Path::common_root(const Path &b) const {
 
 Path::Iterator Path::begin() const { return Iterator(*this); }
 
-Path::Iterator Path::end() const { return Iterator(); }
+Path::Iterator Path::end() const { return {}; }
 
 Path::Iterator::Iterator() : m_path{nullptr}, m_begin{std::string::npos} {}
 

src/odr/internal/html/filesystem.cpp

@@ -65,7 +65,7 @@ class HtmlServiceImpl : public HtmlService {
   HtmlResources write_filesystem(HtmlWriter &out) const {
     HtmlResources resources;
 
-    auto file_walker = m_filesystem.file_walker("");
+    auto file_walker = m_filesystem.file_walker("/");
 
     out.write_begin();
 

src/odr/internal/odf/odf_crypto.cpp

@@ -170,7 +170,7 @@ odf::decrypt(const std::shared_ptr<abstract::ReadableFilesystem> &filesystem,
     throw NotEncryptedError();
   }
 
-  if (auto it = manifest.entries.find(common::Path("encrypted-package"));
+  if (auto it = manifest.entries.find(common::Path("/encrypted-package"));
       it != std::end(manifest.entries)) {
     try {
       const std::string start_key = odf::start_key(it->second, password);

src/odr/internal/odf/odf_document.cpp

@@ -18,10 +18,10 @@ Document::Document(const FileType file_type, const DocumentType document_type,
                    std::shared_ptr<abstract::ReadableFilesystem> filesystem)
     : common::TemplateDocument<Element>(file_type, document_type,
                                         std::move(filesystem)) {
-  m_content_xml = util::xml::parse(*m_filesystem, common::Path("content.xml"));
+  m_content_xml = util::xml::parse(*m_filesystem, common::Path("/content.xml"));
 
-  if (m_filesystem->exists(common::Path("styles.xml"))) {
-    m_styles_xml = util::xml::parse(*m_filesystem, common::Path("styles.xml"));
+  if (m_filesystem->exists(common::Path("/styles.xml"))) {
+    m_styles_xml = util::xml::parse(*m_filesystem, common::Path("/styles.xml"));
   }
 
   m_root_element = parse_tree(
@@ -43,33 +43,33 @@ void Document::save(const common::Path &path) const {
   zip::ZipArchive archive;
 
   // `mimetype` has to be the first file and uncompressed
-  if (m_filesystem->is_file(common::Path("mimetype"))) {
-    archive.insert_file(std::end(archive), common::Path("mimetype"),
-                        m_filesystem->open(common::Path("mimetype")), 0);
+  if (m_filesystem->is_file(common::Path("/mimetype"))) {
+    archive.insert_file(std::end(archive), common::Path("/mimetype"),
+                        m_filesystem->open(common::Path("/mimetype")), 0);
   }
 
-  for (auto walker = m_filesystem->file_walker(common::Path(""));
+  for (auto walker = m_filesystem->file_walker(common::Path("/"));
        !walker->end(); walker->next()) {
     auto p = walker->path();
-    if (p == common::Path("mimetype")) {
+    if (p == common::Path("/mimetype")) {
       continue;
     }
     if (m_filesystem->is_directory(p)) {
       archive.insert_directory(std::end(archive), p);
       continue;
     }
-    if (p == common::Path("content.xml")) {
+    if (p == common::Path("/content.xml")) {
       // TODO stream
       std::stringstream out;
       m_content_xml.print(out, "", pugi::format_raw);
       auto tmp = std::make_shared<common::MemoryFile>(out.str());
       archive.insert_file(std::end(archive), p, tmp);
       continue;
     }
-    if (p == common::Path("META-INF/manifest.xml")) {
+    if (p == common::Path("/META-INF/manifest.xml")) {
       // TODO
       auto manifest = util::xml::parse(*m_filesystem,
-                                       common::Path("META-INF/manifest.xml"));
+                                       common::Path("/META-INF/manifest.xml"));
 
       for (auto &&node : manifest.select_nodes("//manifest:encryption-data")) {
         node.node().parent().remove_child(node.node());

src/odr/internal/odf/odf_element.cpp

@@ -441,7 +441,11 @@ bool Image::is_internal(const abstract::Document *document) const {
     return false;
   }
   try {
-    return doc->as_filesystem()->is_file(common::Path(href(document)));
+    common::Path path(href(document));
+    if (path.relative()) {
+      path = common::Path("/").join(path);
+    }
+    return doc->as_filesystem()->is_file(path);
   } catch (...) {
   }
   return false;
@@ -452,7 +456,11 @@ std::optional<odr::File> Image::file(const abstract::Document *document) const {
   if (!doc || !is_internal(document)) {
     return {};
   }
-  return File(doc->as_filesystem()->open(common::Path(href(document))));
+  common::Path path(href(document));
+  if (path.relative()) {
+    path = common::Path("/").join(path);
+  }
+  return File(doc->as_filesystem()->open(path));
 }
 
 std::string Image::href(const abstract::Document *) const {