Skip to content

Remove unsafe from PathNormalizer #56805

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Aug 5, 2024
Merged

Remove unsafe from PathNormalizer #56805

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b87fa6c
Tests
Jul 7, 2024
3575c45
FirstIndexOfDotSegment
Jul 7, 2024
d830a59
Next iteration
Jul 11, 2024
1de225a
Next iteration
Jul 12, 2024
205eae8
Back to contains from FirstIndexOf
Jul 12, 2024
dfd631e
Next iteration ++
Jul 12, 2024
d3eba1c
READPOINTER
Jul 13, 2024
2b58d1d
Review feedback
Jul 18, 2024
ec418f0
Remove using statement
Jul 18, 2024
8590b1b
PathNormalizer in Microsoft.AspNetCore.HttpSys.Internal
Jul 29, 2024
c95374b
Remove ContainsDotSegments in the shared PathNormalizer
Jul 30, 2024
239f511
Review feedback copy 3 chars when not matched a dot segment
Aug 2, 2024
9d1960d
Quarantined Caching_SendFileWithFullContentLength_Cached due to cache…
Aug 3, 2024
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
1 change: 1 addition & 0 deletions src/Servers/HttpSys/test/FunctionalTests/ResponseCachingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ public async Task Caching_SendFileNoContentLength_NotCached()
}
}

[QuarantinedTest("new issue")]
[ConditionalFact]
public async Task Caching_SendFileWithFullContentLength_Cached()
{
Expand Down
260 changes: 90 additions & 170 deletions src/Servers/Kestrel/Core/src/Internal/Http/PathNormalizer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Text;
using Microsoft.AspNetCore.Internal;
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
Expand Down Expand Up @@ -50,193 +51,112 @@ public static string DecodePath(Span<byte> path, bool pathEncoded, string rawTar
}

// In-place implementation of the algorithm from https://tools.ietf.org/html/rfc3986#section-5.2.4
public static unsafe int RemoveDotSegments(Span<byte> input)
public static int RemoveDotSegments(Span<byte> src)
{
fixed (byte* start = input)
{
var end = start + input.Length;
return RemoveDotSegments(start, end);
}
}

public static unsafe int RemoveDotSegments(byte* start, byte* end)
{
if (!ContainsDotSegments(start, end))
{
return (int)(end - start);
}
Debug.Assert(src[0] == '/', "Path segment must always start with a '/'");
ReadOnlySpan<byte> dotSlash = "./"u8;
ReadOnlySpan<byte> slashDot = "/."u8;

var src = start;
var dst = start;
var writtenLength = 0;
var readPointer = 0;

while (src < end)
while (src.Length > readPointer)
{
var ch1 = *src;
Debug.Assert(ch1 == '/', "Path segment must always start with a '/'");

byte ch2, ch3, ch4;

switch (end - src)
var currentSrc = src[readPointer..];
var nextDotSegmentIndex = currentSrc.IndexOf(slashDot);
if (nextDotSegmentIndex < 0 && readPointer == 0)
{
case 1:
break;
case 2:
ch2 = *(src + 1);

if (ch2 == ByteDot)
{
// B. if the input buffer begins with a prefix of "/./" or "/.",
// where "." is a complete path segment, then replace that
// prefix with "/" in the input buffer; otherwise,
src += 1;
*src = ByteSlash;
continue;
}

break;
case 3:
ch2 = *(src + 1);
ch3 = *(src + 2);

if (ch2 == ByteDot && ch3 == ByteDot)
{
// C. if the input buffer begins with a prefix of "/../" or "/..",
// where ".." is a complete path segment, then replace that
// prefix with "/" in the input buffer and remove the last
// segment and its preceding "/" (if any) from the output
// buffer; otherwise,
src += 2;
*src = ByteSlash;

if (dst > start)
{
do
{
dst--;
} while (dst > start && *dst != ByteSlash);
}

continue;
}
else if (ch2 == ByteDot && ch3 == ByteSlash)
{
// B. if the input buffer begins with a prefix of "/./" or "/.",
// where "." is a complete path segment, then replace that
// prefix with "/" in the input buffer; otherwise,
src += 2;
continue;
}

break;
default:
ch2 = *(src + 1);
ch3 = *(src + 2);
ch4 = *(src + 3);

if (ch2 == ByteDot && ch3 == ByteDot && ch4 == ByteSlash)
{
// C. if the input buffer begins with a prefix of "/../" or "/..",
// where ".." is a complete path segment, then replace that
// prefix with "/" in the input buffer and remove the last
// segment and its preceding "/" (if any) from the output
// buffer; otherwise,
src += 3;

if (dst > start)
{
do
{
dst--;
} while (dst > start && *dst != ByteSlash);
}

continue;
}
else if (ch2 == ByteDot && ch3 == ByteSlash)
{
// B. if the input buffer begins with a prefix of "/./" or "/.",
// where "." is a complete path segment, then replace that
// prefix with "/" in the input buffer; otherwise,
src += 2;
continue;
}

break;
return src.Length;
}

// E. move the first path segment in the input buffer to the end of
// the output buffer, including the initial "/" character (if
// any) and any subsequent characters up to, but not including,
// the next "/" character or the end of the input buffer.
do
if (nextDotSegmentIndex < 0)
{
*dst++ = ch1;
ch1 = *++src;
} while (src < end && ch1 != ByteSlash);
}

if (dst == start)
{
*dst++ = ByteSlash;
}

return (int)(dst - start);
}

public static unsafe bool ContainsDotSegments(byte* start, byte* end)
{
var src = start;
while (src < end)
{
var ch1 = *src;
Debug.Assert(ch1 == '/', "Path segment must always start with a '/'");

byte ch2, ch3, ch4;

switch (end - src)
// Copy the remianing src to dst, and return.
currentSrc.CopyTo(src[writtenLength..]);
writtenLength += src.Length - readPointer;
return writtenLength;
}
else if (nextDotSegmentIndex > 0)
{
case 1:
break;
case 2:
ch2 = *(src + 1);
// Copy until the next segment excluding the trailer.
currentSrc[..nextDotSegmentIndex].CopyTo(src[writtenLength..]);
writtenLength += nextDotSegmentIndex;
readPointer += nextDotSegmentIndex;
}

if (ch2 == ByteDot)
{
return true;
}
var remainingLength = src.Length - readPointer;

break;
case 3:
ch2 = *(src + 1);
ch3 = *(src + 2);
// Case of /../ or /./ or non-dot segments.
if (remainingLength > 3)
{
var nextIndex = readPointer + 2;

if (src[nextIndex] == ByteSlash)
{
// Case: /./
readPointer = nextIndex;
}
else if (MemoryMarshal.CreateSpan(ref src[nextIndex], 2).StartsWith(dotSlash))
{
// Case: /../
// Remove the last segment and replace the path with /
var lastIndex = MemoryMarshal.CreateSpan(ref src[0], writtenLength).LastIndexOf(ByteSlash);

// Move write pointer to the end of the previous segment without / or to start position
writtenLength = int.Max(0, lastIndex);

// Move the read pointer to the next segments beginning including /
readPointer += 3;
}
else
{
// Not a dot segment e.g. /.a, copy the matched /. and the next character then bump the read pointer
src.Slice(readPointer, 3).CopyTo(src[writtenLength..]);
writtenLength += 3;
readPointer = nextIndex + 1;
}
}

if ((ch2 == ByteDot && ch3 == ByteDot) ||
(ch2 == ByteDot && ch3 == ByteSlash))
// Ending with /.. or /./ or non-dot segments.
else if (remainingLength == 3)
{
var nextIndex = readPointer + 2;
if (src[nextIndex] == ByteSlash)
{
// Case: /./ Replace the /./ segment with a closing /
src[writtenLength++] = ByteSlash;
return writtenLength;
}
else if (src[nextIndex] == ByteDot)
{
// Case: /.. Remove the last segment and replace the path with /
var lastSlashIndex = MemoryMarshal.CreateSpan(ref src[0], writtenLength).LastIndexOf(ByteSlash);

// If this was the beginning of the string, then return /
if (lastSlashIndex < 0)
{
return true;
Debug.Assert(src[0] == '/');
return 1;
}

break;
default:
ch2 = *(src + 1);
ch3 = *(src + 2);
ch4 = *(src + 3);

if ((ch2 == ByteDot && ch3 == ByteDot && ch4 == ByteSlash) ||
(ch2 == ByteDot && ch3 == ByteSlash))
else
{
return true;
writtenLength = lastSlashIndex + 1;
}

break;
return writtenLength;
}
else
{
// Not a dot segment e.g. /.a, copy the remaining part.
src[readPointer..].CopyTo(src[writtenLength..]);
return writtenLength + 3;
}
}

do
// Ending with /.
else if (remainingLength == 2)
{
ch1 = *++src;
} while (src < end && ch1 != ByteSlash);
src[writtenLength++] = ByteSlash;
return writtenLength;
}
}

return false;
return writtenLength;
}
}
25 changes: 24 additions & 1 deletion src/Servers/Kestrel/Core/test/PathNormalizerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
Expand Down Expand Up @@ -54,6 +54,29 @@ public class PathNormalizerTests
[InlineData("/", "/")]
[InlineData("/no/segments", "/no/segments")]
[InlineData("/no/segments/", "/no/segments/")]
[InlineData("/././", "/")]
[InlineData("/./.", "/")]
[InlineData("/../..", "/")]
[InlineData("/../../", "/")]
[InlineData("/../.", "/")]
[InlineData("/./..", "/")]
[InlineData("/.././", "/")]
[InlineData("/./../", "/")]
[InlineData("/..", "/")]
[InlineData("/.", "/")]
[InlineData("/a/abc/../abc/../b", "/a/b")]
[InlineData("/a/abc/.a", "/a/abc/.a")]
[InlineData("/a/abc/..a", "/a/abc/..a")]
[InlineData("/a/.b/c", "/a/.b/c")]
[InlineData("/a/.b/../c", "/a/c")]
[InlineData("/a/../.b/./c", "/.b/c")]
[InlineData("/a/.b/./c", "/a/.b/c")]
[InlineData("/a/./.b/./c", "/a/.b/c")]
[InlineData("/a/..b/c", "/a/..b/c")]
[InlineData("/a/..b/../c", "/a/c")]
[InlineData("/a/../..b/./c", "/..b/c")]
[InlineData("/a/..b/./c", "/a/..b/c")]
[InlineData("/a/./..b/./c", "/a/..b/c")]
public void RemovesDotSegments(string input, string expected)
{
var data = Encoding.ASCII.GetBytes(input);
Expand Down
Loading
Loading