Rob--W / proxy-from-env

A Node.js library to get the proxy URL for a given URL based on standard environment variables (http_proxy, no_proxy, ...).
MIT License
51 stars 19 forks source link

What about non URL values? #1

Closed stevenvachon closed 7 years ago

stevenvachon commented 7 years ago

HTTP_PROXY can have a value of "hostname:port", which does not include a protocol. They can also have values like "-" and ".".

This module simply returns the value of the env var.

Rob--W commented 7 years ago

I have submitted a PR that adds the changes. Please review it: #2.

stevenvachon commented 7 years ago

It solves the "hostname:port" part, which is great. It doesn't do anything about the other two possible values, however, if you were aiming to close this issue completely.

Rob--W commented 7 years ago

What do you mean by other values? What are "-" and "." supposed to do?

stevenvachon commented 7 years ago

"-" means use no proxy. "." is invalid. I think it's a common user mistake.

Rob--W commented 7 years ago

Where did you see the use of those values?

"-" -> use an empty string, don't set the proxy environment variable, or set the NO_PROXY environment variable.

"." -> What is the expected behavior? I'm currently adhering to the garbage in-garbage out principle, for the lack of convincing arguments to do otherwise.

stevenvachon commented 7 years ago

I'd heard about them in discussions on IRC. I think skipping "." is fine.

Rob--W commented 7 years ago

I mean no disrespect, but hearing about them in IRC does not mean that much. One of my goals with writing this library is to have an interoperable module for resolving proxies, and supporting unused or extremely uncommon features is counter to that.

I'm inclined to close this ticket after merging #2. If you strongly feel that "-" is not obscure and needs to be supported, please share some evidence that it is widely used in practice.

stevenvachon commented 7 years ago

As far as I could tell, there is no spec for this, so all we have to work on are proxy implementations and community experience. Considering that I myself have not seen "-", it may be a sign of obscurity and, yeah, #2 should be fine for now.