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.

Sign up for GitHub

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

Jump to bottom

fix(client-src): don't use self.location.port #1838

Merged
merged 1 commit into from May 7, 2019
Merged

fix(client-src): don't use self.location.port #1838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 2 additions & 4 deletions client-src/default/index.js
View file
Open in desktop
Expand Up @@ -198,7 +198,6 @@ const onSocketMsg = {

let hostname = urlParts.hostname;
let protocol = urlParts.protocol;
let port = urlParts.port;

// check ipv4 and ipv6 `all hostname`
if (hostname === '0.0.0.0' || hostname === '::') {
Expand All @@ -208,7 +207,6 @@ if (hostname === '0.0.0.0' || hostname === '::') {
// eslint-disable-next-line no-bitwise
if (self.location.hostname && !!~self.location.protocol.indexOf('http')) {
hostname = self.location.hostname;
port = self.location.port;
}
}

Expand All @@ -228,8 +226,8 @@ const socketUrl = url.format({
hostname,
port:
urlParts.path == null || urlParts.path === '/'
? port
: querystring.parse(urlParts.path).sockPort || port,
? urlParts.port
: querystring.parse(urlParts.path).sockPort || urlParts.port,
// If sockPath is provided it'll be passed in via the __resourceQuery as a
// query param so it has to be parsed out of the querystring in order for the
// client to open the socket to the correct location.
Expand Down
45 changes: 27 additions & 18 deletions test/Client.test.js
View file
Open in desktop
Expand Up @@ -7,20 +7,20 @@ const helper = require('./helper');
const config = require('./fixtures/client-config/webpack.config');
const runBrowser = require('./helpers/run-browser');

function startProxy(port) {
const proxy = express();
proxy.use(
'/',
httpProxy({
target: 'http://localhost:9001',
ws: true,
changeOrigin: true,
})
);
return proxy.listen(port);
}

describe('Client code', () => {
function startProxy(port) {
const proxy = express();
proxy.use(
'/',
httpProxy({
target: 'http://localhost:9001',
ws: true,
changeOrigin: true,
})
);
return proxy.listen(port);
}

beforeAll((done) => {
const options = {
compress: true,
Expand All @@ -38,6 +38,7 @@ describe('Client code', () => {

afterAll(helper.close);

// [HPM] Proxy created: / -> http://localhost:9001
describe('behind a proxy', () => {
let proxy;

Expand All @@ -47,13 +48,21 @@ describe('Client code', () => {
proxy = startProxy(9000);
});

afterAll(() => {
proxy.close();
afterAll((done) => {
proxy.close(() => {
done();
});
});

it('responds with a 200', (done) => {
const req = request('http://localhost:9000');
req.get('/sockjs-node').expect(200, 'Welcome to SockJS!\n', done);
{
const req = request('http://localhost:9000');
req.get('/sockjs-node').expect(200, 'Welcome to SockJS!\n', done);
}
{
const req = request('http://localhost:9001');
req.get('/sockjs-node').expect(200, 'Welcome to SockJS!\n', done);
}
});

it('requests websocket through the proxy with proper port number', (done) => {
Expand All @@ -62,7 +71,7 @@ describe('Client code', () => {
.waitForRequest((requestObj) => requestObj.url().match(/sockjs-node/))
Copy link
Member Author

@hiroppy hiroppy May 7, 2019 •
edited

Choose a reason for hiding this comment

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

When the port is 9000, requestObj.url() is http://localhost:9000/main.js(so, match returns null) and then requestObj.url() will be http://localhost:9001/sockjs-node/info?t=xxxx after this page will be proxyed.

.then((requestObj) => {
expect(requestObj.url()).toMatch(
/^http:\/\/localhost:9000\/sockjs-node/
/^http:\/\/localhost:9001\/sockjs-node/
);
browser.close().then(done);
});
Expand Down