[WTF] URL::fileSystemPath() returns Markable<String> and decodes strictly on Windows

RO
25d ago
+54-14

This is the follow-up to oven-sh/bun#45354. It needs two things before a version bump can take it: a maintainer's decision on the Windows change below, and a rebase on main (this branch is based on 0c06faadf6).

Problem

  • The non-CF URL::fileSystemPath() returns a null String when the percent-decoded path is not valid UTF-8, for example file:///tmp/50%deposit.db or %E9. URL.cpp has a FIXME that asks if this is OK. A null String reads as "", and "" reads as the current directory.
  • On Windows, decodeEscapeSequencesFromParsedURLForWindowsPath replaces the bad bytes with U+FFFD. The caller gets the path of a different file.

Fix

  • fileSystemPath() returns Markable<String>. It is empty for a URL that is not a file: URL and for a path that does not decode. Markable has no conversion to String, so a caller that does not unwrap does not compile.
  • The Windows decoder uses String::fromUTF8. fileSystemPathWindows() returns empty before it builds a UNC path from a null string.
  • jsc.cpp (10 calls) and SourceProvider.cpp (1 call) keep their behavior with unsafeValue().
  • Verified on Linux x64 (JSCOnly, debug): WTF, jsc.cpp and SourceProvider.cpp build. The new WTF_URL.FileSystemPathIsEmptyWhenThePathIsNotUTF8 test passes, also with the Windows arm compiled on Linux.

Background

  • Markable<String> stores one String and uses the null string as its empty value (MarkableTraits<String> in WTFString.h). The layout is the same as String, and a valid call pays nothing new.
  • URL.h includes Markable.h at its end. Markable.h includes Hasher.h, and Hasher.h needs a complete URL.
  • The CF variant (URLCF.cpp) does not change. This fork does not build it.
  • Bun does not need this change to fix the empty path on Linux and macOS. oven-sh/bun#45354 reads fileSystemPath() through one accessor, Bun::fileSystemPath(), which returns Markable<String> already, and a regex lint finds a raw call. With this change that accessor compiles with no edit, and a raw call is a compile error.

Downsides

  • On Windows, a file: URL with a path that is not UTF-8 no longer converts. Before, it converted to a name with U+FFFD, which names a different file. In Bun that makes node:fs, Bun.file(), createRequire() and Worker throw on Windows as they do on Linux and macOS after oven-sh/bun#45354, and Bun.SQL uses another file name. This is the decision for a maintainer. The return type and the Windows decode can land as two changes: the return type alone changes no behavior.
  • An upstream merge that adds a fileSystemPath() call in code this fork compiles needs an unwrap. The delta is +54/-14 lines in 5 files.
  • CI does not build TestWTF, so CI does not run the new test.
Notes
  • The version bump in Bun that takes this change deletes test/internal/source-lints/file-system-path.test.ts (the compiler does its work) and rewrites the Windows skips of the tests of oven-sh/bun#45354. The Bun.SQL expectation needs a Windows form.
  • TestWTF does not compile at this commit for a different reason: Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp:786 calls ExternalStringImpl::create with 2 arguments, and the fork's version takes 3. I linked the test binary without that object to run the WTF_URL tests (20 of 20 pass).
  • The Windows arm has no Windows API calls. I compiled URL.cpp and the test with the two #if OS(WINDOWS) blocks forced on, on Linux, and the Windows expectations pass (C:\a b\café.txt, the UNC path, a literal %, and empty for the 7 undecodable URLs).
  • A % that does not start an escape (50%.txt, 50%zz.txt) stays literal, as before.
  • decodeEscapeSequencesFromParsedURL, user() and password() do not change.