Follow cURL's rules for parsing and matching NO_PROXY

[abatkin]

There are a few ways in which reqwest's handling of NO_PROXY differs from cURL (and other implementations). The biggest issue is that whitespace between entries should be ignored/trimmed, but is not (i.e. "NO_PROXY='a, b'" would never match "b"). In addition, according to cURL's rules, a NO_PROXY entry without a leading dot should match the domain itself as well as any subdomains (reqwest only handles exact matches if there is no leading dot) and entries with a leading dot should only match subdomains (but request allows exact matches). Finally, cURL allows a special entry "*" to match all entries (effectively disabling use of the proxy).

All changes in behavior have corresponding changes to the tests.

I tried to keep the logic clear/clean/idiomatic but I'm definitely open to suggestions for improvement.

[seanmonstar]

Thanks for this! I think trying to match curl's rules is a great goal. Just to help me reason on the change, will this "break" any behavior that people might have come to rely on? I realize in a crazy way, people can rely on any ol' bug, but I mean if things were sufficiently different.

[abatkin]

The addition of wildcard (*) support is a new feature that shouldn't break anything, and handling embedded spaces in lists (i.e. google.com,microsoft.com and google.com, microsoft.com should be equivalent) I look at as a bugfix.

This does change behavior in a breaking way for a couple cases:

no_proxy	prior behavior	new behavior
no_proxy=google.com (no leading dot)	google.com (exact match) would match and not use the proxy, www.google.com (subdomains) would not match and would use the proxy	google.com (exact match) will will match and will use the proxy, www.google.com (subdomains) will also match and will not use the proxy
no_proxy=.google.com (leading dot)	google.com (exact match) and www.google.com (subdomains) would match and not use the proxy	google.com (exact match) would not match and will use the proxy, www.google.com (subdomains) will continue to match and use the proxy

I would argue that the first of these changes is absolutely correct (i.e. if this breaks someone, they need to fix their no_proxy) whereas the second one is a little more questionable. Doing some more research, it does appear that - while cURL follows these (newly implemented) rules - not all other implementations do. In other words, some implementations treat no_proxy=google.com and no_proxy=.google.com identically (match on exact strings sans dot, as well as substrings).

I can undo that second bit if you think it is safer. So: in this instance, should reqwest prefer better compatibility with the existing behavior, or copy cURL's behavior? (I'm torn)

[abatkin]

I should add that reqwest's no_proxy behavior already diverges from cURL: reqwest allows IP address (both v4 and v6) matches to specify a "prefix mask" (i.e. no_proxy=192.168.1.0/24 would match 192.168.1.17) whereas cURL only allows exact string matches. There are other implementations that do what reqwest does, and I'd say the behavior here is far superior compared to cURL.

[abatkin]

@seanmonstar Rebased against master, reverted the (incompatible) bit where NO_PROXY entries beginning with . no longer matched themselves, and added some docs to the NoProxy struct.

[seanmonstar]

reverted the (incompatible) bit where NO_PROXY entries beginning with . no longer matched themselves

I kinda think we could make that change even though it's breaking, since it doesn't seem like someone would normally be using .foo.bar unless trying to get the exact behavior of curl. But we can also make that a follow-up... Either way, thanks for this! It's excellent!