Skip to content

Commit 63553a5

Browse files
[Http] Remove some unsafe code and save a string allocation (#31267)
1 parent 64a94f0 commit 63553a5

6 files changed

+58
-92
lines changed

src/Http/Headers/src/ContentDispositionHeaderValue.cs

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33

44
using System;
55
using System.Buffers;
6+
using System.Buffers.Text;
67
using System.Collections.Generic;
78
using System.Diagnostics.CodeAnalysis;
89
using System.Diagnostics.Contracts;
910
using System.Globalization;
1011
using System.Linq;
12+
using System.Runtime.CompilerServices;
1113
using System.Text;
1214
using Microsoft.Extensions.Primitives;
1315

@@ -30,6 +32,9 @@ public class ContentDispositionHeaderValue
3032
private const string SizeString = "size";
3133
private static readonly char[] QuestionMark = new char[] { '?' };
3234
private static readonly char[] SingleQuote = new char[] { '\'' };
35+
private static readonly char[] EscapeChars = new char[] { '\\', '"' };
36+
private static ReadOnlySpan<byte> MimePrefix => new byte[] { (byte)'"', (byte)'=', (byte)'?', (byte)'u', (byte)'t', (byte)'f', (byte)'-', (byte)'8', (byte)'?', (byte)'B', (byte)'?' };
37+
private static ReadOnlySpan<byte> MimeSuffix => new byte[] { (byte)'?', (byte)'=', (byte)'"' };
3338

3439
private static readonly HttpHeaderParser<ContentDispositionHeaderValue> Parser
3540
= new GenericHeaderParser<ContentDispositionHeaderValue>(false, GetDispositionTypeLength);
@@ -466,8 +471,10 @@ private StringSegment EncodeAndQuoteMime(StringSegment input)
466471

467472
if (RequiresEncoding(result))
468473
{
469-
needsQuotes = true; // Encoded data must always be quoted, the equals signs are invalid in tokens
470-
result = EncodeMime(result); // =?utf-8?B?asdfasdfaesdf?=
474+
// EncodeMimeWithQuotes will Base64 encode any quotes in the input, and surround the payload in quotes
475+
// so there is no need to add quotes
476+
needsQuotes = false;
477+
result = EncodeMimeWithQuotes(result); // "=?utf-8?B?asdfasdfaesdf?="
471478
}
472479
else if (!needsQuotes && HttpRuleParser.GetTokenLength(result, 0) != result.Length)
473480
{
@@ -476,8 +483,11 @@ private StringSegment EncodeAndQuoteMime(StringSegment input)
476483

477484
if (needsQuotes)
478485
{
479-
// '\' and '"' must be escaped in a quoted string
480-
result = result.ToString().Replace(@"\", @"\\").Replace(@"""", @"\""");
486+
if (result.IndexOfAny(EscapeChars) != -1)
487+
{
488+
// '\' and '"' must be escaped in a quoted string
489+
result = result.ToString().Replace(@"\", @"\\").Replace(@"""", @"\""");
490+
}
481491
// Re-add quotes "value"
482492
result = string.Format(CultureInfo.InvariantCulture, "\"{0}\"", result);
483493
}
@@ -531,20 +541,41 @@ private bool RequiresEncoding(StringSegment input)
531541
return false;
532542
}
533543

534-
// Encode using MIME encoding
535-
private unsafe string EncodeMime(StringSegment input)
544+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
545+
private static int GetBase64Length(int inputLength)
536546
{
537-
fixed (char* chars = input.Buffer)
547+
// Copied from https://github.com/dotnet/runtime/blob/82ca681cbac89d813a3ce397e0c665e6c051ed67/src/libraries/System.Private.CoreLib/src/System/Convert.cs#L2530
548+
long outlen = ((long)inputLength) / 3 * 4; // the base length - we want integer division here.
549+
outlen += ((inputLength % 3) != 0) ? 4 : 0; // at most 4 more chars for the remainder
550+
551+
if (outlen > int.MaxValue)
538552
{
539-
var byteCount = Encoding.UTF8.GetByteCount(chars + input.Offset, input.Length);
540-
var buffer = new byte[byteCount];
541-
fixed (byte* bytes = buffer)
542-
{
543-
Encoding.UTF8.GetBytes(chars + input.Offset, input.Length, bytes, byteCount);
544-
}
545-
var encodedName = Convert.ToBase64String(buffer);
546-
return "=?utf-8?B?" + encodedName + "?=";
553+
throw new OutOfMemoryException();
547554
}
555+
556+
return (int)outlen;
557+
}
558+
559+
// Encode using MIME encoding
560+
// And adds surrounding quotes, Encoded data must always be quoted, the equals signs are invalid in tokens
561+
private string EncodeMimeWithQuotes(StringSegment input)
562+
{
563+
var requiredLength = MimePrefix.Length +
564+
GetBase64Length(Encoding.UTF8.GetByteCount(input.AsSpan())) +
565+
MimeSuffix.Length;
566+
Span<byte> buffer = requiredLength <= 256
567+
? (stackalloc byte[256]).Slice(0, requiredLength)
568+
: new byte[requiredLength];
569+
570+
MimePrefix.CopyTo(buffer);
571+
var bufferContent = buffer.Slice(MimePrefix.Length);
572+
var contentLength = Encoding.UTF8.GetBytes(input.AsSpan(), bufferContent);
573+
574+
Base64.EncodeToUtf8InPlace(bufferContent, contentLength, out var base64ContentLength);
575+
576+
MimeSuffix.CopyTo(bufferContent.Slice(base64ContentLength));
577+
578+
return Encoding.UTF8.GetString(buffer.Slice(0, MimePrefix.Length + base64ContentLength + MimeSuffix.Length));
548579
}
549580

550581
// Attempt to decode MIME encoded strings

src/Http/Headers/src/HeaderUtilities.cs

Lines changed: 7 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ namespace Microsoft.Net.Http.Headers
1616
/// </summary>
1717
public static class HeaderUtilities
1818
{
19-
private static readonly int _int64MaxStringLength = 19;
2019
private static readonly int _qualityValueMaxCharCount = 10; // Little bit more permissive than RFC7231 5.3.1
2120
private const string QualityName = "q";
2221
internal const string BytesUnit = "bytes";
@@ -325,7 +324,7 @@ public static bool ContainsCacheDirective(StringValues cacheControlDirectives, s
325324
return false;
326325
}
327326

328-
private static unsafe bool TryParseNonNegativeInt64FromHeaderValue(int startIndex, string headerValue, out long result)
327+
private static bool TryParseNonNegativeInt64FromHeaderValue(int startIndex, string headerValue, out long result)
329328
{
330329
// Trim leading whitespace
331330
startIndex += HttpRuleParser.GetWhitespaceLength(headerValue, startIndex);
@@ -366,40 +365,15 @@ private static unsafe bool TryParseNonNegativeInt64FromHeaderValue(int startInde
366365
/// result will be overwritten.
367366
/// </param>
368367
/// <returns><see langword="true" /> if parsing succeeded; otherwise, <see langword="false" />.</returns>
369-
public static unsafe bool TryParseNonNegativeInt32(StringSegment value, out int result)
368+
public static bool TryParseNonNegativeInt32(StringSegment value, out int result)
370369
{
371370
if (string.IsNullOrEmpty(value.Buffer) || value.Length == 0)
372371
{
373372
result = 0;
374373
return false;
375374
}
376375

377-
result = 0;
378-
fixed (char* ptr = value.Buffer)
379-
{
380-
var ch = (ushort*)ptr + value.Offset;
381-
var end = ch + value.Length;
382-
383-
ushort digit = 0;
384-
while (ch < end && (digit = (ushort)(*ch - 0x30)) <= 9)
385-
{
386-
// Check for overflow
387-
if ((result = result * 10 + digit) < 0)
388-
{
389-
result = 0;
390-
return false;
391-
}
392-
393-
ch++;
394-
}
395-
396-
if (ch != end)
397-
{
398-
result = 0;
399-
return false;
400-
}
401-
return true;
402-
}
376+
return int.TryParse(value.AsSpan(), NumberStyles.None, NumberFormatInfo.InvariantInfo, out result);
403377
}
404378

405379
/// <summary>
@@ -417,40 +391,14 @@ public static unsafe bool TryParseNonNegativeInt32(StringSegment value, out int
417391
/// originally supplied in result will be overwritten.
418392
/// </param>
419393
/// <returns><see langword="true" /> if parsing succeeded; otherwise, <see langword="false" />.</returns>
420-
public static unsafe bool TryParseNonNegativeInt64(StringSegment value, out long result)
394+
public static bool TryParseNonNegativeInt64(StringSegment value, out long result)
421395
{
422396
if (string.IsNullOrEmpty(value.Buffer) || value.Length == 0)
423397
{
424398
result = 0;
425399
return false;
426400
}
427-
428-
result = 0;
429-
fixed (char* ptr = value.Buffer)
430-
{
431-
var ch = (ushort*)ptr + value.Offset;
432-
var end = ch + value.Length;
433-
434-
ushort digit = 0;
435-
while (ch < end && (digit = (ushort)(*ch - 0x30)) <= 9)
436-
{
437-
// Check for overflow
438-
if ((result = result * 10 + digit) < 0)
439-
{
440-
result = 0;
441-
return false;
442-
}
443-
444-
ch++;
445-
}
446-
447-
if (ch != end)
448-
{
449-
result = 0;
450-
return false;
451-
}
452-
return true;
453-
}
401+
return long.TryParse(value.AsSpan(), NumberStyles.None, NumberFormatInfo.InvariantInfo, out result);
454402
}
455403

456404
// Strict and fast RFC7231 5.3.1 Quality value parser (and without memory allocation)
@@ -553,7 +501,7 @@ internal static bool TryParseQualityDouble(StringSegment input, int startIndex,
553501
/// <returns>
554502
/// The string representation of the value of this instance, consisting of a sequence of digits ranging from 0 to 9 with no leading zeroes.
555503
/// </returns>
556-
public unsafe static string FormatNonNegativeInt64(long value)
504+
public static string FormatNonNegativeInt64(long value)
557505
{
558506
if (value < 0)
559507
{
@@ -565,19 +513,7 @@ public unsafe static string FormatNonNegativeInt64(long value)
565513
return "0";
566514
}
567515

568-
var position = _int64MaxStringLength;
569-
char* charBuffer = stackalloc char[_int64MaxStringLength];
570-
571-
do
572-
{
573-
// Consider using Math.DivRem() if available
574-
var quotient = value / 10;
575-
charBuffer[--position] = (char)(0x30 + (value - quotient * 10)); // 0x30 = '0'
576-
value = quotient;
577-
}
578-
while (value != 0);
579-
580-
return new string(charBuffer, position, _int64MaxStringLength - position);
516+
return ((ulong)value).ToString(NumberFormatInfo.InvariantInfo);
581517
}
582518

583519
/// <summary>

src/Http/Headers/src/MediaTypeHeaderValue.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Diagnostics.Contracts;
88
using System.Globalization;
99
using System.Linq;
10+
using System.Runtime.InteropServices;
1011
using System.Text;
1112
using Microsoft.Extensions.Primitives;
1213

@@ -645,7 +646,7 @@ private static int GetMediaTypeExpressionLength(StringSegment input, int startIn
645646
}
646647
else
647648
{
648-
mediaType = input.Substring(startIndex, typeLength) + ForwardSlashCharacter + input.Substring(current, subtypeLength);
649+
mediaType = string.Concat(input.AsSpan().Slice(startIndex, typeLength), "/", input.AsSpan().Slice(current, subtypeLength));
649650
}
650651

651652
return mediaTypeLength;

src/Http/Headers/src/Microsoft.Net.Http.Headers.csproj

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
44
<Description>HTTP header parser implementations.</Description>
55
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
66
<IsAspNetCoreApp>true</IsAspNetCoreApp>
7-
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
87
<GenerateDocumentationFile>true</GenerateDocumentationFile>
98
<PackageTags>http</PackageTags>
109
<IsPackable>false</IsPackable>

src/Http/Routing/src/Matching/SingleEntryAsciiJumpTable.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -27,7 +27,7 @@ public SingleEntryAsciiJumpTable(
2727
_destination = destination;
2828
}
2929

30-
public unsafe override int GetDestination(string path, PathSegment segment)
30+
public override int GetDestination(string path, PathSegment segment)
3131
{
3232
var length = segment.Length;
3333
if (length == 0)

src/Http/Routing/src/Microsoft.AspNetCore.Routing.csproj

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ Microsoft.AspNetCore.Routing.RouteCollection</Description>
99
<IsAspNetCoreApp>true</IsAspNetCoreApp>
1010
<GenerateDocumentationFile>true</GenerateDocumentationFile>
1111
<PackageTags>aspnetcore;routing</PackageTags>
12-
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
1312
<IsPackable>false</IsPackable>
1413
<Nullable>enable</Nullable>
1514
</PropertyGroup>

0 commit comments

Comments
 (0)