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

Commit

Permalink
Fix PathString over-encoding
Browse files Browse the repository at this point in the history 
Base on RFC 3986, ensure following characters are not encoded

alpha, digit, "-", "_", ".", "~", "@", ":", "/", "!", "$", ";", "=",
"'", "(", ")", "*", "+", ","
  • Loading branch information
@troydai
troydai committed Jul 8, 2016
1 parent 748e96f commit 150b470
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 12 deletions.
32 changes: 32 additions & 0 deletions src/Microsoft.AspNetCore.Http.Abstractions/Internal/PathStringHelper.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.AspNetCore.Http.Internal
{
internal class PathStringHelper
{
private static bool[] ValidPathChars = {
false, false, false, false, false, false, false, false, // 0x00 - 0x07
false, false, false, false, false, false, false, false, // 0x08 - 0x0F
false, false, false, false, false, false, false, false, // 0x10 - 0x17
false, false, false, false, false, false, false, false, // 0x18 - 0x1F
false, true, false, false, true, false, true, true, // 0x20 - 0x27
true, true, true, true, true, true, true, true, // 0x28 - 0x2F
true, true, true, true, true, true, true, true, // 0x30 - 0x37
true, true, true, true, false, true, false, false, // 0x38 - 0x3F
true, true, true, true, true, true, true, true, // 0x40 - 0x47
true, true, true, true, true, true, true, true, // 0x48 - 0x4F
true, true, true, true, true, true, true, true, // 0x50 - 0x57
true, true, true, false, false, false, false, true, // 0x58 - 0x5F
false, true, true, true, true, true, true, true, // 0x60 - 0x67
true, true, true, true, true, true, true, true, // 0x68 - 0x6F
true, true, true, true, true, true, true, true, // 0x70 - 0x77
true, true, true, false, false, false, true, false, // 0x78 - 0x7F
};

public static bool IsValidPathChar(char c)
{
return c < ValidPathChars.Length && ValidPathChars[c];
}
}
}
83 changes: 71 additions & 12 deletions src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Text.Encodings.Web;
using System.Text;
using Microsoft.AspNetCore.Http.Abstractions;
using Microsoft.AspNetCore.Http.Internal;

namespace Microsoft.AspNetCore.Http
{
Expand Down Expand Up @@ -66,25 +67,83 @@ public override string ToString()
/// <returns>The escaped path value</returns>
public string ToUriComponent()
{
// TODO: Measure the cost of this escaping and consider optimizing.
if (!HasValue)
{
return string.Empty;
}
var values = _value.Split(splitChar);
var changed = false;
for (var i = 0; i < values.Length; i++)
{
var value = values[i];
values[i] = UrlEncoder.Default.Encode(value);

if (!changed && value != values[i])
StringBuilder buffer = null;

var start = 0;
var count = 0;
var requiresEscaping = false;
for (int i = 0; i < _value.Length; ++i)
{
if (PathStringHelper.IsValidPathChar(_value[i]))
{
changed = true;
if (requiresEscaping)
{
// the current segment requires escape
if (buffer == null)
{
buffer = new StringBuilder(_value.Length * 3);
}

buffer.Append(Uri.EscapeDataString(_value.Substring(start, count)));

requiresEscaping = false;
start = i;
count = 0;
}

count++;
}
else
{
if (!requiresEscaping)
{
// the current segument doesn't require escape
if (buffer == null)
{
buffer = new StringBuilder(_value.Length * 3);
}

buffer.Append(_value, start, count);

requiresEscaping = true;
start = i;
count = 0;
}

count++;
}
}

if (count == _value.Length && !requiresEscaping)
{
return _value;
}
else
{
if (count > 0)
{
if (buffer == null)
{
buffer = new StringBuilder(_value.Length * 3);
}

if (requiresEscaping)
{
buffer.Append(Uri.EscapeDataString(_value.Substring(start, count)));
}
else
{
buffer.Append(_value, start, count);
}
}

return changed ? string.Join("/", values) : _value;
return buffer.ToString();
}
}

/// <summary>
Expand Down Expand Up @@ -335,7 +394,7 @@ public static implicit operator PathString(string s)
/// Implicitly calls ToString().
/// </summary>
/// <param name="path"></param>
public static implicit operator string (PathString path)
public static implicit operator string(PathString path)
{
return path.ToString();
}
Expand Down
16 changes: 16 additions & 0 deletions test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,5 +185,21 @@ public void StartsWithSegmentsWithRemainder_DoesMatchUsingSpecifiedComparison(st

Assert.Equal(expectedResult, result);
}

[Theory]
[InlineData("unreserved", "/abc123.-_~", "/abc123.-_~")]
[InlineData("colon", "/:", "/:")]
[InlineData("at", "/@", "/@")]
[InlineData("sub-delims", "/!$&'()*+,;=", "/!$&'()*+,;=")]
[InlineData("reserved", "/?#[]", "/%3F%23%5B%5D")]
[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_")]
public void ToUriComponentEscapeCorrectly(string category, string input, string expected)
{
var path = new PathString(input);

Assert.Equal(expected, path.ToUriComponent());
}
}
}

0 comments on commit 150b470

Please sign in to comment.