From ab129026cbbf95393cd1d13a6ead210916724b03 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Thu, 15 Oct 2026 20:28:33 +1300 Subject: [PATCH 1/9] New version. --- VERSION.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 09983327a32b66dd5e47a19c9141babc5a965af7 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Thu, 15 Oct 2026 20:27:56 +1300 Subject: [PATCH 2/9] Preserve relative file paths. --- src/misc/utils.cpp | 30 +++++++++++--------- src/support/sedml/sedmlfile.cpp | 6 ++-- tests/api/file/basictests.cpp | 5 +++- tests/bindings/javascript/file.basic.test.js | 19 +++++++++++++ tests/bindings/python/test_file_basic.py | 5 +++- tests/misc/utils.cpp | 14 +++++++++ 6 files changed, 60 insertions(+), 19 deletions(-) diff --git a/src/misc/utils.cpp b/src/misc/utils.cpp index 96a9a080f..c7e73c661 100644 --- a/src/misc/utils.cpp +++ b/src/misc/utils.cpp @@ -625,21 +625,22 @@ 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) { @@ -649,8 +650,9 @@ std::string canonicalFileName(const std::string &pFileName) #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"). + // 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()); } 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..ad7ce8dd5 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,6 +72,8 @@ TEST(BasicFileTest, existingRelativeLocalFile) auto file {libOpenCOR::File::create("cellml_2.cellml")}; + EXPECT_EQ(file->fileName(), "cellml_2.cellml"); + EXPECT_EQ(file->path(), "cellml_2.cellml"); EXPECT_FALSE(file->contents().empty()); EXPECT_EQ_ISSUES(file, expectedNoIssues()); diff --git a/tests/bindings/javascript/file.basic.test.js b/tests/bindings/javascript/file.basic.test.js index 75600aad6..460437e9e 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'); + + assert.strictEqual(file.fileName, 'cellml_2.cellml'); + assert.strictEqual(file.path, 'cellml_2.cellml'); + assert.deepStrictEqual(file.contents(), Uint8Array.from([])); + assertIssues(loc, file, expectedNoIssues); + + process.chdir(origDir); + }); + 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/python/test_file_basic.py b/tests/bindings/python/test_file_basic.py index 0eca667d0..276eae63d 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,6 +57,8 @@ def test_existing_relative_local_file(): file = loc.File("cellml_2.cellml") + assert file.file_name == "cellml_2.cellml" + assert file.path == "cellml_2.cellml" assert file.contents != [] assert_issues(file, expected_no_issues) diff --git a/tests/misc/utils.cpp b/tests/misc/utils.cpp index cf46785fd..8d6e2169d 100644 --- a/tests/misc/utils.cpp +++ b/tests/misc/utils.cpp @@ -595,6 +595,20 @@ 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); + // Non-existing absolute file. EXPECT_EQ(libOpenCOR::canonicalFileName(std::string(libOpenCOR::RESOURCE_LOCATION) + "/non/existing/../../file.txt"), From 9650d7e26ef8653113b1816379da421a1526ca4f Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Thu, 15 Oct 2026 20:36:19 +1300 Subject: [PATCH 3/9] Restored the current working directory before file assertions in our tests. --- tests/api/file/basictests.cpp | 4 ++-- tests/bindings/javascript/file.basic.test.js | 4 ++-- tests/bindings/python/test_file_basic.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/api/file/basictests.cpp b/tests/api/file/basictests.cpp index ad7ce8dd5..9cc65ea18 100644 --- a/tests/api/file/basictests.cpp +++ b/tests/api/file/basictests.cpp @@ -72,12 +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/bindings/javascript/file.basic.test.js b/tests/bindings/javascript/file.basic.test.js index 460437e9e..b9b29a138 100644 --- a/tests/bindings/javascript/file.basic.test.js +++ b/tests/bindings/javascript/file.basic.test.js @@ -62,12 +62,12 @@ test.describe('File basic tests', () => { 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); - - process.chdir(origDir); }); test('Non-existing relative local file', () => { diff --git a/tests/bindings/python/test_file_basic.py b/tests/bindings/python/test_file_basic.py index 276eae63d..a09798dbf 100644 --- a/tests/bindings/python/test_file_basic.py +++ b/tests/bindings/python/test_file_basic.py @@ -57,13 +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": From e2959ba75cbf4fc101294b2c66dc7c2807e8340e Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Thu, 15 Oct 2026 22:24:20 +1300 Subject: [PATCH 4/9] Tightened Python SED-ML ID validation tests. --- tests/bindings/python/test_sed_coverage.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) 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 From 44797f82bb9a2f56418cbb9233f03060354e3912 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Thu, 15 Oct 2026 23:00:06 +1300 Subject: [PATCH 5/9] Prefix relative colon paths in SED-ML source. --- src/sed/sedmodel.cpp | 8 +++-- tests/api/sed/serialisetests.cpp | 30 ++++++++++++++++ .../bindings/javascript/sed.serialise.test.js | 28 +++++++++++++++ tests/bindings/python/test_sed_serialise.py | 35 +++++++++++++++++++ 4 files changed, 98 insertions(+), 3 deletions(-) diff --git a/src/sed/sedmodel.cpp b/src/sed/sedmodel.cpp index 1ed591201..725fb1dd5 100644 --- a/src/sed/sedmodel.cpp +++ b/src/sed/sedmodel.cpp @@ -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/tests/api/sed/serialisetests.cpp b/tests/api/sed/serialisetests.cpp index 209942245..c65245a03 100644 --- a/tests/api/sed/serialisetests.cpp +++ b/tests/api/sed/serialisetests.cpp @@ -267,6 +267,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/sed.serialise.test.js b/tests/bindings/javascript/sed.serialise.test.js index 6e7dcabfe..448a15357 100644 --- a/tests/bindings/javascript/sed.serialise.test.js +++ b/tests/bindings/javascript/sed.serialise.test.js @@ -244,6 +244,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_sed_serialise.py b/tests/bindings/python/test_sed_serialise.py index 571f7497c..132af7c7b 100644 --- a/tests/bindings/python/test_sed_serialise.py +++ b/tests/bindings/python/test_sed_serialise.py @@ -294,6 +294,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 From 9b15bceac3990233b51724eaf7dac222b8d24cae Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Thu, 15 Oct 2026 23:35:39 +1300 Subject: [PATCH 6/9] Handle symlink '..' in canonical paths. --- src/misc/utils.cpp | 55 +++++++++++++++-- src/support/combine/combinearchive.cpp | 17 +++++- tests/api/file/childtests.cpp | 42 +++++++++++++ tests/api/sed/basictests.cpp | 36 +++++++++++ tests/bindings/javascript/file.child.test.js | 55 +++++++++++++++++ tests/bindings/javascript/sed.basic.test.js | 38 ++++++++++++ tests/bindings/python/test_file_child.py | 64 ++++++++++++++++++++ tests/bindings/python/test_sed_basic.py | 46 ++++++++++++++ tests/misc/utils.cpp | 32 ++++++++++ 9 files changed, 380 insertions(+), 5 deletions(-) diff --git a/src/misc/utils.cpp b/src/misc/utils.cpp index c7e73c661..2e492f80c 100644 --- a/src/misc/utils.cpp +++ b/src/misc/utils.cpp @@ -644,15 +644,62 @@ std::string canonicalFileName(const std::string &pFileName) 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::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"). + // 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()); } diff --git a/src/support/combine/combinearchive.cpp b/src/support/combine/combinearchive.cpp index d32492d19..4b95aa686 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::none_of(filePathWithinArchive.begin(), filePathWithinArchive.end(), [](const auto &pComponent) { + return pComponent == ".."; + }); + } + + if (!isLocatedWithinArchive) { std::string warning; warning.reserve(location.size() + 64); // NOLINT 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/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/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/misc/utils.cpp b/tests/misc/utils.cpp index 8d6e2169d..044212634 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 @@ -609,6 +610,37 @@ TEST(UtilsTest, canonicalFileName) 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"), From 9d27d20c09a29563f952600c25f7392f0cd2d945 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Thu, 15 Oct 2026 23:53:44 +1300 Subject: [PATCH 7/9] Use ranges::none_of() in the archive path check. --- src/support/combine/combinearchive.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/support/combine/combinearchive.cpp b/src/support/combine/combinearchive.cpp index 4b95aa686..2dad14d84 100644 --- a/src/support/combine/combinearchive.cpp +++ b/src/support/combine/combinearchive.cpp @@ -102,7 +102,7 @@ CombineArchive::Impl::Impl(const FilePtr &pFile, libcombine::CombineArchive *pAr if (isLocatedWithinArchive) { auto filePathWithinArchive {stringToPath(fileName.substr(mArchiveLocationSize))}; - isLocatedWithinArchive = std::none_of(filePathWithinArchive.begin(), filePathWithinArchive.end(), [](const auto &pComponent) { + isLocatedWithinArchive = std::ranges::none_of(filePathWithinArchive, [](const auto &pComponent) { return pComponent == ".."; }); } From b1f384b860e1a40e77d20f0c731bec8ec89c2d54 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Fri, 16 Oct 2026 00:21:34 +1300 Subject: [PATCH 8/9] Normalise relative URI paths on MSVC. --- src/misc/utils.cpp | 9 ++++++ tests/api/sed/serialisetests.cpp | 28 +++++++++++++++++ .../bindings/javascript/sed.serialise.test.js | 27 ++++++++++++++++ tests/bindings/python/test_sed_serialise.py | 31 +++++++++++++++++++ 4 files changed, 95 insertions(+) diff --git a/src/misc/utils.cpp b/src/misc/utils.cpp index 2e492f80c..988a940b9 100644 --- a/src/misc/utils.cpp +++ b/src/misc/utils.cpp @@ -949,6 +949,15 @@ std::string urlPath(const std::string &pPath) #endif } +#ifdef BUILDING_USING_MSVC + // 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/tests/api/sed/serialisetests.cpp b/tests/api/sed/serialisetests.cpp index c65245a03..82e2739da 100644 --- a/tests/api/sed/serialisetests.cpp +++ b/tests/api/sed/serialisetests.cpp @@ -148,6 +148,34 @@ TEST(SerialiseSedTest, relativeLocalCellmlFileWithoutBasePath) EXPECT_EQ(document->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 diff --git a/tests/bindings/javascript/sed.serialise.test.js b/tests/bindings/javascript/sed.serialise.test.js index 448a15357..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. diff --git a/tests/bindings/python/test_sed_serialise.py b/tests/bindings/python/test_sed_serialise.py index 132af7c7b..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. From 9b3e07c44f5abce7569dca434c8017fa5c2f00b2 Mon Sep 17 00:00:00 2001 From: Alan Garny Date: Fri, 16 Oct 2026 00:35:22 +1300 Subject: [PATCH 9/9] Use platform checks for Windows/Apple logic. --- cmake/common.cmake | 2 +- src/misc/utils.cpp | 14 +++++++------- src/misc/utils.h | 2 +- src/sed/sedmodel.cpp | 2 +- src/support/cellml/cellmlfileruntime.cpp | 2 +- tests/api/sed/coveragetests.cpp | 2 +- tests/misc/utils.cpp | 6 +++--- 7 files changed, 15 insertions(+), 15 deletions(-) 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 988a940b9..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(); @@ -717,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}; @@ -940,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); @@ -949,7 +949,7 @@ std::string urlPath(const std::string &pPath) #endif } -#ifdef BUILDING_USING_MSVC +#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"). 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 725fb1dd5..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)}; 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/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