[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