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

Add Windows named pipe support for client #1808

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion lib/websocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -466,10 +466,33 @@ function initAsClient(websocket, address, protocols, options) {
}

let parsedUrl;
let isWinPipe;

if (address instanceof URL) {
parsedUrl = address;
websocket.url = address.href;
} else if ((isWinPipe = /^ws\+unix:\/\/\\\\[.?]\\pipe\\/.test(address))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not happy to skip the URL parser which is why there is no IPC support on Windows. This is not a valid URL.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, I see your point, and such a URL does seem a little bit odd.

let pathname = address.slice(10);
const index = pathname.indexOf('?', 9);
let search = '';

if (index !== -1) {
search = pathname.slice(index);
pathname = pathname.slice(0, index);
}

parsedUrl = {
protocol: 'ws+unix:',
hostname: '',
port: '',
host: '',
origin: '',
pathname,
search,
hash: '',
username: '',
password: ''
};
} else {
parsedUrl = new URL(address);
websocket.url = address;
Expand Down Expand Up @@ -528,7 +551,12 @@ function initAsClient(websocket, address, protocols, options) {
opts.auth = `${parsedUrl.username}:${parsedUrl.password}`;
}

if (isUnixSocket) {
if (isWinPipe) {
const index = opts.path.indexOf(':', 11);
Copy link
Member

@lpinca lpinca Oct 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't index be incorrectly set to -1 if opts.path is something like '\\.\pipe\x:/foo'? '\\.\pipe\x' is a valid named pipe.


opts.socketPath = index === -1 ? opts.path : opts.path.slice(0, index);
opts.path = index === -1 ? '' : opts.path.slice(index + 1);
} else if (isUnixSocket) {
const parts = opts.path.split(':');

opts.socketPath = parts[0];
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "ws",
"version": "7.3.1",
"version": "7.3.2",
"description": "Simple to use, blazing fast and thoroughly tested websocket client and server for Node.js",
"keywords": [
"HyBi",
Expand Down
14 changes: 5 additions & 9 deletions test/websocket-server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,20 +115,16 @@ describe('WebSocketServer', () => {
});

it('uses a precreated http server listening on unix socket', function (done) {
//
// Skip this test on Windows. The URL parser:
//
// - Throws an error if the named pipe uses backward slashes.
// - Incorrectly parses the path if the named pipe uses forward slashes.
//
if (process.platform === 'win32') return this.skip();

const server = http.createServer();
const sockPath = path.join(
let sockPath = path.join(
os.tmpdir(),
`ws.${crypto.randomBytes(16).toString('hex')}.sock`
);

if (process.platform === 'win32') {
sockPath = '\\\\?\\pipe\\' + sockPath;
}

server.listen(sockPath, () => {
const wss = new WebSocket.Server({ server });

Expand Down