Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Route based parameter binding doesn't decode %2F #6388

Closed
Drawaes opened this issue Jun 10, 2017 · 8 comments
Closed

Route based parameter binding doesn't decode %2F #6388

Drawaes opened this issue Jun 10, 2017 · 8 comments

Comments

@Drawaes
Copy link

Drawaes commented Jun 10, 2017

This is a big problem for us, we are going to have to change our entire webapi's to use query based or to put a manual decode step in everyone "in case". I understand there were previous requests for Kestrel to do this but it couldn't due to IIS/Http.sys and that is fine.

But we want to(actually do) use Kestrel internally without anything infront and with ASP.NET 2.0 we are hoping to be able to externally as well.

I have included a repo to show the problem with

http://localhost:5000/api/ExpectedUrl/my%20test%5Curl%2F
returning : my test\url/

Which is what I would expect, and this url

http://localhost:5000/api/DefaultUrl/my%20test%5Curl%2F
returing : my test\url%2F

The difference my "expected" method has

return WebUtility.UrlDecode(someValue);

I am hoping you will tell me there is a "switch" I can flick to solve this across our entire application (I understand, well I don't but if you don't want it on by default that is okay)

https://github.com/Drawaes/ASPNETUrlDecode

@rynowak
Copy link
Member

rynowak commented Jun 11, 2017

@Tratcher - what's the story here? Routing doesn't do any decoding, only encoding

@Drawaes
Copy link
Author

Drawaes commented Jun 11, 2017

Then model binding should. If it's done at the kestrel level it won't work because it will split the parameters

@Tratcher
Copy link
Member

Pretty sure there's already a bug on this somewhere.

IIS/Http.Sys over aggressively un-escape a few characters like forward and backslashes. For Kestrel and HttpSysServer we have stopped doing this. These values can't be unescaped until after the path is split by forward slashes. Not sure if that belongs to routing or model binding, but yes it would be OK to unescape forward and backslashes in the individual path segments.

@Drawaes
Copy link
Author

Drawaes commented Jun 12, 2017

There is a bug at the Kestrel level
aspnet/KestrelHttpServer#1425

But this is not the issue, I think it should be left encoded at the Kestrel level. But if that is the case I think we need a spot to decode on the model binding on MVC (that is why it's in the MVC area) otherwise you need to decode inside every method, you can't decode as middleware because then the router will have the wrong path

@rynowak
Copy link
Member

rynowak commented Jun 12, 2017

OK, maybe this is something that routing needs to do then since in the past it was done by servers. This might have an impact on a few other issues we'd been discussing such as making route values containing a / round-trippable.

@Eilon thoughts?

@pranavkm
Copy link
Contributor

Related bug - #4599. It boiled down to not knowing if the server \ previous middleware had already decoded the value.

@Drawaes
Copy link
Author

Drawaes commented Jun 20, 2017

Fine then don't do it for spaces... it's just crazy to half do it

@Eilon
Copy link
Member

Eilon commented Jul 5, 2017

Closing because there are no plans to implement this. For this particular case you could write custom middleware that "corrects" the data per the requirements of your app. The middleware can detect what it's running behind and change the values of the HttpRequest properties. Then by the time MVC runs it'll see the data however you want.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants