On Tue, 26 Jul 2016, David Woodhouse wrote: Hello!
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 loads a JavaScript interpreter into the address space of anything which uses 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 things:
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 pac is rather old. If this passes the full URL to the PAC javascript then 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 destination URLs. -- / daniel.haxx.se ------------------------------------------------------------------- List admin: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.haxx.se/mail/etiquette.html
