search

delvedor / hpagent

A ready to use http and https agent for working with proxies that keeps connections alive!
MIT License
182 stars 36 forks source link

POC: allow connect options to be passed to the proxy #81

Closed pmuellr closed 2 years ago

pmuellr commented 2 years ago

I'm looking to use this package in a product, but will need support to pass rejectUnauthorized, cert info, etc, into the proxy itself. Right now, looking for the best approach to do this, and this is what I have so far. Totally open for other approaches.

I noticed it's going to be hard to test things like rejectUnauthorized due to the various hacks in the tests - overriding DNS, etc :-) - been there, done that :heart:

Probably we'd want a separate set of tests, that don't set global env vars, override DNS, etc.

pmuellr commented 2 years ago

Ya, I have to test it in our product as well, see if it will do what we want, but I'd be adding more tests.

As mentioned ^^^, if we want to actually test rejectUnauthorized and checkServerIdentity(), that's likely not possible with the current tests with the overriding of env vars / DNS. Note that we don't HAVE to specifically test those properties (though they are the ones I'll be using), could test something else that wouldn't be impacted by the current overrides. Not seeing anything obvious though. I suspect rejectUnauthorized will be the most popular usage of this feature, so it does feel like we should test that.

If we wanted to create tests not using the current overrides, feels like we should create a new directory for the existing tests under /test, and then create another directory under /test for these new tests that don't use overrides, and then the npm tasks would run both set of tests independently. I think the NODE_EXTRA_CA_CERTS is the biggest problem, since I think it can ONLY be set from the CLI and can't be modified within the node runtime.

Alternatively, we could create some new keys/certs to just test those with the new proxy settings, not add those to the PEM file we're using but use them in the new tests, and also provide a way to turn the DNS override off (currently hard-coded "on" for the tests).

delvedor commented 2 years ago

I'm fine with two subfolders inside /test :)

delvedor commented 2 years ago

Closed in https://github.com/delvedor/hpagent/pull/72.