[Mono-dev] Patch for HttpRequest.cs

[Juraj Skripsky]

Hi,

Sorry for taking so long to get back to you. A new patch is attached
which does the following:

- get_RootVirtualDir: small cleanup and optimization. get_RootVirtualDir
and get_BaseVirtualDir are almost identical, so let the former use the
later.
- get_QueryString: using ContentEncoding when UrlDecoding
- url_components: renamed from uri_builder
- UrlComponent: new private property. Its getter does the job of former
method InitUriBuilder(). Allows to eliminate most of the ugly
"uri_builder == null" checks in HttpRequest.
- UrlCompontent.Query is initialized as suggested by Kornél (using
HttpWorker.{GetQueryStringRawBytes,GetQueryString},
HttpRequest.ContentEncoding).

There is one test case failure after applying the patch.

Test "U2" in "Test_PropertiesSimpleConstructor ()":
---------------------------------------------------
string url = "http://www.gnome.org/";
string qs = "key=value&key2=value%32second";
HttpRequest r = new HttpRequest ("file", url, qs);
Assert.AreEqual (url, r.Url.ToString (), "U2");

Result:
-------
expected:<"http://www.gnome.org/">
but was:<"http://www.gnome.org/?key=value&key2=value2second"

I consider this a bug in MS.NET, "r.Url.ToString ()" should be returning
the url including the query string at this point. Are there any known
cases where code depends on the presence of this bug? What should we do
about it?

- Juraj


On Mon, 2006-05-08 at 12:57 +0200, Kornél Pál wrote: 
> Hi,
> 
> You are wrong. HttpRequest.QueryString does the following on MS.NET:
> 
> The only encoding it uses is HttpRequest.ContentEncoding. It tries to obtain 
> HttpWorkerRequest.GetQueryStringRawBytes(). If it fails then falls back to 
> HttpWorkerRequest.GetQueryString(). When it was able to obtain the byte 
> array it will decode it using HttpRequest.ContentEncoding.GetString(). As 
> such query string is decoded correctly. When no byte array is available in 
> HttpWorkerRequest or the query string was set either in constructor or using 
> HttpContext.RewritePath for example the string is assumed to be decoded 
> correctly so no decoding is done.
> 
> Now we have a string that still may be URL encoded. MS.NET probably calls 
> HttpUtility.UrlDecode just like we do but MS.NET passes 
> HttpRequest.ContentEncoding as well because query string is assumed to be 
> URL encoded using that encoding.
> 
> Note that obtaining query string from HttpWorkerRequest in the constructor 
> as we currently do is a wrong implementation as HttpRequest.ContentEncoding 
> can be changed before HttpRequest.QueryString is first accessed.
> 
> We should do the following:
> - delay query string processing until it is needed (don't obtain query 
> string in the constructor)
> - try HttpWorkerRequest.GetQueryStringRawBytes() as well
> - use HttpRequest.ContentEncoding to decode the byte array and for 
> HttpUtility.UrlDecode
> 
> Kornél
> 
> ----- Original Message ----- 
> From: "Juraj Skripsky" <js at hotfeet.ch>
> To: "Miguel de Icaza" <miguel at ximian.com>
> Cc: <mono-devel-list at lists.ximian.com>
> Sent: Monday, May 08, 2006 12:22 PM
> Subject: Re: [Mono-dev] Patch for HttpRequest.cs
> 
> 
> > Hello,
> >
> > After running more tests, I've found out that on MS.NET the decoding in
> > HttpRequest.QueryString does _not_ depend on
> > HttpRequest.ContentEncoding. In fact, MS seems to be always using Latin1
> > here. All other standard encodings fail.
> >
> > A revised patch is attached, including a NUnit test case. If no one
> > objects, I'll commit.
> >
> > - Juraj
> >
> >
> > On Sat, 2006-05-06 at 13:47 -0400, Miguel de Icaza wrote:
> >> Hello Juraj,
> >>
> >> > The attached patch makes sure that the get-parameters in QueryString 
> >> > are
> >> > url-decoded using the proper encoding (when creating the
> >> > NameValueCollection).
> >> >
> >> > May I commit?
> >>
> >> Could you provide NUnit tests for this case?
> >>
> >> Miguel
> >>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: HttpRequest.patch
Type: text/x-patch
Size: 7107 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20060526/9cecacaa/attachment.bin