Skip to content

Conversation

@killmenot
Copy link

@killmenot killmenot commented Aug 9, 2018

http-proxy library that is used in http-server support options.

This PR adds abilities to pass proxy options via cli or programmatically (helps in cases such like https://github.com/divhide/node-grunt-http-server)

Fixes #214, fixes #246

@cortezcristian
Copy link

Please consider merging this PRwe really need to pass options to httpProxy.createProxyServer({}); like { secure: false }

@nLight
Copy link
Contributor

nLight commented Nov 7, 2018

We ended up forking the project to add additional error handling to the proxy. Because in the current implementation if proxy throws an error the server would just crash. https://github.com/dcos-labs/http-server

utc = argv.U || argv.utc,
logger;

var proxyOptionsBooleanProps = [
Copy link
Member

Choose a reason for hiding this comment

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

Can the underlying library handle it's own validation, and we just surface any errors? I'd like to avoid duplicating other people's requirements (since they're likely to change over time).

Copy link
Member

Choose a reason for hiding this comment

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

I think we can pass stuff through without this boolean checking, so long as we handle errors from http-proxy in a user-friendly way.

@BigBlueHat
Copy link
Member

@nLight would you be interested in submitting this as a PR? johntron@5aa9afd Did you add anything beyond that?

@nLight
Copy link
Contributor

nLight commented Nov 7, 2018

@BigBlueHat you mean submitting the error handling to this repo? I can of course, though that still won't be sufficient for us without the secure flag. Also as you can see from the commit the logging solution is less then ideal.

@nLight
Copy link
Contributor

nLight commented Nov 7, 2018

I'll open a PR and let's get the conversation started 👍

@nLight
Copy link
Contributor

nLight commented Nov 7, 2018

@BigBlueHat #477

@motevallian
Copy link

Can we please get this one merged and published?
I am using a self-signed certificate in dev mode with a client-side routing solution that needs to allow all invalid routes to go to /index.html. -P does not allow it as the certificate is invalid and I need to pass in secure=false to the proxy.

Copy link
Member

@thornjad thornjad left a comment

Choose a reason for hiding this comment

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

I really like this idea, and it would expose a ton of already-existing but hidden functionality! I'd like to drive for this to be in the next major release, if possible. There are some merge conflicts to address, though if you're too busy or no longer want to contribute here, I'd be willing to take over and make some of these changes before merging. Let me know though, I don't want to steal it from you if you want to work on it!

utc = argv.U || argv.utc,
logger;

var proxyOptionsBooleanProps = [
Copy link
Member

Choose a reason for hiding this comment

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

I think we can pass stuff through without this boolean checking, so long as we handle errors from http-proxy in a user-friendly way.

README.md Outdated

`-P` or `--proxy` Proxies all requests which can't be resolved locally to the given url. e.g.: -P http://someurl.com

`--proxyOptions` Pass proxy [options](https://github.com/nodejitsu/node-http-proxy#options) using nested dotted objects. e.g.: --proxyOptions.secure false
Copy link
Member

Choose a reason for hiding this comment

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

Given that there are/have been several issues and pull requests that will be solved by this, I think it's worth adding more to this, especially .secure false, which solves errors around self-signed certs (#214)

@roychoo

This comment has been minimized.

@diipak-bisht

This comment has been minimized.

@Mathyn
Copy link

Mathyn commented Oct 27, 2020

Looking at the requested changes it doesn't look like anything big. Any way I can help with this?

@yannickglt

This comment has been minimized.

@thornjad
Copy link
Member

Closing in favor of #688

@thornjad thornjad closed this Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor version non-breaking, non-trivial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add option --auto to set proxy option autoRewrite https certificate error when proxying to server with self-signed certificate

10 participants