Bug 4135 - HttpRequest.Path is unenescaped twice from the raw URL that came over the wire
Summary: HttpRequest.Path is unenescaped twice from the raw URL that came over the wire
Status: RESOLVED FIXED
Alias: None
Product: Class Libraries
Classification: Mono
Component: System.Web ()
Version: master
Hardware: All Linux
: --- normal
Target Milestone: Untriaged
Assignee: Marek Habersack
URL:
Depends on:
Blocks:
 
Reported: 2012-03-28 17:05 UTC by Jared Watts
Modified: 2012-04-11 06:19 UTC (History)
2 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Jared Watts 2012-03-28 17:05:38 UTC
We are using apache/mod-mono/XSP.  When a URL comes into apache, it gets unescaped twice as demonstrated by the Request.Path property.  Example:

Requested URL = /File/101497/1L%25001!.WPD?format=bin
Request.RawUrl = /File/101497/3L%001!.WPD?format=bin
Request.Path = /File/101497/3L^@1!.WPD

The result is an unhandled ArgumentException from the following stack trace because the %00 gets unescaped a second time, resulting in a NUL (0x00) character in the path:
System.ArgumentException: Illegal characters in path.
  at System.IO.Path.Combine (System.String path1, System.String path2) [0x00000] in <filename unknown>:0
  at Mono.WebServer.MonoWorkerRequest.MapPath (System.String path) [0x00000] in <filename unknown>:0
  at System.Web.HttpRequest.get_PhysicalPath () [0x00000] in <filename unknown>:0
  at System.Web.ServerVariablesCollection.loadServerVariablesCollection () [0x00000] in <filename unknown>:0
  at System.Web.ServerVariablesCollection.InsertInfo () [0x00000] in <filename unknown>:0
  at System.Web.BaseParamsCollection.LoadInfo () [0x00000] in <filename unknown>:0
  at System.Web.BaseParamsCollection.get_Count () [0x00000] in <filename unknown>:0
  at System.Collections.Specialized.NameValueCollection.Add (System.Collections.Specialized.NameValueCollection c) [0x00000] in <filename unknown>:0
  at System.Web.HttpParamsCollection.MergeCollections () [0x00000] in <filename unknown>:0
  at System.Web.HttpParamsCollection.Get (System.String name) [0x00000] in <filename unknown>:0
  at System.Collections.Specialized.NameValueCollection.get_Item (System.String name) [0x00000] in <filename unknown>:0
  at Symform.Control.Web.ControlApplication+OverlayIdModelBinder.BindModel (System.Web.Mvc.ControllerContext controllerContext, System.Web.Mvc.ModelBindingContext bindingContext) [0x00000] in <filename unknown>:0
  at System.Web.Mvc.ControllerActionInvoker.GetParameterValue (System.Web.Mvc.ControllerContext controllerContext, System.Web.Mvc.ParameterDescriptor parameterDescriptor) [0x00000] in <filename unknown>:0
  at System.Web.Mvc.ControllerActionInvoker.GetParameterValues (System.Web.Mvc.ControllerContext controllerContext, System.Web.Mvc.ActionDescriptor actionDescriptor) [0x00000] in <filename unknown>:0
  at System.Web.Mvc.ControllerActionInvoker.InvokeAction (System.Web.Mvc.ControllerContext controllerContext, System.String actionName) [0x00000] in <filename unknown>:0
  
From the debugging I have done, it appears that the following sequence of actions occurs:
- mod_mono::send_initial_data passes along request_rec->uri (already unescaped by Apache) to ModMonoRequest.GetInitialData
- The ModMonoWorkerRequest is initialized with this unescaped uri
- The HttpRuntime initializes the HttpContext and HttpRequest
- When HttpRequest.Path is called, the uri is unescaped a second time by: unescaped_path = Uri.UnescapeDataString (PathNoValidation);

Perhaps mod-mono, during send_initial_data, should be using the request_rec->unparsed_uri field instead or request_rec->uri? This field is the original raw URL that has not been unescaped yet.  Then the System.Web runtime can do the unescaping a single time.  I'm not sure of other possible side effects of that change.

Either way, I believe unescaping of the incoming raw URL should be happening just once.  Thanks!

mod-mono version:
Mon, 9 Jan 2012 15:00:54 -0500
056c91ddbbbbd0cead3e93016c251b37f873ec7e

XSP version:
Wed, 14 Mar 2012 13:20:17 -0700
c0a69ec7b4bcb5ff82af4b4854115e9a6f0a0ceb
Comment 1 Jared Watts 2012-03-29 18:17:17 UTC
I modified mod_mono.c::send_initial_data to send the path portion of the request_rec=>unparsed_uri to XSP.  This resulted in a 404 because the path used by the application has no unescaping performed at all.  Therefore, it looks like the "double unescaping" only happens for the HttpRequest.Path property.

Interestingly, I see a CHANGELOG note for HttpRequest about this:

2007-01-04  Vladimir Krasnov  <vladimirkmainsoft.com>
  * HttpRequest.cs: fixed Path property, add call of
      Uri.UnescapeDataString in net_2_0 

Why does HttpRequest.Path need to be unescaped again, after the raw URL has already been unescaped before the HttpRequest gets created?
Comment 2 Jared Watts 2012-04-10 16:46:13 UTC
This is the particular change where the HttpRequest.Path property started being unescapaed:
https://github.com/mono/mono/commit/dbc7b5c86c9c7d4001e28d5c63f1c774ec0a64a0
Comment 3 Marek Habersack 2012-04-11 06:19:06 UTC
It appears that, indeed, the unescape in Path getter is not required. The request worker unescapes the request URI before handing it over to the runtime.

Fix is in git, commit 47f4be4435a6e526c74b1241665d7518bd2596b7