Encode FilePath as a string or an array of code units

[kateinoigakukun]

This changes the FilePath's Codable conformance to encode and decode FilePath as a string or an array of code units instead of using the default synthesized implementation that encodes the storage directly.

Decoding data as internal storage is not user-friendly and allows to construct invalid FilePath instances (e.g. with non-null-terminated storage).

So, the new implementation tries to decode the input as a string first and then as an array of code units. If the array is not null-terminated, the decoding fails.

Close #106

[kateinoigakukun]

@swift-ci test

[kateinoigakukun]

Do we need to support decoding the old synthesized format? In that case, we continue exposing a way to construct an invalid FilePath, so I didn't include it in this PR. WDYT?

[al45tair]

IMO the Codable conformance on FilePath was/is a mistake. The implementation is different for different platforms, and you can't sensibly decode a UNIX path on Windows or vice-versa.

It's also likely someone is already depending on the existing implementation (which is buggy in a way that could be a security issue), so I think while what you've done here is an improvement in many ways, it probably also needs to support the existing encoding — though we should check it carefully to make sure we don't allow the storage to be non-NUL-terminated.

[kateinoigakukun]

@al45tair

IMO the Codable conformance on FilePath was/is a mistake. The implementation is different for different platforms, and you can't sensibly decode a UNIX path on Windows or vice-versa.

Yeah, it might become another security concern. I took a look at how other language ecosystems handle it:

• Rust (serde): They add a tag to distinguish whether a file path is serialized on Unix-like platform or Windows. And deserialization fails if the source platform and the current platform are different.
  https://github.com/serde-rs/serde/blob/7aafa263146f9140b18879c114a69fe96b2ebffc/serde/src/ser/impls.rs#L1007
  https://github.com/serde-rs/serde/blob/7aafa263146f9140b18879c114a69fe96b2ebffc/serde/src/de/impls.rs#L1943
• LLVM (Support/JSON.h): All paths (StringRef) serialized into JSON must be valid as UTF-8 byte sequence.
• Ruby (pathname with json): Pathname responds to to_json but throw exception if the given pathname cannot be transcoded into UTF-8
• Python (pathlib with json): pathlib.Path is not JSON-serializable.

Rust (serde) approach addresses the wrong use case you mentioned where serialization and deserialization happened on different platforms. I think it's worth doing a similar tagging when (de)serializing paths as code points.

it probably also needs to support the existing encoding — though we should check it carefully to make sure we don't allow the storage to be non-NUL-terminated.

Ok, it makes sense to me to accept the existing encoding only if the storage is valid 👍

[kateinoigakukun]

Closing in favor of #189