Inline script tests

[EricCornelson]

Adding tests for inline scripts (e.g. <script> blocks in an html page)

[EricCornelson]

Note: requires the bug fixes from: microsoft/vscode-chrome-debug-core#449 before the tests will pass.

[roblourens]

What problem did you have with vscode-uri and file:// URIs?

[EricCornelson]

Here's what I found:

Executing the following code:

import { URI } from 'vscode-uri';
import { URL } from 'url';

// case: converting Windows file path to file url
const windows = 'file:///C:\\blub\\blubb.js';
// case: converting Windows file path to file url, only two slashes after the protocol (should add the correct number of slashes)
const windows2 = 'file://C:\\blub\\blubb.js';
// case: Windows file path to file url with extra invalid chars we want to encode
const spacesAndTabs = 'file:///C:\\blub with spaces and \ttabs\\blubb.js';
// case: Unix file path with too few slashes after the protocol
const unixFewSlashes = 'file:/blub/blab/bleeb.js';
// case: Unix file path with extra slashes after the protocol
const unixExtraSlashes = 'file://///blub/blab/bleeb.js';

printUrl(windows);
printUrl(windows2);
printUrl(spacesAndTabs);
printUrl(unixFewSlashes);
printUrl(unixExtraSlashes);

function printUrl(urlString: string) {
    console.log(`For string: "${urlString}"`);
    try { console.log(`With vscode-uri: "${URI.parse(urlString).toString()}"`); } catch (err) { console.log('Throws: ' + err.message); }
    try { console.log(`With vscode-uri (no encode): "${URI.parse(urlString).toString(true)}"`); } catch (err) { console.log('Throws: ' + err.message); }
    console.log(`With node URL: "${new URL(urlString).href}"\n`);
}

We get:

For string: "file:///C:\blub\blubb.js"
With vscode-uri: "file:///c%3A%5Cblub%5Cblubb.js"
With vscode-uri (no encode): "file:///c:\blub\blubb.js"
With node URL: "file:///C:/blub/blubb.js"

For string: "file://C:\blub\blubb.js"
With vscode-uri: "file://c:\blub\blubb.js/"
With vscode-uri (no encode): "file://c:\blub\blubb.js/"
With node URL: "file:///C:/blub/blubb.js"

For string: "file:///C:\blub with spaces and    tabs\blubb.js"
With vscode-uri: "file:///c%3A%5Cblub%20with%20spaces%20and%20%09tabs%5Cblubb.js"
With vscode-uri (no encode): "file:///c:\blub with spaces and   tabs\blubb.js"
With node URL: "file:///C:/blub%20with%20spaces%20and%20tabs/blubb.js"

For string: "file:/blub/blab/bleeb.js"
With vscode-uri: "file:///blub/blab/bleeb.js"
With vscode-uri (no encode): "file:///blub/blab/bleeb.js"
With node URL: "file:///blub/blab/bleeb.js"

For string: "file://///blub/blab/bleeb.js"
Throws: [UriError]: If a URI does not contain an authority component, then the path cannot begin with two slash characters ("//")
Throws: [UriError]: If a URI does not contain an authority component, then the path cannot begin with two slash characters ("//")
With node URL: "file:///blub/blab/bleeb.js"

The main problem is creating a file:/// url from a file path in a cross-platform way. The solution we were using before in resourceIdentifier.ts was encoding the entire string before replacing backslashes (which is what vscode-uri is doing by default), which causes issues in Windows paths. What needs to happen is we have to replace the backslashes with forward slashes, and then ulrEncode each chunk. Alternatively, on Mac/Linux, the file path usually starts with a /, which means that we would need to add special handling to intelligently apply the slashes cross-platform. We had been hoping to use the built in pathToFileUrl function, but it's only available in node 10.12+. Fortunately, it seems that the built in node URL class handles the file:/// url case elegantly for us.

I also noticed that we already have an existing pathToFileUrl function from V1. Not sure which is the best approach at this point. Any insight into issues we had there or why we might want to prefer vscode-uri? We can file a bug on vscode-uri, but I don't even know if I would consider this a bug per-se, more like a nice extra feature that URL handles the Windows path cases for us. Definitely want to make sure we're not re-inventing the wheel if this problem has already been solved in V1 or elsewhere.

[digeff]

I'd either move this to a different file, or rename this file to some other name that encompass both launch web server + provide static url.

[digeff]

I'd rename this to something else like testBreakpointHitsOnPageAction
genericBreakpointTest doesn't give me a lot of information about what the test is actually doing

[digeff]

Is it possible to have a race condition here if the stopped event gets sent while we are on line 147?

[roblourens]

The problem is that 'file:///C:\\blub\\blubb.js'; is not actually a well-formed file URI. We might be using those internally somewhere, in which case we should probably just move along with this and address that separately? But it looks like chrome sends URIs that look like file:///c:/foo/bar which are ok.

Not sure what the problem is exactly, maybe you are talking about the %5C in the file URI, but those are not an issue.

Example of how vscode-uri preserves the slash direction and how it formats uris:

encoding the entire string before replacing backslashes (which is what vscode-uri is doing by default), which causes issues in Windows paths

[EricCornelson]

Not sure what the problem is exactly, maybe you are talking about the %5C in the file URI, but those are not an issue.

How are they not an issue? For the instance here, where I'm using the URL class to build a file url for the launch.json for the tests, Chrome can't resolve a url like file:///C%3A%5Cblub%5Cblub.js to the file C:\blub\blub.js on disk, that's one issue. Using vscode-uri with a Windows path builds a file url that I can't use in the launch.json.

Now, as for using vscode-uri in the rest of the test code, because of the encoding, vscode-uri was changing the colon in file:///C:/etc to a %3A. Chrome can resolve that URL just fine, but because of the way we're setting breakpoints in v2 (via regex instead of just by URL) we're unable to set a breakpoint and it's failing the inline script tests. I added a bug to track this issue, because it is something that needs to be fixed in V2, but I think it's logically separate from the inline scripts stuff, and can be addressed later. I think we have more pressing stuff to fix in the core functionality for V2.

IMO, the fact that vscode-uri encodes the colon is a bug, as "file:///c:/blub/blub.js" is a completely valid URI, per https://tools.ietf.org/html/rfc8089

Also found a few other cases where vscode-uri differs from URL, it handles the obscure "vertical pipe" syntax (section E.2.2. "Vertical Line Character" of the RFC), and it resolves relative paths as well.

For string: "file:///C|/development/debug_adapters_v2/vscode-chrome-debug/testdata/inline_scripts/single.html"
With vscode-uri: "file:///C%7C/development/debug_adapters_v2/vscode-chrome-debug/testdata/inline_scripts/single.html"
With vscode-uri (no encode): "file:///C|/development/debug_adapters_v2/vscode-chrome-debug/testdata/inline_scripts/single.html"
With node URL: "file:///C:/development/debug_adapters_v2/vscode-chrome-debug/testdata/inline_scripts/single.html"

For string: "file:///C:/blub/relative/../blab.js"
With vscode-uri: "file:///c%3A/blub/relative/../blab.js"
With vscode-uri (no encode): "file:///c:/blub/relative/../blab.js"
With node URL: "file:///C:/blub/blab.js"

I can definitely understand wanting to use vscode-uri to more closely match what vscode does with URIs, but node's URL class seems to much more closely mimic what Chrome does with URLs, which seems like it might be more useful for our use cases. But I think that's part of a larger discussion, and outside the scope of this PR.

[roblourens]

I think that vscode-uri really wants URI.file(f).fsPath to always === f, and URI.parse to match .toString, which is why it preserves the slash direction, and it can only preserve the slash direction by uri-encoding backslashes. This is fine as long as the external world doesn't have to interpret the uri...

I'm not sure why the colon is uri-encoded, the spec does explicitly call out that case. I will file a bug for it.

vscode-uri also has the .toString(true) option which causes it to only minimally uri-encode, which I think was back-compat for some extensions, and with that it doesn't uri-encode colon but that's probably not what you want.

[roblourens]

Also, vscode-uri normalizes the drive letter to lowercase, which may or may not be an issue. Drive letter case between vscode and Chrome and internally has been a huge issue in the past.

[roblourens]

Filed microsoft/vscode#75027. For now of course you should do whatever unblocks you...