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

add ability to pass proxyOptions #459

Closed
wants to merge 3 commits into from

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!

README.md Outdated Show resolved Hide resolved
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 Show resolved Hide resolved
README.md Outdated
@@ -53,6 +53,8 @@ This will install `http-server` globally so that it may be run from the command

`-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
10 participants