Created attachment 281692 [details] [review]
core: Support a per-remote "proxy" configuration option

Would have liked review, but I think this is OK

Attachment 281692 [details] pushed as b97a5f5 - core: Support a per-remote "proxy" configuration option

My review was on IRC and was in the form of "I have no idea why you would want a per-remote proxy. Proxies should be a global configuration of the current network"

1) yum supports per-remote, presumably someone uses that

2) It's actually reasonable to use a local caching proxy *only* for ostree/package content, but not for say your web browser

3) Current NM does not support proxy config AFAICS, so there's no present concept of "global configuration of current network" - when it does, we can pick it up instead of the current "http_proxy" environment variable.

This should actually be really simple. 

NM should pick up proxy config from DHCP / VPN / wpad / explicit per-connection config, and prod the appropriate configuration into the PacRunner dæmon. The PacRunner dæmon loads the PAC file into a JavaScript interpreter *once*, and answers queries over DBus. For NM, this is very similar to the way it picks up DNS settings and sets up dnsmasq.

Clients then query PacRunner with a simple "what proxy do I use for <this> URL?" and get a simple response. In the fullness of time, all clients should be doing this by *default* and it should be considered a bug if they don't.

There are many ways that cliients can query PacRunner. There's a direct DBus query, or they can do it through libproxy (there's a libproxy-pacrunner module for the real libproxy, and there's also a cut-down implementation of libproxy.so.1 which *only* does the DBus query to PacRunner and doesn't even support all the old nonsense). And isn't there also a glib-networking API for obtaining per-URL proxy settings? Which just uses libproxy IIRC?

So then it all Just Works™.

The horrid hacks in glib-networking which work around the historical brain-damage of libproxy should ideally be removed, and any per-application (and especially per-remote) configuration should be considered part of the *problem*.

AFAICT the patch being discussed here is a step in the *wrong* direction.

(In reply to comment #5)
> This should actually be really simple. 
> 
> NM should pick up proxy config from DHCP / VPN / wpad / explicit per-connection
> config

My understanding is this part doesn't exist.

 In the fullness of time, all clients should be
> doing this by *default* and it should be considered a bug if they don't.

Sounds right.
  
> AFAICT the patch being discussed here is a step in the *wrong* direction.

Even if the NM support appeared *today*, I'd still need to support manual configuration for earlier systems.

> > AFAICT the patch being discussed here is a step in the *wrong* direction.
> 
> Even if the NM support appeared *today*, I'd still need to support manual
> configuration for earlier systems.

Yes, absolutely. But please be *aware* that it's a step in the wrong direction.

In particular, please make sure that you *do* support the "just use libproxy" option, even though it may be too soon to make it the default. Then users for whom libproxy *is* returning correct results only have to enable it. And then it's easy to enable it by default too, if/when that becomes reasonable.

Note that it might be a 'just use glib-networking' option, not 'just use libproxy'.

Or maybe we could observe that talking directly to PacRunner is a safe default — if unconfigured either it'll return 'DIRECT' which was surely what your default behaviour would have been anyway, or it'll be completely absent and you won't get a reply.