Skip to content

Commit b5a97c4

Browse files
ladeakladeak
authored
Remove unsafe from PathNormalizer (#56805)
1 parent b41e166 commit b5a97c4

File tree

5 files changed

+231
-361
lines changed

5 files changed

+231
-361
lines changed

‎src/Servers/HttpSys/test/FunctionalTests/ResponseCachingTests.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,7 @@ public async Task Caching_SendFileNoContentLength_NotCached()
383383
}
384384
}
385385

386+
[QuarantinedTest("new issue")]
386387
[ConditionalFact]
387388
public async Task Caching_SendFileWithFullContentLength_Cached()
388389
{

‎src/Servers/Kestrel/Core/src/Internal/Http/PathNormalizer.cs

Lines changed: 90 additions & 170 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Diagnostics;
5+
using System.Runtime.InteropServices;
56
using System.Text;
67
using Microsoft.AspNetCore.Internal;
78
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
@@ -50,193 +51,112 @@ public static string DecodePath(Span<byte> path, bool pathEncoded, string rawTar
5051
}
5152

5253
// In-place implementation of the algorithm from https://tools.ietf.org/html/rfc3986#section-5.2.4
53-
public static unsafe int RemoveDotSegments(Span<byte> input)
54+
public static int RemoveDotSegments(Span<byte> src)
5455
{
55-
fixed (byte* start = input)
56-
{
57-
var end = start + input.Length;
58-
return RemoveDotSegments(start, end);
59-
}
60-
}
61-
62-
public static unsafe int RemoveDotSegments(byte* start, byte* end)
63-
{
64-
if (!ContainsDotSegments(start, end))
65-
{
66-
return (int)(end - start);
67-
}
56+
Debug.Assert(src[0] == '/', "Path segment must always start with a '/'");
57+
ReadOnlySpan<byte> dotSlash = "./"u8;
58+
ReadOnlySpan<byte> slashDot = "/."u8;
6859

69-
var src = start;
70-
var dst = start;
60+
var writtenLength = 0;
61+
var readPointer = 0;
7162

72-
while (src < end)
63+
while (src.Length > readPointer)
7364
{
74-
var ch1 = *src;
75-
Debug.Assert(ch1 == '/', "Path segment must always start with a '/'");
76-
77-
byte ch2, ch3, ch4;
78-
79-
switch (end - src)
65+
var currentSrc = src[readPointer..];
66+
var nextDotSegmentIndex = currentSrc.IndexOf(slashDot);
67+
if (nextDotSegmentIndex < 0 && readPointer == 0)
8068
{
81-
case 1:
82-
break;
83-
case 2:
84-
ch2 = *(src + 1);
85-
86-
if (ch2 == ByteDot)
87-
{
88-
// B. if the input buffer begins with a prefix of "/./" or "/.",
89-
// where "." is a complete path segment, then replace that
90-
// prefix with "/" in the input buffer; otherwise,
91-
src += 1;
92-
*src = ByteSlash;
93-
continue;
94-
}
95-
96-
break;
97-
case 3:
98-
ch2 = *(src + 1);
99-
ch3 = *(src + 2);
100-
101-
if (ch2 == ByteDot && ch3 == ByteDot)
102-
{
103-
// C. if the input buffer begins with a prefix of "/../" or "/..",
104-
// where ".." is a complete path segment, then replace that
105-
// prefix with "/" in the input buffer and remove the last
106-
// segment and its preceding "/" (if any) from the output
107-
// buffer; otherwise,
108-
src += 2;
109-
*src = ByteSlash;
110-
111-
if (dst > start)
112-
{
113-
do
114-
{
115-
dst--;
116-
} while (dst > start && *dst != ByteSlash);
117-
}
118-
119-
continue;
120-
}
121-
else if (ch2 == ByteDot && ch3 == ByteSlash)
122-
{
123-
// B. if the input buffer begins with a prefix of "/./" or "/.",
124-
// where "." is a complete path segment, then replace that
125-
// prefix with "/" in the input buffer; otherwise,
126-
src += 2;
127-
continue;
128-
}
129-
130-
break;
131-
default:
132-
ch2 = *(src + 1);
133-
ch3 = *(src + 2);
134-
ch4 = *(src + 3);
135-
136-
if (ch2 == ByteDot && ch3 == ByteDot && ch4 == ByteSlash)
137-
{
138-
// C. if the input buffer begins with a prefix of "/../" or "/..",
139-
// where ".." is a complete path segment, then replace that
140-
// prefix with "/" in the input buffer and remove the last
141-
// segment and its preceding "/" (if any) from the output
142-
// buffer; otherwise,
143-
src += 3;
144-
145-
if (dst > start)
146-
{
147-
do
148-
{
149-
dst--;
150-
} while (dst > start && *dst != ByteSlash);
151-
}
152-
153-
continue;
154-
}
155-
else if (ch2 == ByteDot && ch3 == ByteSlash)
156-
{
157-
// B. if the input buffer begins with a prefix of "/./" or "/.",
158-
// where "." is a complete path segment, then replace that
159-
// prefix with "/" in the input buffer; otherwise,
160-
src += 2;
161-
continue;
162-
}
163-
164-
break;
69+
return src.Length;
16570
}
166-
167-
// E. move the first path segment in the input buffer to the end of
168-
// the output buffer, including the initial "/" character (if
169-
// any) and any subsequent characters up to, but not including,
170-
// the next "/" character or the end of the input buffer.
171-
do
71+
if (nextDotSegmentIndex < 0)
17272
{
173-
*dst++ = ch1;
174-
ch1 = *++src;
175-
} while (src < end && ch1 != ByteSlash);
176-
}
177-
178-
if (dst == start)
179-
{
180-
*dst++ = ByteSlash;
181-
}
182-
183-
return (int)(dst - start);
184-
}
185-
186-
public static unsafe bool ContainsDotSegments(byte* start, byte* end)
187-
{
188-
var src = start;
189-
while (src < end)
190-
{
191-
var ch1 = *src;
192-
Debug.Assert(ch1 == '/', "Path segment must always start with a '/'");
193-
194-
byte ch2, ch3, ch4;
195-
196-
switch (end - src)
73+
// Copy the remianing src to dst, and return.
74+
currentSrc.CopyTo(src[writtenLength..]);
75+
writtenLength += src.Length - readPointer;
76+
return writtenLength;
77+
}
78+
else if (nextDotSegmentIndex > 0)
19779
{
198-
case 1:
199-
break;
200-
case 2:
201-
ch2 = *(src + 1);
80+
// Copy until the next segment excluding the trailer.
81+
currentSrc[..nextDotSegmentIndex].CopyTo(src[writtenLength..]);
82+
writtenLength += nextDotSegmentIndex;
83+
readPointer += nextDotSegmentIndex;
84+
}
20285

203-
if (ch2 == ByteDot)
204-
{
205-
return true;
206-
}
86+
var remainingLength = src.Length - readPointer;
20787

208-
break;
209-
case 3:
210-
ch2 = *(src + 1);
211-
ch3 = *(src + 2);
88+
// Case of /../ or /./ or non-dot segments.
89+
if (remainingLength > 3)
90+
{
91+
var nextIndex = readPointer + 2;
92+
93+
if (src[nextIndex] == ByteSlash)
94+
{
95+
// Case: /./
96+
readPointer = nextIndex;
97+
}
98+
else if (MemoryMarshal.CreateSpan(ref src[nextIndex], 2).StartsWith(dotSlash))
99+
{
100+
// Case: /../
101+
// Remove the last segment and replace the path with /
102+
var lastIndex = MemoryMarshal.CreateSpan(ref src[0], writtenLength).LastIndexOf(ByteSlash);
103+
104+
// Move write pointer to the end of the previous segment without / or to start position
105+
writtenLength = int.Max(0, lastIndex);
106+
107+
// Move the read pointer to the next segments beginning including /
108+
readPointer += 3;
109+
}
110+
else
111+
{
112+
// Not a dot segment e.g. /.a, copy the matched /. and the next character then bump the read pointer
113+
src.Slice(readPointer, 3).CopyTo(src[writtenLength..]);
114+
writtenLength += 3;
115+
readPointer = nextIndex + 1;
116+
}
117+
}
212118

213-
if ((ch2 == ByteDot && ch3 == ByteDot) ||
214-
(ch2 == ByteDot && ch3 == ByteSlash))
119+
// Ending with /.. or /./ or non-dot segments.
120+
else if (remainingLength == 3)
121+
{
122+
var nextIndex = readPointer + 2;
123+
if (src[nextIndex] == ByteSlash)
124+
{
125+
// Case: /./ Replace the /./ segment with a closing /
126+
src[writtenLength++] = ByteSlash;
127+
return writtenLength;
128+
}
129+
else if (src[nextIndex] == ByteDot)
130+
{
131+
// Case: /.. Remove the last segment and replace the path with /
132+
var lastSlashIndex = MemoryMarshal.CreateSpan(ref src[0], writtenLength).LastIndexOf(ByteSlash);
133+
134+
// If this was the beginning of the string, then return /
135+
if (lastSlashIndex < 0)
215136
{
216-
return true;
137+
Debug.Assert(src[0] == '/');
138+
return 1;
217139
}
218-
219-
break;
220-
default:
221-
ch2 = *(src + 1);
222-
ch3 = *(src + 2);
223-
ch4 = *(src + 3);
224-
225-
if ((ch2 == ByteDot && ch3 == ByteDot && ch4 == ByteSlash) ||
226-
(ch2 == ByteDot && ch3 == ByteSlash))
140+
else
227141
{
228-
return true;
142+
writtenLength = lastSlashIndex + 1;
229143
}
230-
231-
break;
144+
return writtenLength;
145+
}
146+
else
147+
{
148+
// Not a dot segment e.g. /.a, copy the remaining part.
149+
src[readPointer..].CopyTo(src[writtenLength..]);
150+
return writtenLength + 3;
151+
}
232152
}
233-
234-
do
153+
// Ending with /.
154+
else if (remainingLength == 2)
235155
{
236-
ch1 = *++src;
237-
} while (src < end && ch1 != ByteSlash);
156+
src[writtenLength++] = ByteSlash;
157+
return writtenLength;
158+
}
238159
}
239-
240-
return false;
160+
return writtenLength;
241161
}
242162
}

‎src/Servers/Kestrel/Core/test/PathNormalizerTests.cs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Licensed to the .NET Foundation under one or more agreements.
1+
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System;
@@ -54,6 +54,29 @@ public class PathNormalizerTests
5454
[InlineData("/", "/")]
5555
[InlineData("/no/segments", "/no/segments")]
5656
[InlineData("/no/segments/", "/no/segments/")]
57+
[InlineData("/././", "/")]
58+
[InlineData("/./.", "/")]
59+
[InlineData("/../..", "/")]
60+
[InlineData("/../../", "/")]
61+
[InlineData("/../.", "/")]
62+
[InlineData("/./..", "/")]
63+
[InlineData("/.././", "/")]
64+
[InlineData("/./../", "/")]
65+
[InlineData("/..", "/")]
66+
[InlineData("/.", "/")]
67+
[InlineData("/a/abc/../abc/../b", "/a/b")]
68+
[InlineData("/a/abc/.a", "/a/abc/.a")]
69+
[InlineData("/a/abc/..a", "/a/abc/..a")]
70+
[InlineData("/a/.b/c", "/a/.b/c")]
71+
[InlineData("/a/.b/../c", "/a/c")]
72+
[InlineData("/a/../.b/./c", "/.b/c")]
73+
[InlineData("/a/.b/./c", "/a/.b/c")]
74+
[InlineData("/a/./.b/./c", "/a/.b/c")]
75+
[InlineData("/a/..b/c", "/a/..b/c")]
76+
[InlineData("/a/..b/../c", "/a/c")]
77+
[InlineData("/a/../..b/./c", "/..b/c")]
78+
[InlineData("/a/..b/./c", "/a/..b/c")]
79+
[InlineData("/a/./..b/./c", "/a/..b/c")]
5780
public void RemovesDotSegments(string input, string expected)
5881
{
5982
var data = Encoding.ASCII.GetBytes(input);

0 commit comments

Comments
 (0)