Native canonicalFileName() makes an existing relative file name absolute, unlike the WASM build

VERSION.txt

@@ -1 +1 @@
-1.20261015.0
+1.20261016.0

cmake/common.cmake

@@ -344,7 +344,7 @@ function(configure_target TARGET)
     # Note: we also get duplicate objects when linking the target on Windows, but for some reasons to use /ignore:4006
     #       doesn't work...!?
 
-    if(BUILDING_USING_CLANG AND NOT LIBOPENCOR_CODE_ANALYSIS)
+    if(APPLE)
         add_linker_flags(${TARGET} -Wl,-no_warn_duplicate_libraries)
     endif()
 

src/misc/utils.cpp

@@ -424,7 +424,7 @@ std::string decodeUrl(const std::string &pUrl, bool pDecodeReservedCharacters)
     return res;
 }
 
-#ifdef BUILDING_USING_MSVC
+#ifdef BUILDING_ON_WINDOWS
 std::string forwardSlashPath(const std::string &pPath)
 {
     static constexpr auto BACKSLASH {"\\\\"};
@@ -435,7 +435,7 @@ std::string forwardSlashPath(const std::string &pPath)
 }
 #endif
 
-#ifdef BUILDING_USING_MSVC
+#ifdef BUILDING_ON_WINDOWS
 namespace {
 
 // Conversion between a UTF-8 std::string and a UTF-16 std::wstring, to be used instead of the one done by
@@ -579,7 +579,7 @@ std::wstring stringToUtf16(const std::string &pString)
 
 std::filesystem::path stringToPath(const std::string &pString)
 {
-#ifdef BUILDING_USING_MSVC
+#ifdef BUILDING_ON_WINDOWS
     return stringToUtf16(pString);
 #else
     return std::u8string {pString.begin(), pString.end()};
@@ -590,7 +590,7 @@ std::string pathToString(const std::filesystem::path &pPath)
 {
     auto path {pPath};
 
-#ifdef BUILDING_USING_MSVC
+#ifdef BUILDING_ON_WINDOWS
     return utf16ToUtf8(path.make_preferred().native());
 #else
     return path.make_preferred().string();
@@ -625,32 +625,81 @@ std::string canonicalFileName(const std::string &pFileName)
         return pathToString(filePath.lexically_normal());
     }
 
-    // If the file exists then use std::filesystem::canonical() since it resolves symbolic links, etc.
-    // Note: this fails if the file doesn't exist.
+    // If the file name has a root directory (i.e., it is an absolute file name or, on Windows, a file name relative to
+    // the root of the current drive) then use std::filesystem::canonical() if the file exists since it resolves
+    // symbolic links, etc. or std::filesystem::weakly_canonical() otherwise since it returns a file name that is as
+    // close to the canonical version as possible.
+    // Note: we don't do this for a relative file name since both functions would resolve it against the current working
+    //       directory, meaning that its canonical version would depend on whether a file with that name happens to
+    //       exist in the current working directory.
 
-    std::error_code errorCode;
-    auto res {std::filesystem::canonical(filePath, errorCode)};
-
-    if (!errorCode) {
-        return pathToString(res);
-    }
+    if (filePath.has_root_directory()) {
+        std::error_code errorCode;
+        auto res {std::filesystem::canonical(filePath, errorCode)};
 
-    // The file doesn't exist, so if its file name has a root directory (i.e., it is an absolute file name or, on
-    // Windows, a file name relative to the root of the current drive) then use std::filesystem::weakly_canonical()
-    // since it returns a file name that is as close to the canonical version as possible.
+        if (!errorCode) {
+            return pathToString(res);
+        }
 
-    if (filePath.has_root_directory()) {
         res = std::filesystem::weakly_canonical(filePath, errorCode);
 
         if (!errorCode) {
+            return pathToString(res);
+        }
+    } else {
+        // The file name is relative, so normalise it lexically (see below), except that a ".." component must not
+        // cancel a component that is a symbolic link since it then refers to the parent directory of the target of that
+        // symbolic link (e.g., "link/../file.txt" doesn't refer to "file.txt" if "link" is a symbolic link to
+        // "some/path/dir", but to "some/path/file.txt"), so we keep both components.
+        // Note: whether a component is a symbolic link depends on the current working directory, but a file name that
+        //       has no symbolic link followed by a ".." component is normalised lexically, as below.
+
+        std::filesystem::path res;
+        auto hasSymbolicLink {false};
+        auto hasTrailingSeparator {false};
+
+        for (const auto &component : filePath) {
+            hasTrailingSeparator = false;
+
+            if (component == "..") {
+                if (res.has_relative_path() && (res.filename() != "..")) {
+                    std::error_code errorCode;
+
+                    if (!std::filesystem::is_symlink(res, errorCode)) {
+                        res = res.parent_path();
+                        hasTrailingSeparator = true;
+
+                        continue;
+                    }
+
+                    hasSymbolicLink = true;
+                }
+
+                res /= component;
+            } else if (component.empty() || (component == ".")) {
+                hasTrailingSeparator = true;
+            } else {
+                res /= component;
+            }
+        }
+
+        // Like std::filesystem::path::lexically_normal(), keep a trailing directory separator, unless the last
+        // component is "..".
+
+        if (hasSymbolicLink) {
+            if (hasTrailingSeparator && (res.filename() != "..")) {
+                res /= "";
+            }
+
             return pathToString(res);
         }
     }
 #endif
 
-    // The file name is relative (or something went wrong), so normalise it lexically. Unlike
-    // std::filesystem::weakly_canonical(), this doesn't depend on the current working directory, and it keeps leading
-    // ".." components (e.g., "a/../../b" becomes "../b").
+    // The file name is relative (and has no symbolic link followed by a ".." component) or something went wrong, so
+    // normalise it lexically. Unlike std::filesystem::canonical() and std::filesystem::weakly_canonical(), this doesn't
+    // depend on the current working directory (whether the file exists or not), and it keeps leading ".." components
+    // (e.g., "a/../../b" becomes "../b").
 
     return pathToString(filePath.lexically_normal());
 }
@@ -668,7 +717,7 @@ std::string canonicalUrl(const std::string &pUrl)
     static constexpr auto DOT {"."};
     static constexpr auto DOT_DOT {".."};
 
-#ifdef BUILDING_USING_MSVC
+#ifdef BUILDING_ON_WINDOWS
     auto url {forwardSlashPath(pUrl)};
 #else
     const auto &url {pUrl};
@@ -891,7 +940,7 @@ std::string urlPath(const std::string &pPath)
 
     if (isLocalPath && stringToPath(path).is_absolute()) {
         static const std::string FILE_SCHEME {"file://"};
-#ifdef BUILDING_USING_MSVC
+#ifdef BUILDING_ON_WINDOWS
         static constexpr auto FORWARD_SLASH {"/"};
 
         return FILE_SCHEME + FORWARD_SLASH + forwardSlashPath(path);
@@ -900,6 +949,15 @@ std::string urlPath(const std::string &pPath)
 #endif
     }
 
+#ifdef BUILDING_ON_WINDOWS
+    // A relative file name is used as is, but its backslashes must become forward slashes since it is used as a
+    // relative URI reference (e.g., "some\path\model.cellml" must become "some/path/model.cellml").
+
+    if (isLocalPath) {
+        return forwardSlashPath(path);
+    }
+#endif
+
     return path;
 }
 

src/misc/utils.h

@@ -87,7 +87,7 @@ std::string LIBOPENCOR_UNIT_TESTING_EXPORT percentEncodeNonXmlCharacters(const s
 std::string LIBOPENCOR_UNIT_TESTING_EXPORT encodeUrl(const std::string &pUrl);
 std::string LIBOPENCOR_UNIT_TESTING_EXPORT decodeUrl(const std::string &pUrl, bool pDecodeReservedCharacters = true);
 
-#ifdef BUILDING_USING_MSVC
+#ifdef BUILDING_ON_WINDOWS
 std::string LIBOPENCOR_UNIT_TESTING_EXPORT forwardSlashPath(const std::string &pPath);
 #endif
 

src/sed/sedmodel.cpp

@@ -165,7 +165,7 @@ void SedModel::Impl::serialise(xmlNodePtr pNode) const
 
     auto source {mBasePath.empty() ?
                      urlPath(mFile->path()) :
-#ifdef BUILDING_USING_MSVC
+#ifdef BUILDING_ON_WINDOWS
                      forwardSlashPath(relativePath(mFile->path(), mBasePath))};
 #else
                      relativePath(mFile->path(), mBasePath)};
@@ -200,10 +200,12 @@ void SedModel::Impl::serialise(xmlNodePtr pNode) const
     // A relative model source which first segment contains a colon (e.g., "my:model.cellml") would be mistaken for a URI
     // with a scheme (i.e. "my"), so we prefix it with "./" (i.e. "./my:model.cellml"), as recommended by
     // https://www.rfc-editor.org/rfc/rfc3986#section-4.2.
-    // Note: a model source is only relative if we were given a base path (see relativePath()), so we don't need to worry
-    //       about the drive letter of a Windows file name (e.g., "C:/some/path/model.cellml").
+    // Note: a model source is relative if we were given a base path (see relativePath()) or if the file name of a local
+    //       model is relative (see urlPath()). An absolute file name, including one with the drive letter of a Windows
+    //       file name (e.g., "C:/some/path/model.cellml"), becomes a "file" URL, which must not be prefixed.
 
-    if (!mBasePath.empty() && (source.find(':') < source.find('/'))) {
+    if ((!mBasePath.empty() || (mFile->url().empty() && !stringToPath(mFile->path()).is_absolute()))
+        && (source.find(':') < source.find('/'))) {
         source.insert(0, "./");
     }
 

src/support/cellml/cellmlfileruntime.cpp

@@ -190,7 +190,7 @@ extern void nlaSolve(uintptr_t nlaSolverAddress, size_t objectiveFunctionIndex,
             generatorProfile->setNlaSolveCallString(differentialModel, WITH_EXTERNAL_VARIABLES,
                                                     "nlaSolve(nlaSolverAddress(), [INDEX], u, [SIZE], &rfi);\n");
 #else
-#    ifdef BUILDING_USING_MSVC
+#    ifdef BUILDING_ON_WINDOWS
             generatorProfile->setExternNlaSolveMethodString(R"(typedef unsigned long long uintptr_t;
 typedef unsigned long long size_t;
 

src/support/combine/combinearchive.cpp

@@ -22,6 +22,7 @@ limitations under the License.
 #include "combine/combinearchive.h"
 #include "omex/CaContent.h"
 
+#include <algorithm>
 #include <atomic>
 #include <filesystem>
 #include <unordered_map>
@@ -87,12 +88,26 @@ CombineArchive::Impl::Impl(const FilePtr &pFile, libcombine::CombineArchive *pAr
         //          could then be confused with another managed file (which contents would get replaced) or even with
         //          the COMBINE archive itself (resulting in an infinite recursion), so we ignore such a file. To
         //          determine where the file is located, we use its file name as determined by File::create().
+        // Note #3: the file name of a file that is located within the COMBINE archive has no ".." component after the
+        //          location of the COMBINE archive, unless it follows a symbolic link (see canonicalFileName()), in
+        //          which case the file may be located outside of the COMBINE archive (e.g.,
+        //          "archive.omex.contents/../file.cellml" if "archive.omex.contents" is a symbolic link), so we also
+        //          ignore such a file.
 
         auto decodedLocation {decodeUrl(location)};
         auto fileNameOrUrl {mArchiveLocation + decodedLocation};
         auto [isLocalFile, fileName] {retrieveFileInfo(fileNameOrUrl)};
+        auto isLocatedWithinArchive {fileName.starts_with(mArchiveLocation)};
 
-        if (!fileName.starts_with(mArchiveLocation)) {
+        if (isLocatedWithinArchive) {
+            auto filePathWithinArchive {stringToPath(fileName.substr(mArchiveLocationSize))};
+
+            isLocatedWithinArchive = std::ranges::none_of(filePathWithinArchive, [](const auto &pComponent) {
+                return pComponent == "..";
+            });
+        }
+
+        if (!isLocatedWithinArchive) {
             std::string warning;
 
             warning.reserve(location.size() + 64); // NOLINT

src/support/sedml/sedmlfile.cpp

@@ -387,9 +387,9 @@ void SedmlFile::Impl::populateDocument(const SedDocumentPtr &pDocument)
     for (unsigned int i {0}; i < mDocument->getNumModels(); ++i) {
         auto source {mDocument->getModel(i)->getSource()};
 
-        // Note #1: a relative model source is relative to the SED-ML file, not to the current working directory, so
-        //          we must not canonicalise it before we know whether it is relative (a model source that exists
-        //          relative to the current working directory would otherwise become absolute).
+        // Note #1: a relative model source is relative to the SED-ML file, not to the current working directory, so we
+        //          only canonicalise it once we have resolved it against the location of the SED-ML file (see
+        //          FileManager::file() and File::create()).
         // Note #2: a model source is a URI reference (see https://www.rfc-editor.org/rfc/rfc3986#section-4.1), so it
         //          may be percent-encoded (e.g., "my%20model.cellml"). If the SED-ML file is a local file and the model
         //          source is a file name (i.e. neither a URL nor a "file" URL, which path is decoded by

tests/api/file/basictests.cpp

@@ -63,18 +63,21 @@ TEST(BasicFileTest, localFile)
 
 TEST(BasicFileTest, existingRelativeLocalFile)
 {
-    // Note: this has no JavaScript equivalent since our JavaScript bindings never retrieve the contents of a file.
+    // An existing relative file name must stay relative, i.e. it must not be resolved against the current working
+    // directory.
 
     auto origDir {std::filesystem::current_path()};
 
     std::filesystem::current_path(libOpenCOR::resourcePath());
 
     auto file {libOpenCOR::File::create("cellml_2.cellml")};
 
+    std::filesystem::current_path(origDir);
+
+    EXPECT_EQ(file->fileName(), "cellml_2.cellml");
+    EXPECT_EQ(file->path(), "cellml_2.cellml");
     EXPECT_FALSE(file->contents().empty());
     EXPECT_EQ_ISSUES(file, expectedNoIssues());
-
-    std::filesystem::current_path(origDir);
 }
 
 TEST(BasicFileTest, nonExistingRelativeLocalFile)

tests/api/file/childtests.cpp

@@ -18,6 +18,7 @@ limitations under the License.
 
 #include "tests/utils.h"
 
+#include <filesystem>
 #include <libopencor>
 
 namespace {
@@ -155,6 +156,47 @@ TEST(ChildFileTest, combineArchiveWithFilesOutsideOfIt)
     EXPECT_EQ(libOpenCOR::FileManager::instance().fileCount(), 3U);
 }
 
+#ifndef BUILDING_ON_WINDOWS
+TEST(ChildFileTest, combineArchiveWithFilesOutsideOfItThroughSymbolicLink)
+{
+    // The files of a COMBINE archive are located in a directory which name is that of the COMBINE archive followed by
+    // ".contents". If such a directory happens to exist and to be a symbolic link then the ".." components that follow
+    // it are kept (see canonicalFileName()), but the files they refer to are still located outside of the COMBINE
+    // archive, so they must still be ignored.
+    // Note: a symbolic link usually requires some special privileges on Windows.
+
+    static const libOpenCOR::ExpectedIssues EXPECTED_ISSUES {{
+        {libOpenCOR::Issue::Type::WARNING, "COMBINE archive: the file '../sibling.cellml' is not located within the COMBINE archive. It has been ignored."},
+        {libOpenCOR::Issue::Type::WARNING, "COMBINE archive: the file '%2E%2E/encoded_sibling.cellml' is not located within the COMBINE archive. It has been ignored."},
+        {libOpenCOR::Issue::Type::WARNING, "COMBINE archive: the file '../entries_outside_of_archive.omex' is not located within the COMBINE archive. It has been ignored."},
+    }};
+
+    auto dir {std::filesystem::temp_directory_path() / "libopencor_combine_archive_with_files_outside_of_it_through_symbolic_link"};
+
+    std::filesystem::remove_all(dir);
+    std::filesystem::create_directories(dir / "real");
+    std::filesystem::create_directories(dir / "cwd");
+    std::filesystem::copy_file(libOpenCOR::resourcePath("api/file/entries_outside_of_archive.omex"), dir / "cwd/entries_outside_of_archive.omex");
+    std::filesystem::create_directory_symlink("../real", dir / "cwd/entries_outside_of_archive.omex.contents");
+
+    auto origDir {std::filesystem::current_path()};
+
+    std::filesystem::current_path(dir / "cwd");
+
+    auto file {libOpenCOR::File::create("entries_outside_of_archive.omex")};
+
+    std::filesystem::current_path(origDir);
+
+    EXPECT_EQ(file->type(), libOpenCOR::File::Type::COMBINE_ARCHIVE);
+    EXPECT_EQ(file->childFileCount(), 1U);
+    EXPECT_NE(file->childFile("model.cellml"), nullptr);
+    EXPECT_EQ_ISSUES(file, EXPECTED_ISSUES);
+    EXPECT_EQ(libOpenCOR::FileManager::instance().fileCount(), 2U);
+
+    std::filesystem::remove_all(dir);
+}
+#endif
+
 TEST(ChildFileTest, combineArchiveWithPercentEncodedLocations)
 {
     // A location is a URI reference, so it may be percent-encoded (e.g., "./my%20file.txt"), in which case the name of

tests/api/sed/basictests.cpp

@@ -146,6 +146,42 @@ TEST(BasicSedTest, sedmlFileWithRelativeCellmlFileInWorkingDirectory)
     EXPECT_EQ(document->models()[0]->file()->path(), libOpenCOR::resourcePath("api/sed/cellml_2.cellml"));
 }
 
+#ifndef BUILDING_ON_WINDOWS
+TEST(BasicSedTest, sedmlFileWithRelativeCellmlFileThroughSymbolicLink)
+{
+    // A relative model source that starts with a ".." component is relative to the actual location of the SED-ML file,
+    // i.e. to the parent directory of the target of a symbolic link if the SED-ML file is accessed through it (e.g.,
+    // "link/../model.cellml" refers to "real/model.cellml" if "link" is a symbolic link to "real/sub").
+    // Note: a symbolic link usually requires some special privileges on Windows.
+
+    auto dir {std::filesystem::temp_directory_path() / "libopencor_sedml_file_with_relative_cellml_file_through_symbolic_link"};
+
+    std::filesystem::remove_all(dir);
+    std::filesystem::create_directories(dir / "real/sub");
+    std::filesystem::create_directories(dir / "cwd");
+    std::filesystem::copy_file(libOpenCOR::resourcePath("cellml_2.cellml"), dir / "real/model.cellml");
+    std::filesystem::create_directory_symlink("../real/sub", dir / "cwd/link");
+
+    auto origDir {std::filesystem::current_path()};
+
+    std::filesystem::current_path(dir / "cwd");
+
+    auto file {libOpenCOR::File::create("link/simulation.sedml", false)};
+
+    file->setContents(libOpenCOR::charArrayToUnsignedChars(sedmlContents("../model.cellml").c_str()));
+
+    auto document {libOpenCOR::SedDocument::create(file)};
+
+    std::filesystem::current_path(origDir);
+
+    EXPECT_EQ(document->models().size(), 1U);
+    EXPECT_EQ(document->models()[0]->file()->path(), "link/../model.cellml");
+    EXPECT_EQ(document->models()[0]->file()->type(), libOpenCOR::File::Type::CELLML_FILE);
+
+    std::filesystem::remove_all(dir);
+}
+#endif
+
 TEST(BasicSedTest, sedmlFileWithUnsupportedSchemeCellmlFile)
 {
     // A model source with a scheme other than "http", "https", or "file" is an absolute URI that we cannot retrieve, so