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

protocol option expects a colon #711

Closed
javadoug opened this issue Oct 8, 2014 · 5 comments
Closed

protocol option expects a colon #711

javadoug opened this issue Oct 8, 2014 · 5 comments

Comments

@javadoug
Copy link

javadoug commented Oct 8, 2014

My reverse proxy didn't like my requests because I needed to specify the protocol on the proxy config and include a colon on the end of the string. I wonder if the colon is over zealous code, a typo or really needed.

Error:
400 Bad Request
The plain HTTP request was sent to HTTPS port

https://github.com/nodejitsu/node-http-proxy/blob/e7d50b1a376035a50c82db38605e99feb30afd36/lib/http-proxy/common.js#L28

https://github.com/nodejitsu/node-http-proxy/blob/fcdbf46e4d1301ac78da05eeeac583339a5dc43a/lib/http-proxy/passes/ws-incoming.js#L87

Note: connect does NOT require a colon on the protocol option. The server will fail to start with a fatal error message stating invalid protocol.

The grunt-connect-proxy settings needed to make http-proxy work:
image

@jcrugzz
Copy link
Contributor

jcrugzz commented Oct 9, 2014

@javadoug we expect to receive a full URL string or an object that has been parsed by url.parse(). This is why I just deal with URL strings when it comes to config as it simplifies it infinitely.

@javadoug
Copy link
Author

javadoug commented Oct 9, 2014

@jcrugzz thanks for getting back. pls consider an alternative, perhaps, checking with regex /^https:?/.test(protocol). And if the code construct urls from the parts, the url class supports protocol with and without the colon.

@jcrugzz
Copy link
Contributor

jcrugzz commented Oct 9, 2014

@javadoug I'd take a PR with a proper regex :). I'd export a protocol regex from common so it can be used in both places. Should be something like exports.protocolRegex = /^https|wss/. This also makes sure its precompiled.

@jcrugzz jcrugzz closed this as completed in c044856 Dec 9, 2014
@jcrugzz
Copy link
Contributor

jcrugzz commented Dec 9, 2014

@javadoug implemented in 1.7.3

@javadoug
Copy link
Author

javadoug commented Dec 9, 2014

thank you.

On Dec 8, 2014, at 11:05 PM, Jarrett Cruger notifications@github.com wrote:

@javadoug implemented in 1.7.3


Reply to this email directly or view it on GitHub.

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