[Mono-dev] [PATCH] Improve System.Net.WebClient's CreateUri(Uri address) query string handling

Jon Herron jon.herron at yahoo.com
Mon Mar 22 00:34:36 EDT 2010


Thanks for the commit, there still seems to be an issue when WebClient's QueryString property is set - it appears to be ignoring it if the address has a query string as well, instead of appending it to address' query string.  I've included a patch that works with my test case below, any feedback is welcome.


Index: ChangeLog
===================================================================
--- ChangeLog	(revision 153962)
+++ ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2010-03-22 Jon Herron <jon.herron at yahoo.com>
+
+	* WebClient.cs: fix for appending QueryString params to the address 
+	passed to CreateUri.
+
 2010-03-21 Gonzalo Paniagua Javier <gonzalo at novell.com>
 
 	* WebClient.cs: fix handling of query string, whether it comes from
Index: WebClient.cs
===================================================================
--- WebClient.cs	(revision 153962)
+++ WebClient.cs	(working copy)
@@ -739,7 +739,7 @@
 				return CreateUri (uri);
 			} catch {
 			}
-			return new Uri (Path.GetFullPath (address));
+			return CreateUri (new Uri (Path.GetFullPath (address)));
 		}
 
 		Uri CreateUri (Uri address)
@@ -753,24 +753,19 @@
 				}
 			}
 
-			string query = result.Query;
-			if (String.IsNullOrEmpty (query))
-				query = GetQueryString (true);
-			UriBuilder builder = new UriBuilder (address);
-			if (!String.IsNullOrEmpty (query))
-				builder.Query = query.Substring (1);
-			return builder.Uri;
+			return AppendQueryString (result);
 		}
 
-		string GetQueryString (bool add_qmark)
+		Uri AppendQueryString (Uri address)
 		{
 			if (queryString == null || queryString.Count == 0)
-				return null;
+				return address;
 
 			StringBuilder sb = new StringBuilder ();
-			if (add_qmark)
-				sb.Append ('?');
 
+			if (address.Query != null && address.Query.Length > 1)
+				sb.AppendFormat ("{0}&", address.Query.Substring (1));
+
 			foreach (string key in queryString)
 				sb.AppendFormat ("{0}={1}&", key, UrlEncode (queryString [key]));
 
@@ -778,9 +773,13 @@
 				sb.Length--; // removes last '&' or the '?' if empty.
 
 			if (sb.Length == 0)
-				return null;
+				return address;
 
-			return sb.ToString ();
+			UriBuilder builder = new UriBuilder (address);
+
+			builder.Query = sb.ToString ();
+
+			return builder.Uri;
 		}
 
 		WebRequest SetupRequest (Uri uri)


Thanks,

Jon

--- On Sun, 3/21/10, Gonzalo Paniagua Javier <gonzalo.mono at gmail.com> wrote:

> From: Gonzalo Paniagua Javier <gonzalo.mono at gmail.com>
> Subject: Re: [Mono-dev] [PATCH] Improve System.Net.WebClient's CreateUri(Uri address) query string handling
> To: "Jon Herron" <jon.herron at yahoo.com>
> Cc: mono-devel-list at lists.ximian.com
> Date: Sunday, March 21, 2010, 6:11 PM
> I have checked in a fix in trunk:
> http://lists.ximian.com/pipermail/mono-patches/2010-March/168726.html
> 
> It should fix the issues you are reporting. If you need
> this fix in the
> 2.6/2.4 branches, feel free to backport it.
> 
> Thanks.
> 
> -Gonzalo
> 
> On Sat, 2010-03-20 at 20:50 -0700, Jon Herron wrote:
> > With my patch calling DownloadString with either a
> string or Uri fails the same way under the scenario you
> describe, where as before they failed in different
> ways.  With the updated test:
> > 
> > using System;
> > using System.Collections.Specialized;
> > using System.Net;
> > 
> > public class TestWebClientBug
> > {
> >   public static void Main(string[]
> args)
> >   {
> >     NameValueCollection qs = new
> NameValueCollection();
> >     String url = "http://localhost/?var1=ok and more text
> also&var2=4&var3=caribou";
> >     WebClient wc = new
> WebClient();
> >     Uri uri = new Uri(url);
> > 
> >     qs.Add("qs_a", "1");
> >     qs.Add("qs_b", "some var");
> >     qs.Add("qs_c", "another");
> > 
> >     wc.QueryString = qs;
> > 
> >     wc.DownloadString(url);
> >     wc.DownloadString(uri);
> > 
>>    Console.WriteLine(uri.IsAbsoluteUri);
> >     Console.WriteLine(uri.Query);
>>    Console.WriteLine(uri.ToString());
> >   }
> > }
> > 
> > 2.6.1 yields:
> > 
> > 127.0.0.1 - - [20/Mar/2010:19:03:39 -0400] "GET
> /?var1=ok and more text
> also&var2=4&var3=caribou?qs_a=1&qs_b=some+var&qs_c=another
> HTTP/1.1" 200 3662
> > 127.0.0.1 - - [20/Mar/2010:19:03:39 -0400] "GET
> /?var1=ok and more text
> also&var2=4&var3=caribou?var1=ok%20and%20more%20text%20also&var2=4&var3=caribou
> HTTP/1.1" 200 3662
> > 
> > And trunk with my patch yields:
> > 
> > 127.0.0.1 - - [20/Mar/2010:19:04:03 -0400] "GET
> /?var1=ok and more text
> also&var2=4&var3=caribou?qs_a=1&qs_b=some+var&qs_c=another
> HTTP/1.1" 200 3662
> > 127.0.0.1 - - [20/Mar/2010:19:04:03 -0400] "GET
> /?var1=ok and more text
> also&var2=4&var3=caribou?qs_a=1&qs_b=some+var&qs_c=another
> HTTP/1.1" 200 3662
> > 
> > Granted the second question mark is a problem, but it
> is consistent - however one could argue a new bug is worse
> than an existing bug I suppose.  
> > 
> > I'll look at fixing the issue you describe as well,
> looks like it will need to be fixed for calling the
> CreateUri with a string or Uri.  Is it safe to assume
> that the patch won't get committed until this issue is
> resolved?
> > 
> > Jon
> > 
> > 
> > --- On Sat, 3/20/10, Gonzalo Paniagua Javier <gonzalo.mono at gmail.com>
> wrote:
> > 
> > > From: Gonzalo Paniagua Javier <gonzalo.mono at gmail.com>
> > > Subject: Re: [Mono-dev] [PATCH] Improve
> System.Net.WebClient's CreateUri(Uri address) query string
> handling
> > > To: mono-devel-list at lists.ximian.com
> > > Date: Saturday, March 20, 2010, 4:35 AM
> > > On Fri, 2010-03-19 at 21:08 -0700,
> > > Jon Herron wrote:
> > > > This patch fixes an issue I ran into when
> passing a
> > > Uri to WebClient's
> > > > DownloadString method that contains a query
> string -
> > > CreateUri would
> > > > re-append the query string to the end of
> the
> > > uri.  This makes
> > > > CreateUri work similar to MakeUri.  I
> didn't see
> > > a great way to make a
> > > > test for this, however for this sample app:
> > > > 
> > > > using System;
> > > > using System.Net;
> > > > 
> > > > public class TestWebClientBug
> > > > {
> > > >   public static void
> Main(string[]
> > > args)
> > > >   {
> > > >     String url = "http://localhost/?var1=ok and more text
> > > also&var2=4&var3=caribou";
> > > >     WebClient wc = new
> > > WebClient();
> > > >     Uri uri = new
> Uri(url);
> > > > 
> > > > 
>    wc.DownloadString(url);
> > > > 
>    wc.DownloadString(uri);
> > > >   }
> > > > }
> > > > 
> > > > In my access logs with mono 2.6.1 I see:
> > > > 
> > > > 127.0.0.1 - - [19/Mar/2010:19:50:11 -0400]
> "GET
> > >
> /?var1=ok%20and%20more%20text%20also&var2=4&var3=caribou
> > > HTTP/1.1" 200 3662
> > > > 127.0.0.1 - - [19/Mar/2010:19:50:11 -0400]
> "GET
> > > /?var1=ok and more text
> > >
> also&var2=4&var3=caribou?var1=ok%20and%20more%20text%20also&var2=4&var3=caribou
> > > HTTP/1.1" 200 3662
> > > > 
> > > > With this patch applied to trunk:
> > > > 
> > > > 127.0.0.1 - - [19/Mar/2010:19:50:33 -0400]
> "GET
> > >
> /?var1=ok%20and%20more%20text%20also&var2=4&var3=caribou
> > > HTTP/1.1" 200 3662
> > > > 127.0.0.1 - - [19/Mar/2010:19:50:33 -0400]
> "GET
> > >
> /?var1=ok%20and%20more%20text%20also&var2=4&var3=caribou
> > > HTTP/1.1" 200 3662
> > > 
> > > I'm afraid your patch might be hiding another
> issue: can
> > > you try your
> > > URL + setting some values in wc.QueryString? What
> the
> > > resulting url in
> > > that case?
> > > 
> > > -Gonzalo
> > > 
> > > 
> > > _______________________________________________
> > > Mono-devel-list mailing list
> > > Mono-devel-list at lists.ximian.com
> > > http://lists.ximian.com/mailman/listinfo/mono-devel-list
> > > 
> > 
> > 
> >       
> 
> 
> 


      


More information about the Mono-devel-list mailing list