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

info.origin and info.req.headers.host issue in verifyClient after upgrade ws from 1.1.4 to 3.3.3 #1271

Closed
rutanika opened this issue Jan 3, 2018 · 12 comments

Comments

@rutanika
Copy link

rutanika commented Jan 3, 2018

Hello,

After we upgraded ws from 1.1.4 to 3.3.3 and did the adjustment as suggested in #1099, we get different behaviour in verifyClient.
We recognized that in verifyClient info.origin comes without port and info.req.headers.host comes with port 443.
Without the change both of them comes without the port.
Do you know what can be the reason for that?

Thanks,
Tali

node version: 6.0.0
platform: cf

@lpinca
Copy link
Member

lpinca commented Jan 3, 2018

I'm not sure, info.origin is just a shortcut to req.headers['sec-websocket-origin'] or req.headers.origin depending on the protocol version.

Is the client a browser?

@rutanika
Copy link
Author

rutanika commented Jan 3, 2018

req.headers['sec-websocket-version'] is 13, therefor origin is req.headers['origin']

@lpinca
Copy link
Member

lpinca commented Jan 3, 2018

yes that's correct.

@rutanika
Copy link
Author

rutanika commented Jan 3, 2018

right, but I still don't understand why req.headers['origin'] comes without port and info.req.headers.host comes with port 443 :)

@lpinca
Copy link
Member

lpinca commented Jan 3, 2018

They are set by the client, are origin and host the same on 1.1.4? Is the client the same or are you using a different client?

@rutanika
Copy link
Author

rutanika commented Jan 3, 2018

Yes, on 1.1.4 origin and host are the same.
The client is also the same. The only change is the version of ws and the adjustment for upgradeReq property.

@lpinca
Copy link
Member

lpinca commented Jan 3, 2018

origin is the same on both 1.1.4 and 3.3.3, host is never touched so I don't really know.

Print info.req.headers and see if it is the same on both versions.

@rutanika
Copy link
Author

rutanika commented Jan 3, 2018

I already printed it and saw that the host has port 443 in 3.3.3 and does not has port at all in 1.1.4.

I open this github issue only after I printed it and saw the difference.

@lpinca
Copy link
Member

lpinca commented Jan 3, 2018

I cannot reproduce, here is my code.

const https = require('https');
const fs = require('fs');
const WebSocket = require('.');

const data = `<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
  </head>
  <body>
    <script>
      (function () {
        var ws = new WebSocket('wss://localhost');
      })();
    </script>
  </body>
</html>`;

const server = https.createServer({
  cert: fs.readFileSync('test/fixtures/certificate.pem'),
  key: fs.readFileSync('test/fixtures/key.pem')
});

server.on('request', (req, res) => {
  res.setHeader('Content-Type', 'text/html');
  res.end(data);
});

const wss = new WebSocket.Server({
  verifyClient (info) {
    console.log(info.origin);
    console.log(info.req.headers.host);
    return true;
  },
  server
});

wss.on('connection', (ws) => ws.close());

server.listen(443);

This prints the same stuff on both 1.1.4 and 3.3.3 using Chrome as client.

@lpinca
Copy link
Member

lpinca commented Jan 4, 2018

@rutanika I've also tested it on Firefox and it works the same on both 1.1.4 and 3.3.3.
Are you able to write a test to reproduce the issue?

@lpinca
Copy link
Member

lpinca commented Jan 5, 2018

Closing as I can't reproduce this, discussion can continue on the closed thread.

@lpinca lpinca closed this as completed Jan 5, 2018
@rutanika
Copy link
Author

rutanika commented Jan 7, 2018

Hi,
I will try to isolate the issue and create a standealone example that demonstrates what was done here.
Thanks,
Tali

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

2 participants