Skip to content

Commit 0ae6cc8

Browse files
pakrympakrym
authored
Remove InplaceStringBuilder usages (#6163)
1 parent 3477daf commit 0ae6cc8

File tree

5 files changed

+147
-100
lines changed

5 files changed

+147
-100
lines changed

‎src/Http/Headers/src/HeaderUtilities.cs

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -625,34 +625,40 @@ public static StringSegment UnescapeAsQuotedString(StringSegment input)
625625
{
626626
input = RemoveQuotes(input);
627627

628-
// First pass to calculate the size of the InplaceStringBuilder
628+
// First pass to calculate the size of the string
629629
var backSlashCount = CountBackslashesForDecodingQuotedString(input);
630630

631631
if (backSlashCount == 0)
632632
{
633633
return input;
634634
}
635635

636-
var stringBuilder = new InplaceStringBuilder(input.Length - backSlashCount);
637-
638-
for (var i = 0; i < input.Length; i++)
636+
return string.Create(input.Length - backSlashCount, input, (span, segment) =>
639637
{
640-
if (i < input.Length - 1 && input[i] == '\\')
638+
var spanIndex = 0;
639+
var spanLength = span.Length;
640+
for (var i = 0; i < segment.Length && (uint)spanIndex < (uint)spanLength; i++)
641641
{
642-
// If there is an backslash character as the last character in the string,
643-
// we will assume that it should be included literally in the unescaped string
644-
// Ex: "hello\\" => "hello\\"
645-
// Also, if a sender adds a quoted pair like '\\''n',
646-
// we will assume it is over escaping and just add a n to the string.
647-
// Ex: "he\\llo" => "hello"
648-
stringBuilder.Append(input[i + 1]);
649-
i++;
650-
continue;
651-
}
652-
stringBuilder.Append(input[i]);
653-
}
642+
int nextIndex = i + 1;
643+
if ((uint)nextIndex < (uint)segment.Length && segment[i] == '\\')
644+
{
645+
// If there is an backslash character as the last character in the string,
646+
// we will assume that it should be included literally in the unescaped string
647+
// Ex: "hello\\" => "hello\\"
648+
// Also, if a sender adds a quoted pair like '\\''n',
649+
// we will assume it is over escaping and just add a n to the string.
650+
// Ex: "he\\llo" => "hello"
651+
span[spanIndex] = segment[nextIndex];
652+
i++;
653+
}
654+
else
655+
{
656+
span[spanIndex] = segment[i];
657+
}
654658

655-
return stringBuilder.ToString();
659+
spanIndex++;
660+
}
661+
});
656662
}
657663

658664
private static int CountBackslashesForDecodingQuotedString(StringSegment input)
@@ -696,25 +702,27 @@ public static StringSegment EscapeAsQuotedString(StringSegment input)
696702
// By calling this, we know that the string requires quotes around it to be a valid token.
697703
var backSlashCount = CountAndCheckCharactersNeedingBackslashesWhenEncoding(input);
698704

699-
var stringBuilder = new InplaceStringBuilder(input.Length + backSlashCount + 2); // 2 for quotes
700-
stringBuilder.Append('\"');
705+
// 2 for quotes
706+
return string.Create(input.Length + backSlashCount + 2, input, (span, segment) => {
707+
// Helps to elide the bounds check for span[0]
708+
span[span.Length - 1] = span[0] = '\"';
701709

702-
for (var i = 0; i < input.Length; i++)
703-
{
704-
if (input[i] == '\\' || input[i] == '\"')
710+
var spanIndex = 1;
711+
for (var i = 0; i < segment.Length; i++)
705712
{
706-
stringBuilder.Append('\\');
707-
}
708-
else if ((input[i] <= 0x1F || input[i] == 0x7F) && input[i] != 0x09)
709-
{
710-
// Control characters are not allowed in a quoted-string, which include all characters
711-
// below 0x1F (except for 0x09 (TAB)) and 0x7F.
712-
throw new FormatException($"Invalid control character '{input[i]}' in input.");
713+
if (segment[i] == '\\' || segment[i] == '\"')
714+
{
715+
span[spanIndex++] = '\\';
716+
}
717+
else if ((segment[i] <= 0x1F || segment[i] == 0x7F) && segment[i] != 0x09)
718+
{
719+
// Control characters are not allowed in a quoted-string, which include all characters
720+
// below 0x1F (except for 0x09 (TAB)) and 0x7F.
721+
throw new FormatException($"Invalid control character '{segment[i]}' in input.");
722+
}
723+
span[spanIndex++] = segment[i];
713724
}
714-
stringBuilder.Append(input[i]);
715-
}
716-
stringBuilder.Append('\"');
717-
return stringBuilder.ToString();
725+
});
718726
}
719727

720728
private static int CountAndCheckCharactersNeedingBackslashesWhenEncoding(StringSegment input)

‎src/Http/Headers/src/SetCookieHeaderValue.cs

Lines changed: 60 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Diagnostics;
67
using System.Diagnostics.Contracts;
78
using System.Text;
89
using Microsoft.Extensions.Primitives;
@@ -24,7 +25,8 @@ public class SetCookieHeaderValue
2425
private const string HttpOnlyToken = "httponly";
2526
private const string SeparatorToken = "; ";
2627
private const string EqualsToken = "=";
27-
private const string DefaultPath = "/"; // TODO: Used?
28+
private const int ExpiresDateLength = 29;
29+
private const string ExpiresDateFormat = "r";
2830

2931
private static readonly HttpHeaderParser<SetCookieHeaderValue> SingleValueParser
3032
= new GenericHeaderParser<SetCookieHeaderValue>(false, GetSetCookieLength);
@@ -99,14 +101,11 @@ public override string ToString()
99101
{
100102
var length = _name.Length + EqualsToken.Length + _value.Length;
101103

102-
string expires = null;
103104
string maxAge = null;
104-
string sameSite = null;
105105

106106
if (Expires.HasValue)
107107
{
108-
expires = HeaderUtilities.FormatDate(Expires.GetValueOrDefault());
109-
length += SeparatorToken.Length + ExpiresToken.Length + EqualsToken.Length + expires.Length;
108+
length += SeparatorToken.Length + ExpiresToken.Length + EqualsToken.Length + ExpiresDateLength;
110109
}
111110

112111
if (MaxAge.HasValue)
@@ -132,7 +131,7 @@ public override string ToString()
132131

133132
if (SameSite != SameSiteMode.None)
134133
{
135-
sameSite = SameSite == SameSiteMode.Lax ? SameSiteLaxToken : SameSiteStrictToken;
134+
var sameSite = SameSite == SameSiteMode.Lax ? SameSiteLaxToken : SameSiteStrictToken;
136135
length += SeparatorToken.Length + SameSiteToken.Length + EqualsToken.Length + sameSite.Length;
137136
}
138137

@@ -141,61 +140,75 @@ public override string ToString()
141140
length += SeparatorToken.Length + HttpOnlyToken.Length;
142141
}
143142

144-
var sb = new InplaceStringBuilder(length);
143+
return string.Create(length, (this, maxAge), (span, tuple) =>
144+
{
145+
var (headerValue, maxAgeValue) = tuple;
145146

146-
sb.Append(_name);
147-
sb.Append(EqualsToken);
148-
sb.Append(_value);
147+
Append(ref span, headerValue._name);
148+
Append(ref span, EqualsToken);
149+
Append(ref span, headerValue._value);
149150

150-
if (expires != null)
151-
{
152-
AppendSegment(ref sb, ExpiresToken, expires);
153-
}
151+
if (headerValue.Expires is DateTimeOffset expiresValue)
152+
{
153+
Append(ref span, SeparatorToken);
154+
Append(ref span, ExpiresToken);
155+
Append(ref span, EqualsToken);
154156

155-
if (maxAge != null)
156-
{
157-
AppendSegment(ref sb, MaxAgeToken, maxAge);
158-
}
157+
var formatted = expiresValue.TryFormat(span, out var charsWritten, ExpiresDateFormat);
158+
span = span.Slice(charsWritten);
159159

160-
if (Domain != null)
161-
{
162-
AppendSegment(ref sb, DomainToken, Domain);
163-
}
160+
Debug.Assert(formatted);
161+
}
164162

165-
if (Path != null)
166-
{
167-
AppendSegment(ref sb, PathToken, Path);
168-
}
163+
if (maxAgeValue != null)
164+
{
165+
AppendSegment(ref span, MaxAgeToken, maxAgeValue);
166+
}
169167

170-
if (Secure)
171-
{
172-
AppendSegment(ref sb, SecureToken, null);
173-
}
168+
if (headerValue.Domain != null)
169+
{
170+
AppendSegment(ref span, DomainToken, headerValue.Domain);
171+
}
174172

175-
if (SameSite != SameSiteMode.None)
176-
{
177-
AppendSegment(ref sb, SameSiteToken, sameSite);
178-
}
173+
if (headerValue.Path != null)
174+
{
175+
AppendSegment(ref span, PathToken, headerValue.Path);
176+
}
179177

180-
if (HttpOnly)
181-
{
182-
AppendSegment(ref sb, HttpOnlyToken, null);
183-
}
178+
if (headerValue.Secure)
179+
{
180+
AppendSegment(ref span, SecureToken, null);
181+
}
184182

185-
return sb.ToString();
183+
if (headerValue.SameSite != SameSiteMode.None)
184+
{
185+
AppendSegment(ref span, SameSiteToken, headerValue.SameSite == SameSiteMode.Lax ? SameSiteLaxToken : SameSiteStrictToken);
186+
}
187+
188+
if (headerValue.HttpOnly)
189+
{
190+
AppendSegment(ref span, HttpOnlyToken, null);
191+
}
192+
});
186193
}
187194

188-
private static void AppendSegment(ref InplaceStringBuilder builder, StringSegment name, StringSegment value)
195+
private static void AppendSegment(ref Span<char> span, StringSegment name, StringSegment value)
189196
{
190-
builder.Append(SeparatorToken);
191-
builder.Append(name);
197+
Append(ref span, SeparatorToken);
198+
Append(ref span, name.AsSpan());
192199
if (value != null)
193200
{
194-
builder.Append(EqualsToken);
195-
builder.Append(value);
201+
Append(ref span, EqualsToken);
202+
Append(ref span, value.AsSpan());
196203
}
197204
}
198205

206+
private static void Append(ref Span<char> span, ReadOnlySpan<char> other)
207+
{
208+
other.CopyTo(span);
209+
span = span.Slice(other.Length);
210+
}
211+
199212
/// <summary>
200213
/// Append string representation of this <see cref="SetCookieHeaderValue"/> to given
201214
/// <paramref name="builder"/>.
@@ -452,14 +465,14 @@ private static int GetSetCookieLength(StringSegment input, int startIndex, out S
452465
result.HttpOnly = true;
453466
}
454467
// extension-av = <any CHAR except CTLs or ";">
455-
else
456-
{
468+
else
469+
{
457470
// TODO: skiping it for now to avoid parsing failure? Store it in a list?
458471
// = (no spaces)
459472
if (!ReadEqualsSign(input, ref offset))
460473
{
461474
return 0;
462-
}
475+
}
463476
ReadToSemicolonOrEnd(input, ref offset);
464477
}
465478
}

‎src/Http/Http/perf/HeaderUtilitiesBenchmark.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
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+
using System;
5+
using BenchmarkDotNet.Attributes;
6+
using Microsoft.Extensions.Primitives;
7+
8+
namespace Microsoft.Net.Http.Headers
9+
{
10+
public class HeaderUtilitiesBenchmark
11+
{
12+
[Benchmark]
13+
public StringSegment UnescapeAsQuotedString()
14+
{
15+
return HeaderUtilities.UnescapeAsQuotedString("\"hello\\\"foo\\\\bar\\\\baz\\\\\"");
16+
}
17+
18+
[Benchmark]
19+
public StringSegment EscapeAsQuotedString()
20+
{
21+
return HeaderUtilities.EscapeAsQuotedString("\"hello\\\"foo\\\\bar\\\\baz\\\\\"");
22+
}
23+
}
24+
}

‎src/Mvc/src/Microsoft.AspNetCore.Mvc.Razor/ViewPath.cs

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// 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

4-
using Microsoft.Extensions.Primitives;
5-
64
namespace Microsoft.AspNetCore.Mvc.Razor
75
{
86
internal static class ViewPath
@@ -23,23 +21,21 @@ public static string NormalizePath(string path)
2321
length++;
2422
}
2523

26-
var builder = new InplaceStringBuilder(length);
27-
if (addLeadingSlash)
24+
return string.Create(length, (path, addLeadingSlash), (span, tuple) =>
2825
{
29-
builder.Append('/');
30-
}
26+
var (pathValue, addLeadingSlashValue) = tuple;
27+
var spanIndex = 0;
3128

32-
for (var i = 0; i < path.Length; i++)
33-
{
34-
var ch = path[i];
35-
if (ch == '\\')
29+
if (addLeadingSlashValue)
3630
{
37-
ch = '/';
31+
span[spanIndex++] = '/';
3832
}
39-
builder.Append(ch);
40-
}
4133

42-
return builder.ToString();
34+
foreach (var ch in pathValue)
35+
{
36+
span[spanIndex++] = ch == '\\' ? '/' : ch;
37+
}
38+
});
4339
}
4440
}
4541
}

‎src/Mvc/src/Microsoft.AspNetCore.Mvc.RazorPages/ApplicationModels/PageRouteModelFactory.cs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,18 @@ private static string CreateAreaRoute(string areaName, string viewEnginePath)
144144
Debug.Assert(!string.IsNullOrEmpty(viewEnginePath));
145145
Debug.Assert(viewEnginePath.StartsWith("/", StringComparison.Ordinal));
146146

147-
var builder = new InplaceStringBuilder(1 + areaName.Length + viewEnginePath.Length);
148-
builder.Append('/');
149-
builder.Append(areaName);
150-
builder.Append(viewEnginePath);
147+
return string.Create(1 + areaName.Length + viewEnginePath.Length, (areaName, viewEnginePath), (span, tuple) =>
148+
{
149+
var (areaNameValue, viewEnginePathValue) = tuple;
150+
151+
span[0] = '/';
152+
span = span.Slice(1);
153+
154+
areaNameValue.AsSpan().CopyTo(span);
155+
span = span.Slice(areaNameValue.Length);
151156

152-
return builder.ToString();
157+
viewEnginePathValue.AsSpan().CopyTo(span);
158+
});
153159
}
154160

155161
private static SelectorModel CreateSelectorModel(string prefix, string routeTemplate)

0 commit comments

Comments
 (0)