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

Commit 150b470

Browse files
troydaitroydai
committed
Fix PathString over-encoding
Base on RFC 3986, ensure following characters are not encoded alpha, digit, "-", "_", ".", "~", "@", ":", "/", "!", "$", ";", "=", "'", "(", ")", "*", "+", ","
1 parent 748e96f commit 150b470

File tree

3 files changed

+119
-12
lines changed

3 files changed

+119
-12
lines changed

‎src/Microsoft.AspNetCore.Http.Abstractions/Internal/PathStringHelper.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
namespace Microsoft.AspNetCore.Http.Internal
5+
{
6+
internal class PathStringHelper
7+
{
8+
private static bool[] ValidPathChars = {
9+
false, false, false, false, false, false, false, false, // 0x00 - 0x07
10+
false, false, false, false, false, false, false, false, // 0x08 - 0x0F
11+
false, false, false, false, false, false, false, false, // 0x10 - 0x17
12+
false, false, false, false, false, false, false, false, // 0x18 - 0x1F
13+
false, true, false, false, true, false, true, true, // 0x20 - 0x27
14+
true, true, true, true, true, true, true, true, // 0x28 - 0x2F
15+
true, true, true, true, true, true, true, true, // 0x30 - 0x37
16+
true, true, true, true, false, true, false, false, // 0x38 - 0x3F
17+
true, true, true, true, true, true, true, true, // 0x40 - 0x47
18+
true, true, true, true, true, true, true, true, // 0x48 - 0x4F
19+
true, true, true, true, true, true, true, true, // 0x50 - 0x57
20+
true, true, true, false, false, false, false, true, // 0x58 - 0x5F
21+
false, true, true, true, true, true, true, true, // 0x60 - 0x67
22+
true, true, true, true, true, true, true, true, // 0x68 - 0x6F
23+
true, true, true, true, true, true, true, true, // 0x70 - 0x77
24+
true, true, true, false, false, false, true, false, // 0x78 - 0x7F
25+
};
26+
27+
public static bool IsValidPathChar(char c)
28+
{
29+
return c < ValidPathChars.Length && ValidPathChars[c];
30+
}
31+
}
32+
}

‎src/Microsoft.AspNetCore.Http.Abstractions/PathString.cs

Lines changed: 71 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5-
using System.Text.Encodings.Web;
5+
using System.Text;
66
using Microsoft.AspNetCore.Http.Abstractions;
7+
using Microsoft.AspNetCore.Http.Internal;
78

89
namespace Microsoft.AspNetCore.Http
910
{
@@ -66,25 +67,83 @@ public override string ToString()
6667
/// <returns>The escaped path value</returns>
6768
public string ToUriComponent()
6869
{
69-
// TODO: Measure the cost of this escaping and consider optimizing.
7070
if (!HasValue)
7171
{
7272
return string.Empty;
7373
}
74-
var values = _value.Split(splitChar);
75-
var changed = false;
76-
for (var i = 0; i < values.Length; i++)
77-
{
78-
var value = values[i];
79-
values[i] = UrlEncoder.Default.Encode(value);
8074

81-
if (!changed && value != values[i])
75+
StringBuilder buffer = null;
76+
77+
var start = 0;
78+
var count = 0;
79+
var requiresEscaping = false;
80+
for (int i = 0; i < _value.Length; ++i)
81+
{
82+
if (PathStringHelper.IsValidPathChar(_value[i]))
8283
{
83-
changed = true;
84+
if (requiresEscaping)
85+
{
86+
// the current segment requires escape
87+
if (buffer == null)
88+
{
89+
buffer = new StringBuilder(_value.Length * 3);
90+
}
91+
92+
buffer.Append(Uri.EscapeDataString(_value.Substring(start, count)));
93+
94+
requiresEscaping = false;
95+
start = i;
96+
count = 0;
97+
}
98+
99+
count++;
84100
}
101+
else
102+
{
103+
if (!requiresEscaping)
104+
{
105+
// the current segument doesn't require escape
106+
if (buffer == null)
107+
{
108+
buffer = new StringBuilder(_value.Length * 3);
109+
}
110+
111+
buffer.Append(_value, start, count);
112+
113+
requiresEscaping = true;
114+
start = i;
115+
count = 0;
116+
}
117+
118+
count++;
119+
}
120+
}
121+
122+
if (count == _value.Length && !requiresEscaping)
123+
{
124+
return _value;
85125
}
126+
else
127+
{
128+
if (count > 0)
129+
{
130+
if (buffer == null)
131+
{
132+
buffer = new StringBuilder(_value.Length * 3);
133+
}
134+
135+
if (requiresEscaping)
136+
{
137+
buffer.Append(Uri.EscapeDataString(_value.Substring(start, count)));
138+
}
139+
else
140+
{
141+
buffer.Append(_value, start, count);
142+
}
143+
}
86144

87-
return changed ? string.Join("/", values) : _value;
145+
return buffer.ToString();
146+
}
88147
}
89148

90149
/// <summary>
@@ -335,7 +394,7 @@ public static implicit operator PathString(string s)
335394
/// Implicitly calls ToString().
336395
/// </summary>
337396
/// <param name="path"></param>
338-
public static implicit operator string (PathString path)
397+
public static implicit operator string(PathString path)
339398
{
340399
return path.ToString();
341400
}

‎test/Microsoft.AspNetCore.Http.Abstractions.Tests/PathStringTests.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,5 +185,21 @@ public void StartsWithSegmentsWithRemainder_DoesMatchUsingSpecifiedComparison(st
185185

186186
Assert.Equal(expectedResult, result);
187187
}
188+
189+
[Theory]
190+
[InlineData("unreserved", "/abc123.-_~", "/abc123.-_~")]
191+
[InlineData("colon", "/:", "/:")]
192+
[InlineData("at", "/@", "/@")]
193+
[InlineData("sub-delims", "/!$&'()*+,;=", "/!$&'()*+,;=")]
194+
[InlineData("reserved", "/?#[]", "/%3F%23%5B%5D")]
195+
[InlineData("pct-encoding", "/单行道", "/%E5%8D%95%E8%A1%8C%E9%81%93")]
196+
[InlineData("mixed1", "/index/单行道=(x*y)[abc]", "/index/%E5%8D%95%E8%A1%8C%E9%81%93=(x*y)%5Babc%5D")]
197+
[InlineData("mixed2", "/index/单行道=(x*y)[abc]_", "/index/%E5%8D%95%E8%A1%8C%E9%81%93=(x*y)%5Babc%5D_")]
198+
public void ToUriComponentEscapeCorrectly(string category, string input, string expected)
199+
{
200+
var path = new PathString(input);
201+
202+
Assert.Equal(expected, path.ToUriComponent());
203+
}
188204
}
189205
}

0 commit comments

Comments
 (0)