Skip to content
This repository was archived by the owner on Nov 20, 2018. It is now read-only.

Add raw request URI property to IHttpRequestFeature #596

Closed
cesarblum opened this issue Mar 24, 2016 · 9 comments
Closed

Add raw request URI property to IHttpRequestFeature #596

cesarblum opened this issue Mar 24, 2016 · 9 comments
Assignees
Milestone

Comments

@cesarblum
Copy link
Contributor

Path and PathBase might lose some information. Expose the raw, unaltered request URI via the new property.

Related: aspnet/KestrelHttpServer#594

This will also mitigate the loss of information in aspnet/KestrelHttpServer#709 and aspnet/KestrelHttpServer#666.

@cesarblum cesarblum self-assigned this Mar 24, 2016
@Eilon
Copy link
Member

Eilon commented Mar 25, 2016

@blowdart because we always freak out with regard to security and so-called "raw" URLs. I'm personally a fan of giving raw access to data, but URLs can be dangerous unless you really, really, really know what you're doing.

@Tratcher
Copy link
Member

Yes, sufficiently so that I'd considered making it its own feature, but that seem a little excessive.

@Eilon
Copy link
Member

Eilon commented Mar 25, 2016

I should add a corollary to my earlier comment:

It is a well-known fact that when it comes to URLs, no one knows what they're doing.

@blowdart
Copy link
Member

I have no real objection to them being exposed. However we mustn't use them anywhere ourselves. We must always operate on the canonicialized string.

The doc comments should reflect the danger of using URLs in their RAW forms.

@Tratcher
Copy link
Member

Can we add an attribute that makes using that field a compiler warning you need to suppress? Obsolete(Warning)?

@halter73
Copy link
Member

I think marking the Raw URL getter as deprecated/obsolete would be very misleading considering we have no plans to expose the same data in a newer/better API.

@Tratcher
Copy link
Member

I know Obsolete isn't ideal, but it's the only one I'm aware of that triggers a compiler warning. Are there others?

@Eilon
Copy link
Member

Eilon commented Mar 31, 2016

Please don't mark it as obsolete. There is no other attribute that does this. Either we add the property and it's a real property, or we don't add it at all.

@muratg
Copy link

muratg commented May 19, 2016

@CesarBS Let's do this in 1.0

cesarblum pushed a commit to aspnet/KestrelHttpServer that referenced this issue May 27, 2016
cesarblum pushed a commit to aspnet/KestrelHttpServer that referenced this issue May 31, 2016
cesarblum pushed a commit to aspnet/KestrelHttpServer that referenced this issue May 31, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants