Skip to content

Fix form pipe parser #12749

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 2 commits into from
Aug 1, 2019
Merged

Fix form pipe parser #12749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6917aa7
Fix form pipe parser #12381
Tratcher Jul 30, 2019
a703fd5
Naming, minor cleanup
Tratcher Jul 31, 2019
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
4 changes: 2 additions & 2 deletions ...es/perf/Microsoft.AspNetCore.WebUtilities.Performance/FormPipeReaderInternalsBenchmark.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ namespace Microsoft.AspNetCore.WebUtilities.Performance
public class FormPipeReaderInternalsBenchmark
{
private byte[] _singleUtf8 = Encoding.UTF8.GetBytes("foo=bar&baz=boo&haha=hehe&lol=temp");
private byte[] _firstUtf8 = Encoding.UTF8.GetBytes("foo=bar&baz=boo");
private byte[] _secondUtf8 = Encoding.UTF8.GetBytes("&haha=hehe&lol=temp");
private byte[] _firstUtf8 = Encoding.UTF8.GetBytes("foo=bar&baz=bo");
private byte[] _secondUtf8 = Encoding.UTF8.GetBytes("o&haha=hehe&lol=temp");
private FormPipeReader _formPipeReader;

[IterationSetup]
Expand Down
147 changes: 86 additions & 61 deletions src/Http/WebUtilities/src/FormPipeReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Buffers;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.IO.Pipelines;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -139,74 +140,78 @@ private void ParseFormValuesFast(ReadOnlySpan<byte> span,
bool isFinalBlock,
out int consumed)
{
ReadOnlySpan<byte> key = default;
ReadOnlySpan<byte> value = default;
ReadOnlySpan<byte> key;
ReadOnlySpan<byte> value;
consumed = 0;
var equalsDelimiter = GetEqualsForEncoding();
var andDelimiter = GetAndForEncoding();

while (span.Length > 0)
{
var equals = span.IndexOf(equalsDelimiter);
// Find the end of the key=value pair.
var ampersand = span.IndexOf(andDelimiter);
ReadOnlySpan<byte> keyValuePair;
int equals;
var foundAmpersand = ampersand != -1;

if (equals == -1)
if (foundAmpersand)
{
keyValuePair = span.Slice(0, ampersand);
span = span.Slice(keyValuePair.Length + andDelimiter.Length);
consumed += keyValuePair.Length + andDelimiter.Length;
}
else
{
if (span.Length > KeyLengthLimit)
// We can't know that what is currently read is the end of the form value, that's only the case if this is the final block
// If we're not in the final block, then consume nothing
if (!isFinalBlock)
{
ThrowKeyTooLargeException();
// Don't buffer indefinately
if (span.Length > KeyLengthLimit + ValueLengthLimit)
{
ThrowKeyOrValueTooLargeException();
}
return;
}
break;
}

if (equals > KeyLengthLimit)
{
ThrowKeyTooLargeException();
keyValuePair = span;
span = default;
consumed += keyValuePair.Length;
}

key = span.Slice(0, equals);

span = span.Slice(key.Length + equalsDelimiter.Length);
value = span;
equals = keyValuePair.IndexOf(equalsDelimiter);

var ampersand = span.IndexOf(andDelimiter);

if (ampersand == -1)
if (equals == -1)
{
if (span.Length > ValueLengthLimit)
{
ThrowValueTooLargeException();
return;
}

if (!isFinalBlock)
// Too long for the whole segment to be a key.
if (keyValuePair.Length > KeyLengthLimit)
{
// We can't know that what is currently read is the end of the form value, that's only the case if this is the final block
// If we're not in the final block, then consume nothing
break;
ThrowKeyTooLargeException();
}

// If we are on the final block, the remaining content in value is what we want to add to the KVAccumulator.
// Clear out the remaining span such that the loop will exit.
span = Span<byte>.Empty;
// There is no more data, this segment must be "key" with no equals or value.
key = keyValuePair;
value = default;
}
else
{
if (ampersand > ValueLengthLimit)
key = keyValuePair.Slice(0, equals);
if (key.Length > KeyLengthLimit)
{
ThrowValueTooLargeException();
ThrowKeyTooLargeException();
}

value = span.Slice(0, ampersand);
span = span.Slice(ampersand + andDelimiter.Length);
value = keyValuePair.Slice(equals + equalsDelimiter.Length);
if (value.Length > ValueLengthLimit)
{
ThrowValueTooLargeException();
}
}

var decodedKey = GetDecodedString(key);
var decodedValue = GetDecodedString(value);

AppendAndVerify(ref accumulator, decodedKey, decodedValue);

// Cover case where we don't have an ampersand at the end.
consumed += key.Length + value.Length + (ampersand == -1 ? equalsDelimiter.Length : equalsDelimiter.Length + andDelimiter.Length);
}
}

Expand All @@ -217,53 +222,68 @@ private void ParseValuesSlow(
bool isFinalBlock)
{
var sequenceReader = new SequenceReader<byte>(buffer);
ReadOnlySequence<byte> keyValuePair;

var consumed = sequenceReader.Position;
var consumedBytes = default(long);
var equalsDelimiter = GetEqualsForEncoding();
var andDelimiter = GetAndForEncoding();

while (!sequenceReader.End)
{
// TODO seems there is a bug with TryReadTo (advancePastDelimiter: true). It isn't advancing past the delimiter on second read.
if (!sequenceReader.TryReadTo(out ReadOnlySequence<byte> key, equalsDelimiter, advancePastDelimiter: false) ||
!sequenceReader.IsNext(equalsDelimiter, true))
if (!sequenceReader.TryReadTo(out keyValuePair, andDelimiter))
{
if ((sequenceReader.Consumed - consumedBytes) > KeyLengthLimit)
if (!isFinalBlock)
{
ThrowKeyTooLargeException();
// Don't buffer indefinately
if ((sequenceReader.Consumed - consumedBytes) > KeyLengthLimit + ValueLengthLimit)
{
ThrowKeyOrValueTooLargeException();
}
break;
}

break;
// This must be the final key=value pair
keyValuePair = buffer.Slice(sequenceReader.Position);
sequenceReader.Advance(keyValuePair.Length);
}

if (key.Length > KeyLengthLimit)
if (keyValuePair.IsSingleSegment)
{
ThrowKeyTooLargeException();
ParseFormValuesFast(keyValuePair.FirstSpan, ref accumulator, isFinalBlock: true, out var segmentConsumed);
Debug.Assert(segmentConsumed == keyValuePair.FirstSpan.Length);
continue;
}

if (!sequenceReader.TryReadTo(out ReadOnlySequence<byte> value, andDelimiter, false) ||
!sequenceReader.IsNext(andDelimiter, true))
var keyValueReader = new SequenceReader<byte>(keyValuePair);
ReadOnlySequence<byte> value;

if (keyValueReader.TryReadTo(out var key, equalsDelimiter))
{
if (!isFinalBlock)
if (key.Length > KeyLengthLimit)
{
if ((sequenceReader.Consumed - consumedBytes - key.Length) > ValueLengthLimit)
{
ThrowValueTooLargeException();
}
break;
ThrowKeyTooLargeException();
}

value = buffer.Slice(sequenceReader.Position);

sequenceReader.Advance(value.Length);
value = keyValuePair.Slice(keyValueReader.Position);
if (value.Length > ValueLengthLimit)
{
ThrowValueTooLargeException();
}
}

if (value.Length > ValueLengthLimit)
else
{
ThrowValueTooLargeException();
// Too long for the whole segment to be a key.
if (keyValuePair.Length > KeyLengthLimit)
{
ThrowKeyTooLargeException();
}

// There is no more data, this segment must be "key" with no equals or value.
key = keyValuePair;
value = default;
}

// Need to call ToArray if the key/value spans multiple segments
var decodedKey = GetDecodedStringFromReadOnlySequence(key);
var decodedValue = GetDecodedStringFromReadOnlySequence(value);

Expand All @@ -276,6 +296,11 @@ private void ParseValuesSlow(
buffer = buffer.Slice(consumed);
}

private void ThrowKeyOrValueTooLargeException()
{
throw new InvalidDataException($"Form key length limit {KeyLengthLimit} or value length limit {ValueLengthLimit} exceeded.");
}

private void ThrowKeyTooLargeException()
{
throw new InvalidDataException($"Form key length limit {KeyLengthLimit} exceeded.");
Expand Down
Loading