On Thu, Sep 13, 2012 at 08:03:06AM -0400, C. Michael Pilato wrote:
> On 09/12/2012 07:52 PM, Josh Triplett wrote:
> > The attached patch implements support for the http_proxy and https_proxy
> > environment variables. This allows subversion to automatically make use
> > of system-wide proxy configuration, rather than requiring
> > subversion-specific proxy configuration; environment variables also make
> > it easier to have network-specific configuration on machines that use
> > more than one network. Subversion-specific configuration (in
> > ~/.subversion/servers) will override the environment.
> >
> > Written after observing some ChromeOS build system documentation that
> > showed how to configure proxy environment variables, and then listed
> > subversion as the one special case requiring its own configuration.
> >
> > - Josh Triplett
>
> Josh,
>
> Thanks for the patch. This missing feature in Subversion has been observed
> several times in the past[1]. I'm in favor of rectifying it -- where
> longstanding and useful conventions exist, it just makes sense to honor them.
Glad to hear that.
> Some thoughts on your patch:
>
> - Do the environment variables "http_proxy" and "https_proxy" mean anything
> on non-*nix systems? If so, do they mean exactly the same thing as in the
> *nix case? Do we need to avoid honoring those on, say, Windows platforms?
They're less likely to be set on Windows, but they don't have any
special meaning there that would conflict with using them if available.
I'd suggest honoring them on all platforms.
Ideally, Subversion should also honor the Windows and OS X system proxy
settings, but that would require a significantly more involved patch to
retrieve and use those.
> - Should the code ensure that the scheme of the proxy URL matches the scheme
> of the Subversion repository URL?
No; quite frequently, https_proxy=http://...
> - You return an error ("Invalid proxy URL '%s'") when the proxy URL fails to
> parse cleanly. I'd suggested indicating in that error message that we got
> the URL from the environment (so users know where to look in order to make
> corrections).
Fair enough; I can do that.
> But stepping back a second: what is the convention employed
> by other programs when the environment variable value is malformed? Do they
> error, or simply ignore the variable? If we're going to track a convention
> here, we might as well track it completely.
Well-behaved programs always attempt to observe the proxy and fail if
they can't (though the quality of such validation varies):
$ http_proxy=http://bad.host.and.port:100000 wget http://example.org/
Error parsing proxy URL http://bad.host.and.port:100000: Bad port number.
$ http_proxy=http://bad.host.and.port:100000 curl http://example.org/
curl: (5) Couldn't resolve proxy 'bad.host.and.port'
$ http_proxy=http://bad.host.and.port:100000 GET http://example.org/
Can't connect to bad.host.and.port:100000 (Bad hostname)
LWP::Protocol::http::Socket: Bad hostname 'bad.host.and.port' at
/usr/share/perl5/LWP/Protocol/http.pm line 51.
For a rationale, consider the case of an anonymizing or otherwise
privacy-protecting proxy; ignoring it because of a typo would cause the
user to leak private information. Better to fail closed here.
> - While your approach was the super simple one to take, how do we feel about
> the fact that this would cause all programs which linked against
> Subversion's libs to start noticing those environment variables which are
> really kind meant to be specific to just the command-line clients. In the
> past, I (and others) have gone on record against such things. Perhaps a
> better approach would be for 'svn' -- or one of the common svn_cmdline_*
> functions in libsvn_subr -- to parse the env variables and use the results
> to override the parsed runtime configuration values in the svn_config_t
> structures that get passed all over the place. I dunno.
Why should only the command-line clients notice those environment
variables? If someone has http_proxy set, and runs some graphical
subversion client using libsvn, that client ought to use the proxy as
well. I'd suggest that libsvn should always respect system proxy
settings, rather than forcing the caller to do extra (duplicate) work to
parse and set the proxy.
- Josh Triplett
Received on 2012-09-13 16:51:23 CEST