Re: [RFC PATCH] Add libproxy support
Date: Mon, 1 Aug 2016 01:00:24 +0200 (CEST)
On Tue, 26 Jul 2016, David Woodhouse wrote:
Thanks a lot for your contribution. Sorry for the slight delay in responding
to this patch.
> The original libproxy has evolved into a C++ monstrosity which potentially
> it... but the API is clean and simple.
Is the API documentation available online? I couldn't find it.
> Thus the expectation will be that asking libproxy should *consistently* give
> correct answers. Once that's true, I hope to change Linux distribution
> packaging guidelines to suggest that packaged applications *should* be using
> libproxy's results (or at least PacRunner's results) by default.
I can certainly see how that would be really useful for all those users behind
proxies, and especially ones using very creative PAC scripts.
I looked at your patch and it seems generally fine but I'm missing some
1. Tests. Can we provide some input to fake libproxy to respond something or
is there any other support to make tests with libproxy? We can of course
wrap/inject something ourselves otherwise. We need tests to make sure
things work as expected. Especially in combination with other
options/environment variables etc.
2. Clarity. This adds an alternative way to figure out proxy. What happens to
the other options if you enable/disable this? If libproxy detection fails,
should we consider the other options then? Should we fail the connection?
3. Version. We should probably add "libproxy/$version" to the version string
similar to how we output other dependencies and their versions.
4. Security. I see that you pass the full URL to
px_proxy_factory_get_proxies() and the libproxy code I found for running
it is a security problem - this was recently addressed in browsers to make
sure maliciously injected PAC-scripts can't be made to leak full
-- / daniel.haxx.se ------------------------------------------------------------------- List admin: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.haxx.se/mail/etiquette.htmlReceived on 2016-08-01