Skip to content

Commit

Permalink
Address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
EgorBo committed Nov 18, 2024
1 parent 7d643e5 commit a58a31f
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 97 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ System.Text.Encodings.Web.JavaScriptEncoder</PackageDescription>
<Compile Include="System\Text\Encodings\Web\OptimizedInboxTextEncoder.cs" />
<Compile Include="System\Text\Encodings\Web\DefaultUrlEncoder.cs" />
<Compile Include="System\Text\Encodings\Web\DefaultJavaScriptEncoder.cs" />
<Compile Include="System\Text\Encodings\Web\SpanUtility.cs" />
<Compile Include="System\Text\Encodings\Web\ScalarEscaperBase.cs" />
<Compile Include="System\Text\Encodings\Web\HtmlEncoder.cs" />
<Compile Include="System\Text\Encodings\Web\DefaultHtmlEncoder.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ static int TryEncodeScalarAsHex(object @this, uint scalarValue, Span<byte> desti
int idxOfSemicolon = (int)((uint)BitOperations.Log2(scalarValue) / 4) + 4;
Debug.Assert(4 <= idxOfSemicolon && idxOfSemicolon <= 9, "Expected '&#x0;'..'&#x10FFFF;'.");

if (!SpanUtility.IsValidIndex(destination, idxOfSemicolon)) { goto OutOfSpaceInner; }
if ((uint)destination.Length <= (uint)idxOfSemicolon) { goto OutOfSpaceInner; }
destination[idxOfSemicolon] = (byte)';';

if (!"&#x0"u8.TryCopyTo(destination))
Expand All @@ -115,7 +115,7 @@ static int TryEncodeScalarAsHex(object @this, uint scalarValue, Span<byte> desti
}

destination = destination.Slice(3, idxOfSemicolon - 3);
for (int i = destination.Length - 1; SpanUtility.IsValidIndex(destination, i); i--)
for (int i = destination.Length - 1; (uint)destination.Length > (uint)i; i--)
{
char asUpperHex = HexConverter.ToCharUpper((int)scalarValue);
destination[i] = (byte)asUpperHex;
Expand Down Expand Up @@ -181,7 +181,7 @@ static int TryEncodeScalarAsHex(object @this, uint scalarValue, Span<char> desti
int idxOfSemicolon = (int)((uint)BitOperations.Log2(scalarValue) / 4) + 4;
Debug.Assert(4 <= idxOfSemicolon && idxOfSemicolon <= 9, "Expected '&#x0;'..'&#x10FFFF;'.");

if (!SpanUtility.IsValidIndex(destination, idxOfSemicolon)) { goto OutOfSpaceInner; }
if ((uint)destination.Length <= (uint)idxOfSemicolon) { goto OutOfSpaceInner; }
destination[idxOfSemicolon] = ';';

// It's more efficient to write 4 chars at a time instead of 1 char.
Expand All @@ -192,7 +192,7 @@ static int TryEncodeScalarAsHex(object @this, uint scalarValue, Span<char> desti
}

destination = destination.Slice(3, idxOfSemicolon - 3);
for (int i = destination.Length - 1; SpanUtility.IsValidIndex(destination, i); i--)
for (int i = destination.Length - 1; (uint)destination.Length > (uint)i; i--)
{
char asUpperHex = HexConverter.ToCharUpper((int)scalarValue);
destination[i] = asUpperHex;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ internal override int EncodeUtf8(Rune value, Span<byte> destination)
{
if (_preescapedMap.TryLookup(value, out byte preescapedForm))
{
if (!SpanUtility.IsValidIndex(destination, 1)) { goto OutOfSpace; }
if ((uint)destination.Length <= 1) { goto OutOfSpace; }
destination[0] = (byte)'\\';
destination[1] = preescapedForm;
return 2;
Expand All @@ -132,7 +132,7 @@ static int TryEncodeScalarAsHex(object @this, Rune value, Span<byte> destination
if (value.IsBmp)
{
// Write 6 bytes: "\uXXXX"
if (!SpanUtility.IsValidIndex(destination, 5)) { goto OutOfSpaceInner; }
if ((uint)destination.Length <= 5) { goto OutOfSpaceInner; }
destination[0] = (byte)'\\';
destination[1] = (byte)'u';
HexConverter.ToBytesBuffer((byte)value.Value, destination, 4);
Expand All @@ -143,7 +143,7 @@ static int TryEncodeScalarAsHex(object @this, Rune value, Span<byte> destination
{
// Write 12 bytes: "\uXXXX\uYYYY"
UnicodeHelpers.GetUtf16SurrogatePairFromAstralScalarValue((uint)value.Value, out char highSurrogate, out char lowSurrogate);
if (!SpanUtility.IsValidIndex(destination, 11)) { goto OutOfSpaceInner; }
if ((uint)destination.Length <= 11) { goto OutOfSpaceInner; }
destination[0] = (byte)'\\';
destination[1] = (byte)'u';
HexConverter.ToBytesBuffer((byte)highSurrogate, destination, 4);
Expand All @@ -165,7 +165,7 @@ internal override int EncodeUtf16(Rune value, Span<char> destination)
{
if (_preescapedMap.TryLookup(value, out byte preescapedForm))
{
if (!SpanUtility.IsValidIndex(destination, 1)) { goto OutOfSpace; }
if ((uint)destination.Length <= 1) { goto OutOfSpace; }
destination[0] = '\\';
destination[1] = (char)preescapedForm;
return 2;
Expand All @@ -183,7 +183,7 @@ static int TryEncodeScalarAsHex(object @this, Rune value, Span<char> destination
if (value.IsBmp)
{
// Write 6 chars: "\uXXXX"
if (!SpanUtility.IsValidIndex(destination, 5)) { goto OutOfSpaceInner; }
if ((uint)destination.Length <= 5) { goto OutOfSpaceInner; }
destination[0] = '\\';
destination[1] = 'u';
HexConverter.ToCharsBuffer((byte)value.Value, destination, 4);
Expand All @@ -194,7 +194,7 @@ static int TryEncodeScalarAsHex(object @this, Rune value, Span<char> destination
{
// Write 12 chars: "\uXXXX\uYYYY"
UnicodeHelpers.GetUtf16SurrogatePairFromAstralScalarValue((uint)value.Value, out char highSurrogate, out char lowSurrogate);
if (!SpanUtility.IsValidIndex(destination, 11)) { goto OutOfSpaceInner; }
if ((uint)destination.Length <= 11) { goto OutOfSpaceInner; }
destination[0] = '\\';
destination[1] = 'u';
HexConverter.ToCharsBuffer((byte)highSurrogate, destination, 4);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,22 +143,22 @@ internal override int EncodeUtf8(Rune value, Span<byte> destination)
{
uint utf8lsb = (uint)UnicodeHelpers.GetUtf8RepresentationForScalarValue((uint)value.Value);

if (!SpanUtility.IsValidIndex(destination, 2)) { goto OutOfSpace; }
if ((uint)destination.Length <= 2) { goto OutOfSpace; }
destination[0] = (byte)'%';
HexConverter.ToBytesBuffer((byte)utf8lsb, destination, startingIndex: 1);
if ((utf8lsb >>= 8) == 0) { return 3; } // "%XX"

if (!SpanUtility.IsValidIndex(destination, 5)) { goto OutOfSpace; }
if ((uint)destination.Length <= 5) { goto OutOfSpace; }
destination[3] = (byte)'%';
HexConverter.ToBytesBuffer((byte)utf8lsb, destination, startingIndex: 4);
if ((utf8lsb >>= 8) == 0) { return 6; } // "%XX%YY"

if (!SpanUtility.IsValidIndex(destination, 8)) { goto OutOfSpace; }
if ((uint)destination.Length <= 8) { goto OutOfSpace; }
destination[6] = (byte)'%';
HexConverter.ToBytesBuffer((byte)utf8lsb, destination, startingIndex: 7);
if ((utf8lsb >>= 8) == 0) { return 9; } // "%XX%YY%ZZ"

if (!SpanUtility.IsValidIndex(destination, 11)) { goto OutOfSpace; }
if ((uint)destination.Length <= 11) { goto OutOfSpace; }
destination[9] = (byte)'%';
HexConverter.ToBytesBuffer((byte)utf8lsb, destination, startingIndex: 10);
return 12; // "%XX%YY%ZZ%WW"
Expand All @@ -172,22 +172,22 @@ internal override int EncodeUtf16(Rune value, Span<char> destination)
{
uint utf8lsb = (uint)UnicodeHelpers.GetUtf8RepresentationForScalarValue((uint)value.Value);

if (!SpanUtility.IsValidIndex(destination, 2)) { goto OutOfSpace; }
if ((uint)destination.Length <= 2) { goto OutOfSpace; }
destination[0] = '%';
HexConverter.ToCharsBuffer((byte)utf8lsb, destination, startingIndex: 1);
if ((utf8lsb >>= 8) == 0) { return 3; } // "%XX"

if (!SpanUtility.IsValidIndex(destination, 5)) { goto OutOfSpace; }
if ((uint)destination.Length <= 5) { goto OutOfSpace; }
destination[3] = '%';
HexConverter.ToCharsBuffer((byte)utf8lsb, destination, startingIndex: 4);
if ((utf8lsb >>= 8) == 0) { return 6; } // "%XX%YY"

if (!SpanUtility.IsValidIndex(destination, 8)) { goto OutOfSpace; }
if ((uint)destination.Length <= 8) { goto OutOfSpace; }
destination[6] = '%';
HexConverter.ToCharsBuffer((byte)utf8lsb, destination, startingIndex: 7);
if ((utf8lsb >>= 8) == 0) { return 9; } // "%XX%YY%ZZ"

if (!SpanUtility.IsValidIndex(destination, 11)) { goto OutOfSpace; }
if ((uint)destination.Length <= 11) { goto OutOfSpace; }
destination[9] = '%';
HexConverter.ToCharsBuffer((byte)utf8lsb, destination, startingIndex: 10);
return 12; // "%XX%YY%ZZ%WW"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public OperationStatus Encode(ReadOnlySpan<char> source, Span<char> destination,

while (true)
{
if (!SpanUtility.IsValidIndex(source, srcIdx))
if ((uint)source.Length <= (uint)srcIdx)
{
break; // EOF
}
Expand All @@ -135,7 +135,7 @@ public OperationStatus Encode(ReadOnlySpan<char> source, Span<char> destination,
goto NotAscii; // forward jump predicted not taken
}

if (!SpanUtility.IsValidIndex(destination, dstIdx))
if ((uint)destination.Length <= (uint)dstIdx)
{
goto DestTooSmall; // forward jump predicted not taken
}
Expand All @@ -155,7 +155,7 @@ public OperationStatus Encode(ReadOnlySpan<char> source, Span<char> destination,
int dstIdxTemp = dstIdx + 1;
do
{
if (!SpanUtility.IsValidIndex(destination, dstIdxTemp))
if ((uint)destination.Length <= (uint)dstIdxTemp)
{
goto DestTooSmall; // forward jump predicted not taken
}
Expand All @@ -172,7 +172,7 @@ public OperationStatus Encode(ReadOnlySpan<char> source, Span<char> destination,
if (!Rune.TryCreate(thisChar, out Rune scalarValue))
{
int srcIdxTemp = srcIdx + 1;
if (SpanUtility.IsValidIndex(source, srcIdxTemp))
if ((uint)source.Length > (uint)srcIdxTemp)
{
if (Rune.TryCreate(thisChar, source[srcIdxTemp], out scalarValue))
{
Expand Down Expand Up @@ -243,7 +243,7 @@ public OperationStatus EncodeUtf8(ReadOnlySpan<byte> source, Span<byte> destinat

while (true)
{
if (!SpanUtility.IsValidIndex(source, srcIdx))
if ((uint)source.Length <= (uint)srcIdx)
{
break; // EOF
}
Expand Down Expand Up @@ -272,7 +272,7 @@ public OperationStatus EncodeUtf8(ReadOnlySpan<byte> source, Span<byte> destinat
int dstIdxTemp = dstIdx;
do
{
if (!SpanUtility.IsValidIndex(destination, dstIdxTemp))
if ((uint)destination.Length <= (uint)dstIdxTemp)
{
goto DestTooSmall; // forward jump predicted not taken
}
Expand Down Expand Up @@ -371,7 +371,7 @@ public int GetIndexOfFirstByteToEncode(ReadOnlySpan<byte> data)
}
}

if (!SpanUtility.IsValidIndex(data, asciiBytesSkipped))
if ((uint)data.Length <= (uint)asciiBytesSkipped)
{
Debug.Assert(asciiBytesSkipped == data.Length);
return -1; // all data consumed
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ private bool TryEncodeUnicodeScalarUtf8(uint unicodeScalar, Span<char> utf16Scra
uint utf8lsb = (uint)UnicodeHelpers.GetUtf8RepresentationForScalarValue((uint)nextScalarValue.Value);
do
{
if (SpanUtility.IsValidIndex(utf8Destination, dstIdx))
if ((uint)utf8Destination.Length > (uint)dstIdx)
{
utf8Destination[dstIdx++] = (byte)utf8lsb;
}
Expand Down
41 changes: 0 additions & 41 deletions src/libraries/System.Text.Encodings.Web/tests/SpanUtilityTests.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,12 @@
<Compile Include="..\src\System\Text\Encodings\Web\AsciiByteMap.cs" Link="System\Text\Encodings\Web\AsciiByteMap.cs" />
<Compile Include="..\src\System\Text\Encodings\Web\OptimizedInboxTextEncoder.Ascii.cs" Link="System\Text\Encodings\Web\OptimizedInboxTextEncoder.Ascii.cs" />
<Compile Include="..\src\System\Text\Encodings\Web\ScalarEscaperBase.cs" Link="System\Text\Encodings\Web\ScalarEscaperBase.cs" />
<Compile Include="..\src\System\Text\Encodings\Web\SpanUtility.cs" Link="System\Text\Encodings\Web\SpanUtility.cs" />
<Compile Include="..\src\System\Text\Unicode\UnicodeHelpers.cs" Link="System\Text\Unicode\UnicodeHelpers.cs" />
<Compile Include="..\src\System\Text\Unicode\UnicodeHelpers.generated.cs" Link="System\Text\Unicode\UnicodeHelpers.generated.cs" />
<Compile Include="AsciiByteMapTests.cs" />
<Compile Include="AsciiPreescapedDataTests.cs" />
<Compile Include="AllowedAsciiCodePointsTests.cs" />
<Compile Include="InboxEncoderCommonTests.cs" />
<Compile Include="SpanUtilityTests.cs" />
<Compile Include="AllowedBmpCodePointsBitmapTests.cs" />
<Compile Include="TextEncoderBatteryTests.cs" />
<Compile Include="TextEncoderTests.cs" />
Expand Down

0 comments on commit a58a31f

Please sign in to comment.