url.pathToFileURL doesn't generate valid URLs for UNC paths

[mceachen]

• Node.js version: v12.18.3 and v14.8.0

• Platform: Windows 10 (10.0.19041)

• Subsystem: url

What steps will reproduce the bug?

const url = require("url")
url.pathToFileURL("\\\\laptop\\My Documents\\FileSchemeURIs.doc")

Expected behavior

As per Microsoft's UNC URI documentation):

file://laptop/My%20Documents/FileSchemeURIs.doc

Actual behavior

URL {
  href: 'file:///laptop/My%20Documents/FileSchemeURIs.doc',
  origin: 'null',
  protocol: 'file:',
  username: '',
  password: '',
  host: '',
  hostname: '',
  port: '',
  pathname: '/laptop/My%20Documents/FileSchemeURIs.doc',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}

1. hostname should be laptop (not '').

2. pathname should be /My%20Documents/FileSchemeURIs.doc (not be prefixed by /laptop/).

[guybedford]

This sounds like a valid bug to me. The implementation is at https://github.com/nodejs/node/blob/master/lib/internal/url.js#L1368 where it seems clear this isn't properly being taken into account.

[mceachen]

I'd be happy to swing at it, I haven't committed to node yet.

[guybedford]

That would be really great, please feel free if you can. I believe that on posix machines such a path should be ignored, like for other path functions I believe. Building for Windows is documented at https://github.com/nodejs/node/blob/master/BUILDING.md#windows.

[mceachen]

(interestingly enough, fileURLToPath already behaves correctly)

[mceachen]

Do you want pathToFileURL to throw a new error code for invalid UNC paths (for example, when the path is missing like \\invalid?)

[jasnell]

Shouldn't need a new error code. Using one like ERR_INVALID_ARG_VALUE should work for that.

[guybedford]

I'm not sure what would be best - I don't think we currently do path validations, but if there is no adequate URL representation that would support fileURLToPath as its converse it likely makes sense to.