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

Add HttpReqeuest GetEncodedUrl and GetDecodedUrl extensions. #359

Merged
merged 1 commit into from
Aug 4, 2015

Conversation

Tratcher
Copy link
Member

@Tratcher Tratcher commented Aug 3, 2015

/// <returns></returns>
public static string GetDecodedUrl(this HttpRequest request)
{
return request.Scheme + "://" + request.Host.Value + request.PathBase.Value + request.Path.Value + request.QueryString.Value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does all this concatenation work when some sections are missing? E.g. no query string or empty path etc.?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Added tests for empty params.

@Eilon
Copy link
Contributor

Eilon commented Aug 4, 2015

Also commented in the bug:

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.

@Tratcher
Copy link
Member Author

Tratcher commented Aug 4, 2015

Renamed GetDecodedUrl to GetDisplayUrl.

@@ -71,5 +71,27 @@ public static string Encode(Uri uri)
return uri.GetComponents(UriComponents.SerializationInfoString, UriFormat.UriEscaped);
}
}

/// <summary>
/// Returns the combine components of the request URL in a fully escaped form suitable for use in HTTP headers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

combined

@Eilon
Copy link
Contributor

Eilon commented Aug 4, 2015

:shipit: after the comment fix.

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

Successfully merging this pull request may close these issues.

3 participants