Deno: [std/path] APIs not handling percent symbol characters well

[asos-craigmorten]

Setup

OS: MacOS Mojave

$ deno --version
deno 1.3.0
v8 8.6.334
typescript 3.9.7

Issue

The likes of fromFileUrl() does not handle percent symbol (%) characters in the provided file url.

An error is thrown in the use of decodeURIComponent() which does not handle isolated percent symbol characters in the provided url as it deems them an invalid URL encoding. Subsequently Deno.readTextFile() and potentially other APIs are impacted.

I'm not brushed up on File URIs but gut feeling is likely this is less an issue with the aforementioned APIs but rather one with the likes of join, which at the moment will happily let you construct an invalid file URL with an unencoded % making it easy for user error.

Minimal Reproduceable Example(s)

Example with fromFileUrl() error:

// repro.ts
import {
  fromFileUrl,
  dirname,
  join,
} from "https://deno.land/[email protected]/path/mod.ts";

const __dirname = dirname(import.meta.url); // This returns a stringified file url
const fileUrl = join(__dirname, "% users.txt");
console.log({ __dirname, fileUrl });

console.log(fromFileUrl(fileUrl)); // boom

$ touch "% users.txt" # create file with special character in name / path
$ echo "20%" > "./% users.txt"
$ deno run --allow-read ./repro.ts
Check file:///Users/craig.morten/git/asos-craigmorten/opine/repro.ts
{
  __dirname: "file:///Users/craig.morten/git/asos-craigmorten/opine",
  fileUrl: "file:/Users/craig.morten/git/asos-craigmorten/opine/% users.txt" # <--- this causes an issue
}
error: Uncaught URIError: URI malformed
  return decodeURIComponent(url.pathname);
         ^
    at decodeURIComponent (<anonymous>)
    at fromFileUrl (posix.ts:439:10)
    at repro.ts:11:13

Example with Deno.readTextFile() error:

$ touch "% users.txt" # create file with special character in name / path
$ echo "20%" > "./% users.txt"
$ deno # start repl

> const filePath = "/Users/craig.morten/git/asos-craigmorten/opine/% users.txt";
undefined
> Deno.readTextFile(filePath).then(m => console.log(m)); # works as expected
Promise { <pending> }
20%

> const fileUrl = new URL(`file:/Users/craig.morten/git/asos-craigmorten/opine/% users.txt`);  # <--- this causes an issue
undefined
> Deno.readTextFile(fileUrl).then(m => console.log(m));
Promise { <pending> }
error: Uncaught URIError: URI malformed
    at decodeURIComponent (<anonymous>)
    at pathFromURLPosix (rt/06_util.js:86:12)
    at pathFromURL (rt/06_util.js:97:11)
    at opOpen (rt/30_files.js:35:41)
    at open (rt/30_files.js:52:23)
    at Object.readTextFile (rt/40_read_file.js:30:24)
    at <unknown>:3:6
    at evaluate (rt/40_repl.js:60:36)
    at replLoop (rt/40_repl.js:160:15)

Expectation

This should be easily solvable on any user's side by first encoding any / all path components, e.g.

const __dirname = dirname(import.meta.url); # This returns a stringified file url
const fileUrl = join(__dirname, encodeURIComponent("% users.txt"));

But this _feels_ like a hidden gotcha for users, and it would be great if APIs such as join could gracefully handle such path segments and construct a valid file url? Happy to be overruled and have the responsibility lie with the consumer! It certainly is a niche issue.

[nayeemrmn]

FYI you don't use join() for URLs. You "base" them with the URL constructor, which will also do the encoding for you:

const fileUrl = new URL("% users.txt", import.meta.url); // 

But even though the encoding is done for you, decodeURIComponent() will fail. This is because encodeURIComponent()/decodeURIComponent() are actually the wrong thing to use for url.pathname... I'll get around to this soon.

EDIT: Actually decodeURIComponent() should work. We just have to specially handle isolated percents first.

[asos-craigmorten]

Ah good to know, user error on my part mostly then 😅 guess initially have wrongly conflated

const __dirname = dirname(import.meta.url)

in my mind to be an equivalent of __dirname in Node when it isn't (as the prior is a file URL string, and the latter is a path string), and then let that confusion persist through to path based operations (e.g. join) instead of using the appropriate URL methods as you shared 🤦

Regardless we still hit an error as you appear to allude to in your comment 🙃

// repro.ts
import {
  fromFileUrl,
} from "https://deno.land/[email protected]/path/mod.ts";

const fileUrl = new URL("% users.txt", import.meta.url);
console.log(fileUrl);
console.log(fromFileUrl(fileUrl)); // boom

$ touch "% users.txt" # create file with special character in name / path
$ echo "20%" > "./% users.txt"
$ deno run repro.ts 
Check file:///Users/craig.morten/git/asos-craigmorten/permission-guard/repro.ts
URL { href: "file:///Users/craig.morten/git/asos-craigmorten/permission-guard/%%20users.txt", origin: "null", protocol: "file:", username: "", password: "", host: "", hostname: "", port: "", pathname: "/Users/craig.morten/git/asos-craigmorten/permission-guard/%%20users.txt", hash: "", search: "" }
error: Uncaught URIError: URI malformed
  return decodeURIComponent(url.pathname);
         ^
    at decodeURIComponent (<anonymous>)
    at fromFileUrl (posix.ts:439:10)
    at repro.ts:7:13