Skip to content

Conversation

@rkalra247
Copy link
Collaborator

No description provided.

if (port)
return proxy.ws(req, socket, { target: `http://127.0.0.1:${port}` })

server.listen(443)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is wrong. You can't listen to port 443 AFTER websocket connection.

return `${prefix}${domain}` === req.headers.host
}) || {}
if (port)
return proxy.ws(req, socket, { target: `http://127.0.0.1:${port}` })
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't hardcode the ip address, you already got the ip address from the variable.

const { ip, port }: ProxyMapping =
mappings.find(({ subDomain, domain }) => {
const prefix = subDomain ? `${subDomain}.` : ''
return `${prefix}${domain}` === req.headers.host
Copy link
Collaborator

Choose a reason for hiding this comment

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

return fullDomain === req.headers.host

const mappings = getMappings()
const { ip, port }: ProxyMapping =
mappings.find(({ subDomain, domain, fullDomain }) => {
const prefix = subDomain ? `${subDomain}.` : ''
Copy link
Collaborator

Choose a reason for hiding this comment

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

prefix is not an unused variable. please remove it

const prefix = subDomain ? `${subDomain}.` : ''
return fullDomain === req.headers.host
}) || {}
if (port) return proxy.ws(req, socket, { target: `${ip}:${port}` })
Copy link
Collaborator

Choose a reason for hiding this comment

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

please put back http://

@songz songz merged commit 7dcbe58 into master Dec 6, 2019
@songz songz deleted the improveUI branch December 6, 2019 05:07
hwong0305 pushed a commit to hwong0305/myProxy that referenced this pull request Dec 6, 2019
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

Successfully merging this pull request may close these issues.

4 participants