Builder::add_ingredient may fail with non-ascii characters in source_path

[fwanderse]

Describe the bug
Builder::add_ingredient fails with non-ascii characters in source_path on Windows if the current character encoding is not utf-8.

An C2paException exception will occur with message: "Failed to open file: "

There is a conversion to utf8 string called path_to_string(...) in open_file_binary(...) that causes this. We believe this conversion should be removed. Sending std::filesystem::path directly into the constructor of the stream would handle character encoding/conversion correctly.

Please see suggested fix on our fork:
https://github.com/contentauth/c2pa-c/compare/vNext...fwanderse:c2pa-c:fw-anderse/fix-utf8-convert-issue-in-open_file_binary?diff=split&w

To Reproduce
Use an ingredient filename like for example: image_name-using-øæå.jpg.

Attachments
Can add on request, but do not think it is needed to explain this issue.

Expected behavior
No error opening ingredient file.

Desktop (please complete the following information):

• Windows 10
  Using vNext branch latest
• Using Microsoft Visual C++. C++20.
• N/A

Additional context
Note: When opening the same file using the Reader a similar conversion of the file path is not applied and an error like this does not occur.

[tmathern]

Why not opening a pull request with the suggested fix? Since it seems you have one?

[fwanderse]

Ok, I created a pull request: #160

[tmathern]

Thank you, will review in the next few days!

[tmathern]

(Would you mind signing the CLA in the PR? The PR blocks on that currently)

[tmathern]

Alternative suggestion: #160 (comment)
Let me know what you think.

[fwanderse]

• Your PR fix: Builder::add_ingredient may fail with non-ascii characters in source_path #161 looks good and better to me. It also includes tests.
• I would probably have preferred keeping the function over inline implementation, and maybe renamed it path_to_utf8_string and made a comment something like "Use only when utf-8 strings are always expected, e.g. not with StreamType which expects a path in the current string encoding (which may not be utf8)"
• Feel free to reject my PR #160 if you go forward with #161

[tmathern]

Let me add another member of the team as reviewer.
Regarding inlining, I made that call because the other locations are in deprecated functions that, one day, we will remove. So we'd have 1 function called at one place (after the deprecations are removed)... Which I believe justifies inlining it at the place where it is used.