Fix FMI 2.0 resource location parsing in the Chrono::Vehicle FMUs - #772
Fix FMI 2.0 resource location parsing in the Chrono::Vehicle FMUs#772DanNegrut wants to merge 2 commits into
Conversation
Fixes #762. ## Problem The FMI 2.0 standard specifies that the `fmuResourceLocation` argument of `fmi2Instantiate` is a URI (RFC 3986/8089) pointing to the `resources` directory of the unpacked FMU, not a plain filesystem path. All six Chrono::Vehicle FMUs converted it by removing a fixed-length prefix: ```cpp auto resources_dir = std::string(fmuResourceLocation).erase(0, 8); ``` Chopping 8 characters removes `file:///`. On Windows that is correct, because the drive letter follows the third slash and `file:///C:/temp` becomes `C:/temp`. On POSIX it removes the leading slash of an absolute path, so `file:///tmp/xyz/resources` becomes the relative `tmp/xyz/resources` and the JSON specification files are not found. ## Why this went unnoticed Chrono's own importer does not emit a conforming URI. fmu-forge builds the location in `FmuUnit::Instantiate` as ```cpp "file:///" + m_directory + "/resources" ``` and on POSIX `m_directory` is already absolute, so the result carries four slashes (`file:////tmp/...`). A blind 8-character chop happens to land correctly on that. A standards-conforming importer such as FMPy, Dymola or OpenModelica emits three slashes, and the chop then eats the root slash. The practical effect is that the Chrono vehicle FMUs work on Linux and macOS only with Chrono's own loader, which defeats the purpose of shipping FMI components. ## Change Replace the prefix removal at all six call sites with a shared `chrono::fmi2::GetResourcesPath()`, in a new header with no fmu-forge dependency so it can be included and tested on its own. `ChFmuToolsExport.h` includes it, so the call sites change only in the call itself. The parser handles an empty or `localhost` authority (matched case-insensitively, since host names are), the Windows drive-letter form, the RFC 8089 minimal `file:/path` form, repeated leading slashes so the existing four-slash form keeps working, percent-encoded octets, and a plain path for importers that pass one instead of a URI. Interpretation is deliberately platform-dependent, because file URIs are: the drive-letter rule applies only on Windows, since `/C:/temp` is a legal POSIX path naming a directory called `C:`. A URI naming a remote host is mapped to a UNC path on Windows and returned untouched elsewhere, so it fails visibly rather than being silently redirected to an unrelated local path (on Linux `//host/share` would otherwise resolve as `/host/share`). Any trailing separator is dropped so callers can uniformly append `/<filename>`. FMI 3.0 is unaffected: it passes `resourcePath` as a plain path, and no Chrono FMI 3.0 component parses it. ## Verification New unit test `utest_FMI_resource_path` (9 cases) covering the conforming and Chrono-emitted forms, authority handling, percent decoding including UTF-8 and 0xFF escapes and malformed escapes, plain paths, platform-guarded drive-letter and remote-host behavior, locale independence, and malformed input. Passes on both Windows (clang 19 / MSVC headers) and Linux (gcc 13.3), where 8 pass and the locale case skips itself when no Turkish locale is installed. End-to-end on Linux (Ubuntu 24.04, gcc 13.3), static build with `FMU_EXPORT_SUPPORT=ON`, `FMU2cs_Powertrain` built and loaded through `fmi2Instantiate` with a conforming three-slash URI: ``` WITHOUT this change: Engine JSON: tmp/ch-static/.../resources/EngineShafts.json ERROR: Could not open JSON file: tmp/ch-static/.../resources/EngineShafts.json Segmentation fault (core dumped) WITH this change: Engine JSON: /tmp/ch-static/.../resources/EngineShafts.json RESULT: resources resolved, initialization completed ``` That reproduces the exact symptom reported in #762, including the segfault, and shows it resolved. ## Test coverage notes All six modified translation units compile on Windows, but the Windows FMU targets were not built locally, since FMU export requires a static build with a static MSVC runtime; CI covers that configuration. Radu owns this code, so flagging for his review: cc @rserban Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Follow-up verification. The PR body reports the end-to-end check for Each FMU was exercised by All six resolve their resources with the leading slash intact:
This includes As a control, reverting just the changed line and rebuilding on the same machine reproduces the reported failure exactly, including the crash: Not covered locally, left to CI: the Windows static-FMU configuration (FMU export requires a static build with a static MSVC runtime) and macOS. One further note that is out of scope for this PR but worth recording. The reason this survived so long is that |
|
"This PR keeps the four-slash form working, but the URI construction in fmu-forge is arguably the thing to fix upstream." This is exactly what I was planning on looking into. We own the To summarize, I think the following should be done in this order:
|
|
No argument on ownership, and agreed that a construction fix belongs in The
On Windows the current code already emits a spec-conforming three-slash URI and the old parser already So if step 2 is run on Windows, Chrono::FMI will pass with and without this PR, and the reasonable To be precise about scope: it is the What the old parser does to conforming URIs, measured.
The single-slash rows are worth a look independently of anything the standard does or does not require. Correcting my own earlier framing. In the comment above I implied the reporter's trace identified On ordering. I am not suggesting your sequence breaks anything: the submodule bump is step 3, so |
The comment introduced its whole list of accepted resource-location forms with "per RFC 8089", but two of the entries are not standard file URIs and RFC 8089 does not describe them: the four-slash form and a bare filesystem path. Only a later sentence noted the four-slash form was non-standard, so the list as written implied the specification endorses both. Split into two lists, standard and compatibility, with the non-standard status stated where the forms are introduced rather than afterwards. No behavior change; comments only. Also documents "file:/C:/temp/fmu/resources", the authority-omitted form with a drive letter, which the parser already handled but the list did not mention. All seven documented rows were checked against actual output before committing. Adds one observation to the four-slash note: that form only arises where an absolute path begins with a separator, so the same concatenation in our importer yields the standard three-slash form on Windows, making the four-slash case POSIX-only. That asymmetry is the reason a Windows-only test of this code path passes whether or not the parser is fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Fixes #762.
Problem
The FMI 2.0 standard specifies that the
fmuResourceLocationargument offmi2Instantiateis a URI (RFC 3986/8089) pointing to theresourcesdirectory of the unpacked FMU, not a plain filesystem path. All six Chrono::Vehicle FMUs converted it by removing a fixed-length prefix:Chopping 8 characters removes
file:///. On Windows that is correct, because the drive letter follows the third slash andfile:///C:/tempbecomesC:/temp. On POSIX it removes the leading slash of an absolute path, sofile:///tmp/xyz/resourcesbecomes the relativetmp/xyz/resourcesand the JSON specification files are not found.Why this went unnoticed
Chrono's own importer does not emit a conforming URI. fmu-forge builds the location in
FmuUnit::Instantiateasand on POSIX
m_directoryis already absolute, so the result carries four slashes (file:////tmp/...). A blind 8-character chop happens to land correctly on that. A standards-conforming importer such as FMPy, Dymola or OpenModelica emits three slashes, and the chop then eats the root slash. The practical effect is that the Chrono vehicle FMUs work on Linux and macOS only with Chrono's own loader, which defeats the purpose of shipping FMI components.Change
Replace the prefix removal at all six call sites with a shared
chrono::fmi2::GetResourcesPath(), in a new header with no fmu-forge dependency so it can be included and tested on its own.ChFmuToolsExport.hincludes it, so the call sites change only in the call itself.The parser handles an empty or
localhostauthority (matched case-insensitively, since host names are), the Windows drive-letter form, the RFC 8089 minimalfile:/pathform, repeated leading slashes so the existing four-slash form keeps working, percent-encoded octets, and a plain path for importers that pass one instead of a URI. Interpretation is deliberately platform-dependent, because file URIs are: the drive-letter rule applies only on Windows, since/C:/tempis a legal POSIX path naming a directory calledC:. A URI naming a remote host is mapped to a UNC path on Windows and returned untouched elsewhere, so it fails visibly rather than being silently redirected to an unrelated local path (on Linux//host/sharewould otherwise resolve as/host/share). Any trailing separator is dropped so callers can uniformly append/<filename>.FMI 3.0 is unaffected: it passes
resourcePathas a plain path, and no Chrono FMI 3.0 component parses it.Verification
New unit test
utest_FMI_resource_path(9 cases) covering the conforming and Chrono-emitted forms, authority handling, percent decoding including UTF-8 and 0xFF escapes and malformed escapes, plain paths, platform-guarded drive-letter and remote-host behavior, locale independence, and malformed input. Passes on both Windows (clang 19 / MSVC headers) and Linux (gcc 13.3), where 8 pass and the locale case skips itself when no Turkish locale is installed.End-to-end on Linux (Ubuntu 24.04, gcc 13.3), static build with
FMU_EXPORT_SUPPORT=ON,FMU2cs_Powertrainbuilt and loaded throughfmi2Instantiatewith a conforming three-slash URI:That reproduces the exact symptom reported in #762, including the segfault, and shows it resolved.
Test coverage notes
All six modified translation units compile on Windows, but the Windows FMU targets were not built locally, since FMU export requires a static build with a static MSVC runtime; CI covers that configuration.
Radu owns this code, so flagging for his review: cc @rserban