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

Avoid double escaping in PathString #809

Merged
merged 1 commit into from
Apr 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,20 @@ public static bool IsValidPathChar(char c)
{
return c < ValidPathChars.Length && ValidPathChars[c];
}

public static bool IsPercentEncodedChar(string str, int index)
{
return index < str.Length - 2
&& str[index] == '%'
&& IsHexadecimalChar(str[index + 1])
&& IsHexadecimalChar(str[index + 2]);
}

public static bool IsHexadecimalChar(char c)
{
return ('0' <= c && c <= '9')
|| ('A' <= c && c <= 'F')
|| ('a' <= c && c <= 'f');
}
}
}
26 changes: 20 additions & 6 deletions src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public bool HasValue
}

/// <summary>
/// Provides the path string escaped in a way which is correct for combining into the URI representation.
/// Provides the path string escaped in a way which is correct for combining into the URI representation.
/// </summary>
/// <returns>The escaped path value</returns>
public override string ToString()
Expand All @@ -77,9 +77,12 @@ public string ToUriComponent()
var start = 0;
var count = 0;
var requiresEscaping = false;
for (int i = 0; i < _value.Length; ++i)
var i = 0;

while (i < _value.Length)
{
if (PathStringHelper.IsValidPathChar(_value[i]))
var isPercentEncodedChar = PathStringHelper.IsPercentEncodedChar(_value, i);
if (PathStringHelper.IsValidPathChar(_value[i]) || isPercentEncodedChar)
{
if (requiresEscaping)
{
Expand All @@ -96,7 +99,16 @@ public string ToUriComponent()
count = 0;
}

count++;
if (isPercentEncodedChar)
{
count += 3;
i += 3;
}
else
{
count++;
i++;
}
}
else
{
Expand All @@ -116,6 +128,7 @@ public string ToUriComponent()
}

count++;
i++;
}
}

Expand Down Expand Up @@ -146,6 +159,7 @@ public string ToUriComponent()
}
}


/// <summary>
/// Returns an PathString given the path as it is escaped in the URI format. The string MUST NOT contain any
/// value that is not a path.
Expand Down Expand Up @@ -279,7 +293,7 @@ public bool StartsWithSegments(PathString other, StringComparison comparisonType
}

/// <summary>
/// Adds two PathString instances into a combined PathString value.
/// Adds two PathString instances into a combined PathString value.
/// </summary>
/// <returns>The combined PathString value</returns>
public PathString Add(PathString other)
Expand All @@ -297,7 +311,7 @@ public PathString Add(PathString other)
}

/// <summary>
/// Combines a PathString and QueryString into the joined URI formatted string value.
/// Combines a PathString and QueryString into the joined URI formatted string value.
/// </summary>
/// <returns>The joined URI formatted string value</returns>
public string Add(QueryString other)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ public void StartsWithSegmentsWithRemainder_DoesMatchUsingSpecifiedComparison(st
[InlineData("pct-encoding", "/单行道", "/%E5%8D%95%E8%A1%8C%E9%81%93")]
[InlineData("mixed1", "/index/单行道=(x*y)[abc]", "/index/%E5%8D%95%E8%A1%8C%E9%81%93=(x*y)%5Babc%5D")]
[InlineData("mixed2", "/index/单行道=(x*y)[abc]_", "/index/%E5%8D%95%E8%A1%8C%E9%81%93=(x*y)%5Babc%5D_")]
[InlineData("encoded", "/http%3a%2f%2f[foo]%3A5000/", "/http%3a%2f%2f%5Bfoo%5D%3A5000/")]
Copy link
Member

Choose a reason for hiding this comment

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

Add tests for strings that end with %, %A, and %AB.

[InlineData("encoded", "/http%3a%2f%2f[foo]%3A5000/%", "/http%3a%2f%2f%5Bfoo%5D%3A5000/%25")]
[InlineData("encoded", "/http%3a%2f%2f[foo]%3A5000/%2", "/http%3a%2f%2f%5Bfoo%5D%3A5000/%252")]
[InlineData("encoded", "/http%3a%2f%2f[foo]%3A5000/%2F", "/http%3a%2f%2f%5Bfoo%5D%3A5000/%2F")]
public void ToUriComponentEscapeCorrectly(string category, string input, string expected)
{
var path = new PathString(input);
Expand Down