*** Bug 604016 has been marked as a duplicate of this bug. ***

Review of attachment 163334 [details] [review]:

Overall looks good.  Some minor whitespace issues, and one question regarding scheme checking (keep in mind I know nothing about proxies, have never really used them).

::: Tomboy/Addins/WebSyncService/Api/ProxiedWebRequest.cs
@@ +21,3 @@
+			if (useLibProxy) {
+				try {
+					ApplyProxy(webRequest, uri);

whitespace around (

@@ +23,3 @@
+					ApplyProxy(webRequest, uri);
+
+					Console.WriteLine("WARNING: libproxy not installed");

Please use the Logger class instead of Console, and fix whitespace around (

@@ +34,3 @@
+		{
+			ProxyFactory pf = new LibProxy.ProxyFactory();
+			string[] proxies = pf.GetProxies(uri);

whitespace around (

@@ +37,3 @@
+			
+			foreach (string proxy in proxies) {
+				Uri proxyUri=new Uri(proxy);

whitespace around = and (

@@ +41,3 @@
+				if (scheme == "direct") {
+					break;
+				} else if (scheme == "http" || scheme == "https") {

Couldn't the user have different proxies for http and https?  Should this be checked against the URI?

In fact, maybe the first thing to do should be to extract the scheme from the URI, and then look for a libproxy proxy set up for that scheme?

@@ +44,3 @@
+					WebProxy webProxy = new WebProxy();
+					
+					if (UseAuthentication()) {

whitespace around (

@@ +46,3 @@
+					if (UseAuthentication()) {
+						ICredentials credentials = 
+							new NetworkCredential(GetAuthUser(), GetAuthPass());

whitespace around (

(In reply to comment #2)
> Review of attachment 163334 [details] [review]:
> 
> Overall looks good.  Some minor whitespace issues, and one question regarding
> scheme checking (keep in mind I know nothing about proxies, have never really
> used them).
> 
> @@ +41,3 @@
> +                if (scheme == "direct") {
> +                    break;
> +                } else if (scheme == "http" || scheme == "https") {
> 
> Couldn't the user have different proxies for http and https?  Should this be
> checked against the URI?
> 
> In fact, maybe the first thing to do should be to extract the scheme from the
> URI, and then look for a libproxy proxy set up for that scheme?

Hi, thanks for the review. I'm attaching a second patch with the suggested corrections, and I'm sorry for the style mismatches; my brain is kinda hardcoded for Python whitespace style rules :-)

Regarding proxies and uris: Yes, the user may have different proxies for http and https, and the system dependent selection of those proxies is handled by libproxy when pf.GetProxies is called with the "destination uri" that needs to be accessed. This means that a http proxy can be setup to reach https uris, and viceversa.

The return value of GetProxies is a list of some "proxy uris" that are able to reach the "destination uri" that needs to be accessed; it's over that list of proxy uris that this code goes over, trying to find a proxy uri whose scheme is "direct" (ie, no proxy) or one that WebProxy can handle (http or https). If the proxy uri has some other scheme (let's say socks4), the code has to skip it.

Created attachment 164252 [details] [review]
Requested fixes

Thanks for taking the time to fix this, Alejandro. :-)  Great work!

(In reply to comment #0)

> According to http://code.google.com/p/libproxy/wiki/Roadmap getting the proxy
> credentials is a feature planned for the next version (0.5) of libproxy.

Hi,

A short 'notice' on the 'getting proxy credentials': libproxy DOES get the credentials from gconf since version 0.4.0. Just 'setting' them is not supported yet.

0.4.0 has been released on Feb 25 2010.

libproxy is crashing tomboy at least on linux:

 * https://bugzilla.gnome.org/show_bug.cgi?id=647701
 * http://bugs.sabayon.org/show_bug.cgi?id=2404

tested up to 1.7.2 here.