diff --git a/VERSION.txt b/VERSION.txt index 908436027..f800d458e 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -1.20261015.0 +1.20261016.0 diff --git a/cmake/common.cmake b/cmake/common.cmake index 676c34152..b838ad87a 100644 --- a/cmake/common.cmake +++ b/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() diff --git a/src/misc/utils.cpp b/src/misc/utils.cpp index 96a9a080f..7c3f342f4 100644 --- a/src/misc/utils.cpp +++ b/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; } diff --git a/src/misc/utils.h b/src/misc/utils.h index 09ba25a50..e2c7190e1 100644 --- a/src/misc/utils.h +++ b/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 diff --git a/src/sed/sedmodel.cpp b/src/sed/sedmodel.cpp index 1ed591201..48571bd7c 100644 --- a/src/sed/sedmodel.cpp +++ b/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, "./"); } diff --git a/src/support/cellml/cellmlfileruntime.cpp b/src/support/cellml/cellmlfileruntime.cpp index 79a199cbf..91182e092 100644 --- a/src/support/cellml/cellmlfileruntime.cpp +++ b/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; diff --git a/src/support/combine/combinearchive.cpp b/src/support/combine/combinearchive.cpp index d32492d19..2dad14d84 100644 --- a/src/support/combine/combinearchive.cpp +++ b/src/support/combine/combinearchive.cpp @@ -22,6 +22,7 @@ limitations under the License. #include "combine/combinearchive.h" #include "omex/CaContent.h" +#include #include #include #include @@ -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 diff --git a/src/support/sedml/sedmlfile.cpp b/src/support/sedml/sedmlfile.cpp index cd732fd7f..276498f3d 100644 --- a/src/support/sedml/sedmlfile.cpp +++ b/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 diff --git a/tests/api/file/basictests.cpp b/tests/api/file/basictests.cpp index a9cfd8666..9cc65ea18 100644 --- a/tests/api/file/basictests.cpp +++ b/tests/api/file/basictests.cpp @@ -63,7 +63,8 @@ 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()}; @@ -71,10 +72,12 @@ TEST(BasicFileTest, existingRelativeLocalFile) 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) diff --git a/tests/api/file/childtests.cpp b/tests/api/file/childtests.cpp index e330163ea..0d31d83c0 100644 --- a/tests/api/file/childtests.cpp +++ b/tests/api/file/childtests.cpp @@ -18,6 +18,7 @@ limitations under the License. #include "tests/utils.h" +#include #include 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 diff --git a/tests/api/sed/basictests.cpp b/tests/api/sed/basictests.cpp index da2ec3cd2..00c266ff1 100644 --- a/tests/api/sed/basictests.cpp +++ b/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 diff --git a/tests/api/sed/coveragetests.cpp b/tests/api/sed/coveragetests.cpp index 3fcf15179..feec4190e 100644 --- a/tests/api/sed/coveragetests.cpp +++ b/tests/api/sed/coveragetests.cpp @@ -109,7 +109,7 @@ std::string sedChangesExpectedSerialisation(const std::string &pSource, const st serialise(), cvodeExpectedSerialisation("cellml_2.cellml")); } +TEST(SerialiseSedTest, relativeLocalCellmlFileInDirectoryWithoutBasePath) +{ + // A relative file name is serialised as a relative model source, so its directory separators must be forward + // slashes (i.e. not backslashes on Windows). + + auto file {libOpenCOR::File::create("some/path/cellml_2.cellml", false)}; + + file->setContents(libOpenCOR::charArrayToUnsignedChars(libOpenCOR::textFileContents(libOpenCOR::resourcePath("cellml_2.cellml")).c_str())); + + auto document {libOpenCOR::SedDocument::create(file)}; + + EXPECT_EQ(document->serialise(), cvodeExpectedSerialisation("some/path/cellml_2.cellml")); + + // Load the SED-ML file back. + + auto sedmlFile {libOpenCOR::File::create("my_model.sedml", false)}; + + sedmlFile->setContents(libOpenCOR::charArrayToUnsignedChars(document->serialise().c_str())); + + auto sedmlDocument {libOpenCOR::SedDocument::create(sedmlFile)}; + + EXPECT_FALSE(sedmlDocument->hasIssues()); + EXPECT_EQ(sedmlDocument->models().size(), 1U); + EXPECT_EQ(sedmlDocument->models()[0]->file(), file); + + libOpenCOR::FileManager::instance().unmanage(sedmlFile); +} + TEST(SerialiseSedTest, localCellmlFileWithPercentSigns) { // A model source is a URI reference, so the percent signs in the file name of a local model must be percent-encoded @@ -267,6 +295,36 @@ TEST(SerialiseSedTest, localCellmlFileWithColon) libOpenCOR::FileManager::instance().unmanage(sedmlFile); } } + +TEST(SerialiseSedTest, relativeLocalCellmlFileWithColon) +{ + // A relative file name is serialised as a relative model source (even without a base path), so it must also be + // prefixed with "./" if its first segment contains a colon. + // Note: a file name cannot contain a colon on Windows. + + auto cellmlContents {libOpenCOR::charArrayToUnsignedChars(libOpenCOR::textFileContents(libOpenCOR::resourcePath("cellml_2.cellml")).c_str())}; + auto file {libOpenCOR::File::create("my:model.cellml", false)}; + + file->setContents(cellmlContents); + + auto document {libOpenCOR::SedDocument::create(file)}; + + EXPECT_EQ(document->serialise(), cvodeExpectedSerialisation("./my:model.cellml")); + + // Load the SED-ML file back. + + auto sedmlFile {libOpenCOR::File::create("my_model.sedml", false)}; + + sedmlFile->setContents(libOpenCOR::charArrayToUnsignedChars(document->serialise().c_str())); + + auto sedmlDocument {libOpenCOR::SedDocument::create(sedmlFile)}; + + EXPECT_FALSE(sedmlDocument->hasIssues()); + EXPECT_EQ(sedmlDocument->models().size(), 1U); + EXPECT_EQ(sedmlDocument->models()[0]->file(), file); + + libOpenCOR::FileManager::instance().unmanage(sedmlFile); +} #endif TEST(SerialiseSedTest, localCellmlFileWithNonUtf8Character) diff --git a/tests/bindings/javascript/file.basic.test.js b/tests/bindings/javascript/file.basic.test.js index 75600aad6..b9b29a138 100644 --- a/tests/bindings/javascript/file.basic.test.js +++ b/tests/bindings/javascript/file.basic.test.js @@ -51,6 +51,25 @@ test.describe('File basic tests', () => { assertIssues(loc, file, expectedNoIssues); }); + test('Existing relative local file', () => { + // An existing relative file name must stay relative, i.e. it must not be resolved against the current working + // directory. + // Note: the file has no contents since our JavaScript bindings never retrieve the contents of a file. + + const origDir = process.cwd(); + + process.chdir(utils.resourcePath()); + + const file = new loc.File('cellml_2.cellml'); + + process.chdir(origDir); + + assert.strictEqual(file.fileName, 'cellml_2.cellml'); + assert.strictEqual(file.path, 'cellml_2.cellml'); + assert.deepStrictEqual(file.contents(), Uint8Array.from([])); + assertIssues(loc, file, expectedNoIssues); + }); + test('Non-existing relative local file', () => { // Note: the file is of unknown type and has no issues since our JavaScript bindings never retrieve the contents of // a file. diff --git a/tests/bindings/javascript/file.child.test.js b/tests/bindings/javascript/file.child.test.js index eb90664b7..929ad086c 100644 --- a/tests/bindings/javascript/file.child.test.js +++ b/tests/bindings/javascript/file.child.test.js @@ -15,6 +15,9 @@ limitations under the License. */ import assert from 'node:assert'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; import test from 'node:test'; import libOpenCOR from './libopencor.js'; @@ -161,6 +164,58 @@ test.describe('File child tests', () => { assert.strictEqual(loc.FileManager.instance().fileCount, 3); }); + test('COMBINE archive with files outside of it through symbolic link', () => { + // 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: our JavaScript bindings use a virtual file system, so a symbolic link is never followed (i.e. the ".." + // components are normalised lexically), but the result must be the same. + + const expectedIssues = [ + [ + loc.Issue.Type.WARNING, + "COMBINE archive: the file '../sibling.cellml' is not located within the COMBINE archive. It has been ignored." + ], + [ + loc.Issue.Type.WARNING, + "COMBINE archive: the file '%2E%2E/encoded_sibling.cellml' is not located within the COMBINE archive. It has been ignored." + ], + [ + loc.Issue.Type.WARNING, + "COMBINE archive: the file '../entries_outside_of_archive.omex' is not located within the COMBINE archive. It has been ignored." + ] + ]; + const dir = path.join(os.tmpdir(), 'libopencor_combine_archive_with_files_outside_of_it_through_symbolic_link'); + + fs.rmSync(dir, { recursive: true, force: true }); + fs.mkdirSync(path.join(dir, 'real'), { recursive: true }); + fs.mkdirSync(path.join(dir, 'cwd'), { recursive: true }); + fs.copyFileSync( + utils.resourcePath('api/file/entries_outside_of_archive.omex'), + path.join(dir, 'cwd/entries_outside_of_archive.omex') + ); + fs.symlinkSync('../real', path.join(dir, 'cwd/entries_outside_of_archive.omex.contents')); + + const origDir = process.cwd(); + + process.chdir(path.join(dir, 'cwd')); + + const file = new loc.File('entries_outside_of_archive.omex'); + + file.setContents(utils.fileContents(file.path)); + + process.chdir(origDir); + + assert.strictEqual(file.type.value, loc.File.Type.COMBINE_ARCHIVE.value); + assert.strictEqual(file.childFileCount, 1); + assert.notStrictEqual(file.childFileFromFileName('model.cellml'), null); + utils.assertIssues(loc, file, expectedIssues); + assert.strictEqual(loc.FileManager.instance().fileCount, 2); + + fs.rmSync(dir, { recursive: true, force: true }); + }); + test('COMBINE archive with percent-encoded locations', () => { // A location is a URI reference, so it may be percent-encoded (e.g., "./my%20file.txt"), in which case the name of // its ZIP entry should be its decoded location (e.g., "my file.txt"), although some tools use the location itself diff --git a/tests/bindings/javascript/sed.basic.test.js b/tests/bindings/javascript/sed.basic.test.js index 008646cdf..3e137e546 100644 --- a/tests/bindings/javascript/sed.basic.test.js +++ b/tests/bindings/javascript/sed.basic.test.js @@ -15,6 +15,9 @@ limitations under the License. */ import assert from 'node:assert'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; import test from 'node:test'; import libOpenCOR from './libopencor.js'; @@ -152,6 +155,41 @@ test.describe('Sed basic tests', () => { assert.strictEqual(document.models[0].file.path, utils.resourcePath('api/sed/cellml_2.cellml')); }); + test('SED-ML file with relative CellML file through symbolic link', () => { + // 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: our JavaScript bindings use a virtual file system, so a symbolic link is never followed (i.e. the model + // source is normalised lexically, so "link/../model.cellml" becomes "model.cellml"), and they never retrieve + // the contents of a file (i.e. the type of the model is unknown). + + const dir = path.join(os.tmpdir(), 'libopencor_sedml_file_with_relative_cellml_file_through_symbolic_link'); + + fs.rmSync(dir, { recursive: true, force: true }); + fs.mkdirSync(path.join(dir, 'real/sub'), { recursive: true }); + fs.mkdirSync(path.join(dir, 'cwd'), { recursive: true }); + fs.copyFileSync(utils.resourcePath('cellml_2.cellml'), path.join(dir, 'real/model.cellml')); + fs.symlinkSync('../real/sub', path.join(dir, 'cwd/link')); + + const origDir = process.cwd(); + + process.chdir(path.join(dir, 'cwd')); + + const file = new loc.File('link/simulation.sedml'); + + file.setContents(sedmlContents('../model.cellml')); + + const document = new loc.SedDocument(file); + + process.chdir(origDir); + + assert.strictEqual(document.models.length, 1); + assert.strictEqual(document.models[0].file.path, 'model.cellml'); + assert.strictEqual(document.models[0].file.type.value, loc.File.Type.UNKNOWN_FILE.value); + + fs.rmSync(dir, { recursive: true, force: true }); + }); + test('SED-ML file with unsupported scheme CellML file', () => { // A model source with a scheme other than "http", "https", or "file" is an absolute URI that we cannot retrieve, so // its model is ignored rather than considered to be relative to the SED-ML file. No task is then created for the diff --git a/tests/bindings/javascript/sed.serialise.test.js b/tests/bindings/javascript/sed.serialise.test.js index 6e7dcabfe..4b3760d0d 100644 --- a/tests/bindings/javascript/sed.serialise.test.js +++ b/tests/bindings/javascript/sed.serialise.test.js @@ -139,6 +139,33 @@ test.describe('Sed serialise tests', () => { assert.strictEqual(document.serialise(), cvodeExpectedSerialisation('cellml_2.cellml')); }); + test('Relative local CellML file in directory without base path', () => { + // A relative file name is serialised as a relative model source, so its directory separators must be forward + // slashes (i.e. not backslashes on Windows). + + const file = new loc.File('some/path/cellml_2.cellml'); + + file.setContents(utils.fileContents(utils.resourcePath('cellml_2.cellml'))); + + const document = new loc.SedDocument(file); + + assert.strictEqual(document.serialise(), cvodeExpectedSerialisation('some/path/cellml_2.cellml')); + + // Load the SED-ML file back. + + const sedmlFile = new loc.File('my_model.sedml'); + + sedmlFile.setContents(new TextEncoder().encode(document.serialise())); + + const sedmlDocument = new loc.SedDocument(sedmlFile); + + assert.strictEqual(sedmlDocument.hasIssues, false); + assert.strictEqual(sedmlDocument.models.length, 1); + assert.strictEqual(sedmlDocument.models[0].file.isAliasOf(file), true); + + loc.FileManager.instance().unmanage(sedmlFile); + }); + test('Local CellML file with percent signs', () => { // A model source is a URI reference, so the percent signs in the file name of a local model must be percent-encoded // for the model source to refer to that model when the SED-ML file is loaded back. @@ -244,6 +271,34 @@ test.describe('Sed serialise tests', () => { } }); + test('Relative local CellML file with colon', () => { + // A relative file name is serialised as a relative model source (even without a base path), so it must also be + // prefixed with "./" if its first segment contains a colon. + + const cellmlContents = utils.fileContents(utils.resourcePath('cellml_2.cellml')); + const file = new loc.File('my:model.cellml'); + + file.setContents(cellmlContents); + + const document = new loc.SedDocument(file); + + assert.strictEqual(document.serialise(), cvodeExpectedSerialisation('./my:model.cellml')); + + // Load the SED-ML file back. + + const sedmlFile = new loc.File('my_model.sedml'); + + sedmlFile.setContents(new TextEncoder().encode(document.serialise())); + + const sedmlDocument = new loc.SedDocument(sedmlFile); + + assert.strictEqual(sedmlDocument.hasIssues, false); + assert.strictEqual(sedmlDocument.models.length, 1); + assert.strictEqual(sedmlDocument.models[0].file.isAliasOf(file), true); + + loc.FileManager.instance().unmanage(sedmlFile); + }); + test('Local CellML file with non-UTF-8 character', () => { // A model source is an XML attribute, so the bytes in the file name of a local model that are not valid UTF-8 must // be percent-encoded for the SED-ML file to be valid and for the model source to refer to that model when the diff --git a/tests/bindings/python/test_file_basic.py b/tests/bindings/python/test_file_basic.py index 0eca667d0..a09798dbf 100644 --- a/tests/bindings/python/test_file_basic.py +++ b/tests/bindings/python/test_file_basic.py @@ -48,7 +48,8 @@ def test_local_file(): def test_existing_relative_local_file(): - # 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. orig_dir = os.getcwd() @@ -56,11 +57,13 @@ def test_existing_relative_local_file(): file = loc.File("cellml_2.cellml") + os.chdir(orig_dir) + + assert file.file_name == "cellml_2.cellml" + assert file.path == "cellml_2.cellml" assert file.contents != [] assert_issues(file, expected_no_issues) - os.chdir(orig_dir) - def test_non_existing_relative_local_file(): if platform.system() == "Windows": diff --git a/tests/bindings/python/test_file_child.py b/tests/bindings/python/test_file_child.py index 6bafcd7c0..501e0fec9 100644 --- a/tests/bindings/python/test_file_child.py +++ b/tests/bindings/python/test_file_child.py @@ -15,6 +15,10 @@ from libopencor import File, FileManager, Issue import os +import platform +import pytest +import shutil +import tempfile import utils @@ -171,6 +175,66 @@ def test_combine_archive_with_files_outside_of_it(): assert FileManager.instance().file_count == 3 +@pytest.mark.skipif( + platform.system() == "Windows", + reason="A symbolic link usually requires some special privileges on Windows.", +) +def test_combine_archive_with_files_outside_of_it_through_symbolic_link(): + # 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. + + expected_issues = [ + [ + Issue.Type.Warning, + "COMBINE archive: the file '../sibling.cellml' is not located within the COMBINE archive. It has been ignored.", + ], + [ + Issue.Type.Warning, + "COMBINE archive: the file '%2E%2E/encoded_sibling.cellml' is not located within the COMBINE archive. It has been ignored.", + ], + [ + Issue.Type.Warning, + "COMBINE archive: the file '../entries_outside_of_archive.omex' is not located within the COMBINE archive. It has been ignored.", + ], + ] + + dir_path = os.path.join( + tempfile.gettempdir(), + "libopencor_combine_archive_with_files_outside_of_it_through_symbolic_link", + ) + + shutil.rmtree(dir_path, ignore_errors=True) + os.makedirs(os.path.join(dir_path, "real")) + os.makedirs(os.path.join(dir_path, "cwd")) + shutil.copyfile( + utils.resource_path("api/file/entries_outside_of_archive.omex"), + os.path.join(dir_path, "cwd", "entries_outside_of_archive.omex"), + ) + os.symlink( + "../real", + os.path.join(dir_path, "cwd", "entries_outside_of_archive.omex.contents"), + ) + + orig_dir = os.getcwd() + + os.chdir(os.path.join(dir_path, "cwd")) + + file = File("entries_outside_of_archive.omex") + + os.chdir(orig_dir) + + assert file.type == File.Type.CombineArchive + assert file.child_file_count == 1 + assert file.child_file("model.cellml") is not None + utils.assert_issues(file, expected_issues) + assert FileManager.instance().file_count == 2 + + shutil.rmtree(dir_path) + + def test_combine_archive_with_percent_encoded_locations(): # A location is a URI reference, so it may be percent-encoded (e.g., "./my%20file.txt"), in which case the name of # its ZIP entry should be its decoded location (e.g., "my file.txt"), although some tools use the location itself diff --git a/tests/bindings/python/test_sed_basic.py b/tests/bindings/python/test_sed_basic.py index d3b3c5f33..b27a7dda2 100644 --- a/tests/bindings/python/test_sed_basic.py +++ b/tests/bindings/python/test_sed_basic.py @@ -16,6 +16,9 @@ import libopencor as loc import os import platform +import pytest +import shutil +import tempfile import utils from utils import assert_issues @@ -141,6 +144,49 @@ def test_sedml_file_with_relative_cellml_file_in_working_directory(): ) +@pytest.mark.skipif( + platform.system() == "Windows", + reason="A symbolic link usually requires some special privileges on Windows.", +) +def test_sedml_file_with_relative_cellml_file_through_symbolic_link(): + # 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. + + dir_path = os.path.join( + tempfile.gettempdir(), + "libopencor_sedml_file_with_relative_cellml_file_through_symbolic_link", + ) + + shutil.rmtree(dir_path, ignore_errors=True) + os.makedirs(os.path.join(dir_path, "real", "sub")) + os.makedirs(os.path.join(dir_path, "cwd")) + shutil.copyfile( + utils.resource_path("cellml_2.cellml"), + os.path.join(dir_path, "real", "model.cellml"), + ) + os.symlink("../real/sub", os.path.join(dir_path, "cwd", "link")) + + orig_dir = os.getcwd() + + os.chdir(os.path.join(dir_path, "cwd")) + + file = loc.File("link/simulation.sedml", False) + + file.contents = utils.text_to_list(sedml_contents("../model.cellml")) + + document = loc.SedDocument(file) + + os.chdir(orig_dir) + + assert len(document.models) == 1 + assert document.models[0].file.path == "link/../model.cellml" + assert document.models[0].file.type == loc.File.Type.CellmlFile + + shutil.rmtree(dir_path) + + def test_sedml_file_with_unsupported_scheme_cellml_file(): # A model source with a scheme other than "http", "https", or "file" is an absolute URI that we cannot retrieve, so # its model is ignored rather than considered to be relative to the SED-ML file. No task is then created for the diff --git a/tests/bindings/python/test_sed_coverage.py b/tests/bindings/python/test_sed_coverage.py index 2c34b27ba..3f0ee91b9 100644 --- a/tests/bindings/python/test_sed_coverage.py +++ b/tests/bindings/python/test_sed_coverage.py @@ -417,10 +417,11 @@ def test_sed_base(): ]: assert not loc.SedBase.is_valid_id(id) - with pytest.raises(ValueError) as exception_info: + with pytest.raises( + ValueError, match=r"^The id is not a valid SED-ML identifier\.$" + ): simulation.id = id - assert str(exception_info.value) == "The id is not a valid SED-ML identifier." assert simulation.id == "my_simulation" # A string with a lone surrogate that doesn't stand for an invalid byte (i.e. other than U+DC80 to U+DCFF) is @@ -469,10 +470,9 @@ def test_sed_ids(): assert loc.SedBase.is_valid_id(id) assert other_model.is_duplicate_id(id) - with pytest.raises(ValueError) as exception_info: + with pytest.raises(ValueError, match=r"^The id is already used\.$"): other_model.id = id - assert str(exception_info.value) == "The id is already used." assert other_model.id == "model1" assert not other_model.is_duplicate_id("model1") @@ -491,10 +491,9 @@ def test_sed_ids(): assert model.is_duplicate_id("model2") - with pytest.raises(ValueError) as exception_info: + with pytest.raises(ValueError, match=r"^The id is already used\.$"): model.id = "model2" - assert str(exception_info.value) == "The id is already used." assert not document.add_model(model) model.id = "my_model" @@ -502,11 +501,9 @@ def test_sed_ids(): assert document.add_model(model) assert other_model.is_duplicate_id("my_model") - with pytest.raises(ValueError) as exception_info: + with pytest.raises(ValueError, match=r"^The id is already used\.$"): other_model.id = "my_model" - assert str(exception_info.value) == "The id is already used." - assert len(document.models) == 1 assert document.models[0] == model diff --git a/tests/bindings/python/test_sed_serialise.py b/tests/bindings/python/test_sed_serialise.py index 571f7497c..02d87e12b 100644 --- a/tests/bindings/python/test_sed_serialise.py +++ b/tests/bindings/python/test_sed_serialise.py @@ -155,6 +155,37 @@ def test_relative_local_cellml_file_without_base_path(): assert document.serialise() == cvode_expected_serialisation("cellml_2.cellml") +def test_relative_local_cellml_file_in_directory_without_base_path(): + # A relative file name is serialised as a relative model source, so its directory separators must be forward + # slashes (i.e. not backslashes on Windows). + + file = loc.File("some/path/cellml_2.cellml", False) + + file.contents = utils.text_to_list( + utils.text_file_contents(utils.resource_path("cellml_2.cellml")) + ) + + document = loc.SedDocument(file) + + assert document.serialise() == cvode_expected_serialisation( + "some/path/cellml_2.cellml" + ) + + # Load the SED-ML file back. + + sedml_file = loc.File("my_model.sedml", False) + + sedml_file.contents = utils.text_to_list(document.serialise()) + + sedml_document = loc.SedDocument(sedml_file) + + assert not sedml_document.has_issues + assert len(sedml_document.models) == 1 + assert sedml_document.models[0].file == file + + loc.FileManager.instance().unmanage(sedml_file) + + def test_local_cellml_file_with_percent_signs(): # A model source is a URI reference, so the percent signs in the file name of a local model must be percent-encoded # for the model source to refer to that model when the SED-ML file is loaded back. @@ -294,6 +325,41 @@ def test_local_cellml_file_with_colon(): loc.FileManager.instance().unmanage(sedml_file) +@pytest.mark.skipif( + platform.system() == "Windows", + reason="A file name cannot contain a colon on Windows.", +) +def test_relative_local_cellml_file_with_colon(): + # A relative file name is serialised as a relative model source (even without a base path), so it must also be + # prefixed with "./" if its first segment contains a colon. + # Note: a file name cannot contain a colon on Windows. + + cellml_contents = utils.text_to_list( + utils.text_file_contents(utils.resource_path("cellml_2.cellml")) + ) + file = loc.File("my:model.cellml", False) + + file.contents = cellml_contents + + document = loc.SedDocument(file) + + assert document.serialise() == cvode_expected_serialisation("./my:model.cellml") + + # Load the SED-ML file back. + + sedml_file = loc.File("my_model.sedml", False) + + sedml_file.contents = utils.text_to_list(document.serialise()) + + sedml_document = loc.SedDocument(sedml_file) + + assert not sedml_document.has_issues + assert len(sedml_document.models) == 1 + assert sedml_document.models[0].file == file + + loc.FileManager.instance().unmanage(sedml_file) + + def test_local_cellml_file_with_non_utf8_character(): # A model source is an XML attribute, so the bytes in the file name of a local model that are not valid UTF-8 must # be percent-encoded for the SED-ML file to be valid and for the model source to refer to that model when the SED-ML diff --git a/tests/misc/utils.cpp b/tests/misc/utils.cpp index cf46785fd..107a03628 100644 --- a/tests/misc/utils.cpp +++ b/tests/misc/utils.cpp @@ -20,6 +20,7 @@ limitations under the License. #include #include +#include #include #ifdef NAN @@ -264,7 +265,7 @@ TEST(UtilsTest, retrieveFileInfo) std::tie(isLocalFile, fileNameOrUrl) = libOpenCOR::retrieveFileInfo("https://EXAMPLE.com\\A"); EXPECT_FALSE(isLocalFile); -#ifdef BUILDING_USING_MSVC +#ifdef BUILDING_ON_WINDOWS EXPECT_EQ(fileNameOrUrl, "https://example.com/A"); #else EXPECT_EQ(fileNameOrUrl, "https://example.com\\A"); @@ -283,7 +284,7 @@ TEST(UtilsTest, stringToPathAndPathToString) EXPECT_EQ(libOpenCOR::pathToString(libOpenCOR::stringToPath(fileName)), fileName); } -#ifdef BUILDING_USING_MSVC +#ifdef BUILDING_ON_WINDOWS // On Windows, a path may contain lone surrogates, which can be converted to a file name and back without being // altered. @@ -595,6 +596,51 @@ TEST(UtilsTest, canonicalFileName) EXPECT_EQ(libOpenCOR::canonicalFileName(std::string(libOpenCOR::RESOURCE_LOCATION) + "/api/../cellml_2.cellml"), libOpenCOR::resourcePath("cellml_2.cellml")); + // Existing relative file, which must be normalised lexically (i.e. without using the current working directory). + + auto origDir {std::filesystem::current_path()}; + + std::filesystem::current_path(libOpenCOR::resourcePath("api")); + +#ifdef BUILDING_ON_WINDOWS + EXPECT_EQ(libOpenCOR::canonicalFileName("../api/./../cellml_2.cellml"), R"(..\cellml_2.cellml)"); +#else + EXPECT_EQ(libOpenCOR::canonicalFileName("../api/./../cellml_2.cellml"), "../cellml_2.cellml"); +#endif + + std::filesystem::current_path(origDir); + +#ifndef BUILDING_ON_WINDOWS + // Relative files with a symbolic link followed by a ".." component, which must be kept since the ".." component + // then refers to the parent directory of the target of the symbolic link (i.e. "real" rather than "cwd"). + // Note: a symbolic link usually requires some special privileges on Windows. + + auto dir {std::filesystem::temp_directory_path() / "libopencor_canonical_file_name_with_symbolic_link"}; + + std::filesystem::remove_all(dir); + std::filesystem::create_directories(dir / "real/sub/inner"); + std::filesystem::create_directories(dir / "cwd"); + std::filesystem::create_directory_symlink("../real/sub", dir / "cwd/link"); + + std::filesystem::current_path(dir / "cwd"); + + EXPECT_EQ(libOpenCOR::canonicalFileName("link/../file.txt"), "link/../file.txt"); + EXPECT_EQ(libOpenCOR::canonicalFileName("link/./../file.txt"), "link/../file.txt"); + EXPECT_EQ(libOpenCOR::canonicalFileName("non_existing/../link/../file.txt"), "link/../file.txt"); + EXPECT_EQ(libOpenCOR::canonicalFileName("../cwd/link/../file.txt"), "../cwd/link/../file.txt"); + EXPECT_EQ(libOpenCOR::canonicalFileName("link/../../file.txt"), "link/../../file.txt"); + EXPECT_EQ(libOpenCOR::canonicalFileName("link/../dir/"), "link/../dir/"); + EXPECT_EQ(libOpenCOR::canonicalFileName("link/../dir/."), "link/../dir/"); + EXPECT_EQ(libOpenCOR::canonicalFileName("link/../dir/.."), "link/.."); + + // A ".." component that doesn't follow a symbolic link is still normalised lexically. + + EXPECT_EQ(libOpenCOR::canonicalFileName("link/inner/../file.txt"), "link/file.txt"); + + std::filesystem::current_path(origDir); + std::filesystem::remove_all(dir); +#endif + // Non-existing absolute file. EXPECT_EQ(libOpenCOR::canonicalFileName(std::string(libOpenCOR::RESOURCE_LOCATION) + "/non/existing/../../file.txt"), @@ -650,7 +696,7 @@ TEST(UtilsTest, canonicalUrl) EXPECT_EQ(libOpenCOR::canonicalUrl("https://example.com#a/../b"), "https://example.com#a/../b"); EXPECT_EQ(libOpenCOR::canonicalUrl("http://user@example.com:8080/a/../model.cellml"), "http://user@example.com:8080/model.cellml"); EXPECT_EQ(libOpenCOR::canonicalUrl("example.com/a/../model.cellml"), "example.com/model.cellml"); -#ifdef BUILDING_USING_MSVC +#ifdef BUILDING_ON_WINDOWS EXPECT_EQ(libOpenCOR::canonicalUrl(R"(https://example.com\a\..\model.cellml)"), "https://example.com/model.cellml"); #endif }