Handle URL and path escaping on Windows (#146)

Author: herberttn

index.d.ts

@@ -31,6 +31,17 @@ declare namespace open {
 		You may also pass in the app's full path. For example on WSL, this can be `/mnt/c/Program Files (x86)/Google/Chrome/Application/chrome.exe` for the Windows installation of Chrome.
 		*/
 		readonly app?: string | readonly string[];
+
+		/**
+		Uses `encodeURI` to encode the `target` before executing it.
+
+		The use with targets that are not URLs is not recommended.
+
+		Especially useful when dealing with the [double-quotes on Windows](https://github.com/sindresorhus/open#double-quotes-on-windows) caveat.
+
+		@default false
+		*/
+		readonly url?: boolean;
 	}
 }
 
@@ -39,6 +50,8 @@ Open stuff like URLs, files, executables. Cross-platform.
 
 Uses the command `open` on OS X, `start` on Windows and `xdg-open` on other platforms.
 
+There is a caveat for [double-quotes on Windows](https://github.com/sindresorhus/open#double-quotes-on-windows) where all double-quotes are stripped from the `target`.
+
 @param target - The thing you want to open. Can be a URL, file, or executable. Opens in the default app for the file type. For example, URLs open in your default browser.
 @returns The [spawned child process](https://nodejs.org/api/child_process.html#child_process_class_childprocess). You would normally not need to use this for anything, but it can be useful if you'd like to attach custom event listeners or perform other operations directly on the spawned process.
 

index.js

@@ -26,6 +26,7 @@ module.exports = async (target, options) => {
 	options = {
 		wait: false,
 		background: false,
+		url: false,
 		...options
 	};
 
@@ -39,6 +40,13 @@ module.exports = async (target, options) => {
 		options.app = options.app[0];
 	}
 
+	// Encodes the target as if it were an URL. Especially useful to get
+	// double-quotes through the double-quotes on windows caveat, but it
+	// can be used in any platform.
+	if (options.url) {
+		target = encodeURI(target);
+	}
+
 	if (process.platform === 'darwin') {
 		command = 'open';
 
@@ -55,8 +63,16 @@ module.exports = async (target, options) => {
 		}
 	} else if (process.platform === 'win32' || isWsl) {
 		command = 'cmd' + (isWsl ? '.exe' : '');
-		cliArguments.push('/c', 'start', '""', '/b');
-		target = target.replace(/&/g, '^&');
+		cliArguments.push('/s', '/c', 'start', '""', '/b');
+
+		// Always quoting target allows for URLs/paths to have spaces and unmarked characters, as `cmd.exe` will
+		// interpret them as plain text to be forwarded as one unique argument. Enabling `windowsVerbatimArguments`
+		// disables Node.js's default quotes and escapes handling (https://git.io/fjdem).
+		// References: Issues #17, #44, #55, #77, #101 and #115 / Pull requests: #74 and #98
+		//
+		// As a result, all double-quotes are stripped from the `target` and do not get to your desired destination.
+		target = `"${target}"`;
+		childProcessOptions.windowsVerbatimArguments = true;
 
 		if (options.wait) {
 			cliArguments.push('/wait');

index.test-d.ts

@@ -9,3 +9,4 @@ expectType<Promise<ChildProcess>>(open('foo', {app: 'bar'}));
 expectType<Promise<ChildProcess>>(open('foo', {app: ['bar', '--arg']}));
 expectType<Promise<ChildProcess>>(open('foo', {wait: true}));
 expectType<Promise<ChildProcess>>(open('foo', {background: true}));
+expectType<Promise<ChildProcess>>(open('foo', {url: true}));

readme.md

@@ -93,6 +93,39 @@ The app name is platform dependent. Don't hard code it in reusable modules. For
 
 You may also pass in the app's full path. For example on WSL, this can be `/mnt/c/Program Files (x86)/Google/Chrome/Application/chrome.exe` for the Windows installation of Chrome.
 
+##### url
+
+Type: `boolean`<br>
+Default: `false`
+
+Uses `encodeURI` to encode the target before executing it.<br>
+We do not recommend using it on targets that are not URLs.
+
+Especially useful when dealing with the [double-quotes on Windows](#double-quotes-on-windows) caveat.
+
+## Caveats
+
+### Double-quotes on Windows
+TL;DR: All double-quotes are stripped from the `target` and do not get to your desired destination (on Windows!).
+
+Due to specific behaviors of Window's Command Prompt (`cmd.exe`) regarding ampersand (`&`) characters breaking commands and URLs, double-quotes are now a special case.
+
+The solution ([#146](https://github.com/sindresorhus/open/pull/146)) to this and other problems was to leverage the fact that `cmd.exe` interprets a double-quoted argument as a plain text argument just by quoting it (like Node already does). Unfortunatelly `cmd.exe` can only do **one** of two things: handle them all **OR** not handle them at all. As per its own documentation:
+
+>*If /C or /K is specified, then the remainder of the command line after the switch is processed as a command line, where the following logic is used to process quote (") characters:*
+>
+>    1.  *If all of the following conditions are met, then quote characters on the command line are preserved:*
+>        - *no /S switch*
+>        - *exactly two quote characters*
+>        - *no special characters between the two quote characters, where special is one of: &<>()@^|*
+>        - *there are one or more whitespace characters between the two quote characters*
+>        - *the string between the two quote characters is the name of an executable file.*
+>
+>    2.  *Otherwise, old behavior is to see if the first character is a quote character and if so, strip the leading character and remove the last quote character on the command line, preserving any text after the last quote character.*
+
+The option that solved all of the problems was the second one, and for additional behavior consistency we're also now using the `/S` switch, so we **always** get the second option. The caveat is that this built-in double-quotes handling ends up stripping all of them from the command line and so far we weren't able to find a scaping method that works (if you do, please feel free to contribute!).
+
+To make this caveat somewhat less impactful (at least for URLs), check out the [url option](#url). Double-quotes will be "preserved" when using it with an URL.
 
 ## Related
 

test.js

@@ -33,6 +33,10 @@ test('wait for the app to close if wait: true', async () => {
 	await open('https://sindresorhus.com', {wait: true});
 });
 
+test('encode url if url: true', async () => {
+	await open('https://sindresorhus.com', {url: true});
+});
+
 test('open url in default app', async () => {
 	await open('https://sindresorhus.com');
 });
@@ -50,6 +54,30 @@ test('return the child process when called', async t => {
 	t.true('stdout' in cp);
 });
 
+test('open url with query strings', async () => {
+	await open('https://sindresorhus.com/?abc=123&def=456');
+});
+
+test('open url with a fragment', async () => {
+	await open('https://sindresorhus.com#projects');
+});
+
+test('open url with query strings and spaces', async () => {
+	await open('https://sindresorhus.com/?abc=123&def=456&ghi=with spaces');
+});
+
+test('open url with query strings and a fragment', async () => {
+	await open('https://sindresorhus.com/?abc=123&def=456#projects');
+});
+
+test('open url with query strings and pipes', async () => {
+	await open('https://sindresorhus.com/?abc=123&def=456&ghi=w|i|t|h');
+});
+
+test('open url with query strings, spaces, pipes and a fragment', async () => {
+	await open('https://sindresorhus.com/?abc=123&def=456&ghi=w|i|t|h spaces#projects');
+});
+
 if (isWsl) {
 	test('open url in specified windows app given a wsl path to the app', async () => {
 		await open('https://sindresorhus.com', {app: firefoxWslName});