> On Sept. 25, 2011, 2:20 p.m., Andreas Hartmetz wrote:
> > I'd actually be interested to hear which testing you did.
> >
> > The "ResolveHostNamesBeforeProxyCheck" option seems strange. In which
> > situations is this supposed to be set / not set?
>
> Dawit Alemayehu wrote:
> The "ResolveHostNamesBeforeProxyCheck" option is used to give the user
> the ability to turn DNS lookups of request URLs on or off before checking
> them against the "No Proxy For" list. This makes it possible for us to let
> people enter IP address ranges, e.g. "192.168.0.1/24" in the "NoProxyFor"
> list while at the same time protecting those people that do not want to have
> any form of name resolution to happen at the client application level. The
> default behavior is what it currently is, no lookup, since we do not support
> IP address ranges right now.
>
> As far as testing goes, I created a basic SOCKS server using ssh, ssh -D
> 9999 127.0.0.1, and a very basic PAC script file that contains the following:
>
> function FindProxyForURL( url, host )
> {
> var resolved_ip = dnsResolve(host);
>
> if (isInNet(resolved_ip, "127.0.0.1", "255.255.255.0"))
> return "DIRECT";
>
> return "SOCKS 127.0.0.1:9999; DIRECT";
> }
>
> I am also in the process of testing all these changes agains a real proxy
> sever. I am going to test against bother privoxy and squid. To test this
> however, you also need the next patch in the series,
> https://git.reviewboard.kde.org/r/102696/.
OK, I see what ResolveHostNamesBeforeProxyCheck does now. Thanks.
> On Sept. 25, 2011, 2:20 p.m., Andreas Hartmetz wrote:
> > kio/kio/kprotocolmanager.cpp, line 471
> > <http://git.reviewboard.kde.org/r/102691/diff/1/?file=37173#file37173line471>
> >
> > This looks more like
> > "if no proxy scheme is given, assume SOCKS"
>
> Dawit Alemayehu wrote:
> Ahh... Did you mean, "if no proxy could be found for the scheme of the
> given url, assume SOCKS" ? Even then that comment belongs to the if statement
> above the one where this comment currently resides. Perhaps I should move the
> comment down inside the if statement.
I am now more confused than before.
What I (still) think the code is doing is: if there is a proxy URL given with
no scheme, prepend "socks://" to the proxy URL.
- Andreas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102691/#review6796
-----------------------------------------------------------
On Sept. 25, 2011, 4:15 p.m., Dawit Alemayehu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102691/
> -----------------------------------------------------------
>
> (Updated Sept. 25, 2011, 4:15 p.m.)
>
>
> Review request for kdelibs.
>
>
> Description
> -------
>
> This patch is the 4th in the serious of patches designed to resolve bugs and
> missing functionality in KDE's proxy manager. The changes made with this
> patch are as follows:
>
> * Add code that resolves a request url's hostname before attempting to
> match
> it against the no proxy for list so long as the
> "ResolveHostNamesBeforeProxyCheck"
> option is set.
>
> * Allow "DIRECT" as a special keyword in the list of proxy server
> addresses
> returned in slaveProtocol(const QString& protocol, QStringList& proxy).
>
> * Change KProtocolManager::proxyFor to properly handle the changes in the
> new
> proxy management dialog (KDE 4.8) where the proxy server port, in the
> manual proxy configuration mode, will be saved separated from the
> address with
> a white space.
>
> * Move the code that accounts for SOCKS proxy from
> KProtocolManager::proxyFor
> to KProtocolManager::proxyForUrl where it belongs. The current
> implementation
> only works correctly under one circumstance while breaking the previous
> behavior
> of the function.
>
> * Fix KProtocoManager::proxiesForUrl so that it accounts for the proxy
> exception list.
>
> * Update API documentation to reflect the changes above.
>
>
> Diffs
> -----
>
> kio/kio/kprotocolmanager.h 11e43fe
> kio/kio/kprotocolmanager.cpp 50ebb6e
>
> Diff: http://git.reviewboard.kde.org/r/102691/diff/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dawit Alemayehu
>
>
