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

New method: HttpRequest.GetFullUrl #341

Closed
muratg opened this issue Jul 6, 2015 · 9 comments
Closed

New method: HttpRequest.GetFullUrl #341

muratg opened this issue Jul 6, 2015 · 9 comments

Comments

@muratg
Copy link

muratg commented Jul 6, 2015

Add a new extension method HttpRequest (GetFullUrl) that returns a string.

  • GetEncodedUrl
  • GetDecodedUrl
@muratg muratg added this to the 1.0.0-beta6 milestone Jul 6, 2015
@Tratcher
Copy link
Member

Tratcher commented Jul 8, 2015

We've won't-fixed this request a few times already. What's different now?
#110, #308

@davidfowl
Copy link
Member

  1. It's commonly asked for
  2. It's a string so we avoid all of the Uri problems with round tripping

@Tratcher
Copy link
Member

Tratcher commented Jul 8, 2015

No, we still have many of the round tripping problems. What format should all of the components be in? Human readable or encoded for use in headers? I suspect you need at least two methods.

@davidfowl
Copy link
Member

Not sure why we'd have roundtripping problems. It's just a concat of the parts of the URL that we already have

@Tratcher
Copy link
Member

Tratcher commented Jul 8, 2015

The values have already been processed by multiple layers and are in different formats. e.g. Paths are unescaped, query is still escaped, who knows what state the Host is in, etc.. Just concatenating them back together gives you a Frankenstein url. You have to consider both the current format of the data and the desired format/usage of the result.

This is one of the reasons we haven't done this. The only reasonable request I've heard for the full url so far was for logging. What other usages have been requested? Almost everybody that has asked for this ends up just wanting the pieces, not the full url.

@borgdylan
Copy link

Would this include the query string as well? While developing I print each request path to the console to make sure stuff gets called by JS. Having the query string as well would be nice.

@TomGroeneboer
Copy link

I think you can get the querystring with HttpContext.GetFeature<IHttpRequestFeature>().QueryString https://github.com/aspnet/HttpAbstractions/blob/dev/src/Microsoft.AspNet.Http.Features/IHttpRequestFeature.cs

@Tratcher
Copy link
Member

Tratcher commented Jul 9, 2015

@TomGroeneboer You can get it from HttpContext.Request.QueryString.

@Eilon
Copy link
Contributor

Eilon commented Aug 4, 2015

We should re-discuss the names of these. Those the current names are technically correct, I don't believe they will lead to correct usage. The doc comments explain what's what, but I'm sure they'll be often ignored. I recommend perhaps changing GetDecodedUrl() to be something more like GetDisplayUrl(), or perhaps even getting rid of it entirely.

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

8 participants