Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open Unable to Connect to localhost on Windows WSL (v. 7.0.0) #154

Closed
ITenthusiasm opened this issue Oct 30, 2019 · 19 comments
Closed

Open Unable to Connect to localhost on Windows WSL (v. 7.0.0) #154

ITenthusiasm opened this issue Oct 30, 2019 · 19 comments

Comments

@ITenthusiasm
Copy link

ITenthusiasm commented Oct 30, 2019

For some reason I cannot open to my localhost port with the current version of the package (7.0.0). Attempting to do so yields the following error:

Windows cannot find '\http://localhost:3000\'. Make sure you typed the name correctly, and then try again.

Here is my JavaScript code:

import express from 'express';
import path from 'path';
import open from 'open';

const app = express();
const port = process.env.PORT || 3000;

app.get(['/'], function(req, res) {
  res.sendFile(path.join(__dirname, '../public/index.html'));
});

app.listen(port, function(err) {
  if (err) {
    console.error(err);
  } else {
    console.info(`App listening on port ${port}`);
    open('http://localhost:' + port);
  }
});

Note that everything works correctly on version 6.4.0, and I can open to my localhost just fine with that. It's only on 7.0.0 that I'm seeing this issue.

@sindresorhus
Copy link
Owner

// @herberttn

@btd
Copy link

btd commented Nov 2, 2019

This is not about really localhost problem, you can call open with just ./file.html and get the same result. Reproduced both in wsl and ps

@ArthurBiazonMachado
Copy link

The problem is that the url is being enclosed in double quotes to allow spaces and other unmarked characters in the target. As a result, the quotes are getting passed to the browser.

		// target = `"${target}"`;
		// childProcessOptions.windowsVerbatimArguments = true;

Commenting lines 76 and 77 of index.js corrects the issue.

@herberttn
Copy link
Contributor

I cannot reproduce the problem. I'm trying require('open')('http://localhost:7070') on the Node.js v12 REPL. It correctly opens the browser with the correct URL.

Can you guys post some more details about your setup? Versions, envs, shells, etc.

@btd
Copy link

btd commented Nov 14, 2019

@herberttn i created very simple repro case. Just clone and run node index.js. I used WSL1 and node 10.
Untitled
If you install open@6 it will starts workimg

@FKobus
Copy link

FKobus commented Dec 4, 2019

@herberttn I can confirm that this bug on Windows 10 + WSL + node v10.15.3 + open 7.0.0 exists. I'm also getting this popup after upgrading to open 7

[edit] added node version

@herberttn
Copy link
Contributor

Thanks @btd and @FKobus, I haven't used WSL all that much recently, so I'll take some time to refresh that knowledge and reproduce this.

@endiliey
Copy link

endiliey commented Dec 5, 2019

Can confirm this is happening

I just upgraded react-scripts to 3.3.0 (which upgraded open v6 to v7)

Reproducible repo here. https://github.com/endiliey/rengorum

Can only be reproduced in WSL. Edit: I think windows too
image

@herberttn
Copy link
Contributor

Edit: I think windows too

Hey @endiliey, what do you mean by this? Can you give me some more details?

@endiliey
Copy link

endiliey commented Dec 5, 2019

@herberttn my bad. I tried to reproduce it again, couldn't reproduce it on Windows. Only on WSL for me

@ArthurBiazonMachado
Copy link

It only happens on WSL, because of some argument handling on Windows that allows arguments to have spaces and other characters. Apparently, it doesn't work for WSL. Lines 76 and 77 of index.js are the culprits.

	} else if (process.platform === 'win32' || isWsl) {
		command = 'cmd' + (isWsl ? '.exe' : '');
		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, #115
		// - Pull requests: #74, #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');
		}

		if (options.app) {
			if (isWsl && options.app.startsWith('/mnt/')) {
				const windowsPath = await wslToWindowsPath(options.app);
				// eslint-disable-next-line require-atomic-updates
				options.app = windowsPath;
			}

			cliArguments.push(options.app);
		}

		if (appArguments.length > 0) {
			cliArguments.push(...appArguments);
		}
	} else {

@FKobus
Copy link

FKobus commented Dec 12, 2019

Did some more testing. So far it looks like it's only happening when you run via WSL.

✔️ Regular cmd/bash on Windows
✔️ Mac
❌ WSL on Windows

@herberttn
Copy link
Contributor

Did some more testing. So far it looks like it's only happening when you run via WSL.

✔️ Regular cmd/bash on Windows
✔️ Mac
❌ WSL on Windows

Yeah, I've arrived at the same conclusion. The sad part is I've not been able to find any documentation that even begins to explain why. I guess we could just check if we're in WSL (as some other caveats already do).

@sindresorhus would you be comfortable with it?

@ITenthusiasm ITenthusiasm changed the title Open Unable to Connect to localhost on Windows (v. 7.0.0) Open Unable to Connect to localhost on Windows WSL (v. 7.0.0) Dec 12, 2019
@Summon528
Copy link

Yeah, I've arrived at the same conclusion. The sad part is I've not been able to find any documentation that even begins to explain why. I guess we could just check if we're in WSL (as some other caveats already do).

If I have to make a guess I will think it is because node thinks it is in Unix, therefore ignore the windowsVerbatimArguments option.

windowsVerbatimArguments <boolean> No quoting or escaping of arguments is done on Windows. Ignored on Unix. Default: false. source

@pluma
Copy link
Contributor

pluma commented Feb 8, 2020

Just a heads up: this should be fixed now via #166 and #168.

@FKobus
Copy link

FKobus commented Feb 8, 2020

Which release should we expect to this being fixed so we can update accordingly?

I'll test and report the feedback in this thread

@bodnarbm
Copy link

bodnarbm commented Feb 9, 2020

@FKobus The final PR (#168) landed in release v7.0.2. (#166 itself landed in v7.0.1)

karlhorky added a commit to karlhorky/heml that referenced this issue Feb 9, 2020
@karlhorky
Copy link

Nice! @XxX-MLGNoob-XxX can this be closed now?

@ITenthusiasm
Copy link
Author

@karlhorky can confirm open is behaving properly on my WSL now.

Thanks, guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests