Skip to content

Commit 75d1bd2

Browse files
committed
Apply HeadersLengthLimit to preamble in MultipartReader
1 parent 620d45d commit 75d1bd2

File tree

4 files changed

+60
-5
lines changed

4 files changed

+60
-5
lines changed

src/Http/WebUtilities/src/MultipartBoundary.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ public void ExpectLeadingCrlf()
2525
_expectLeadingCrlf = true;
2626
}
2727

28+
// Lets us throw a more specific error from MultipartReaderStream when reading any preamble data.
29+
public bool BeforeFirstBoundary()
30+
{
31+
return !_expectLeadingCrlf;
32+
}
33+
2834
// Return either "--{boundary}" or "\r\n--{boundary}" depending on if we're looking for the end of a section
2935
public ReadOnlySpan<byte> BoundaryBytes => _boundaryBytes.AsSpan(_expectLeadingCrlf ? 0 : 2);
3036

src/Http/WebUtilities/src/MultipartReader.cs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public class MultipartReader
2727

2828
private readonly BufferedReadStream _stream;
2929
private readonly MultipartBoundary _boundary;
30-
private MultipartReaderStream _currentStream;
30+
private MultipartReaderStream? _currentStream;
3131

3232
/// <summary>
3333
/// Initializes a new instance of <see cref="MultipartReader"/>.
@@ -57,9 +57,6 @@ public MultipartReader(string boundary, Stream stream, int bufferSize)
5757
_stream = new BufferedReadStream(stream, bufferSize);
5858
boundary = HeaderUtilities.RemoveQuotes(new StringSegment(boundary)).ToString();
5959
_boundary = new MultipartBoundary(boundary, false);
60-
// This stream will drain any preamble data and remove the first boundary marker.
61-
// TODO: HeadersLengthLimit can't be modified until after the constructor.
62-
_currentStream = new MultipartReaderStream(_stream, _boundary) { LengthLimit = HeadersLengthLimit };
6360
}
6461

6562
/// <summary>
@@ -86,6 +83,13 @@ public MultipartReader(string boundary, Stream stream, int bufferSize)
8683
/// <returns></returns>
8784
public async Task<MultipartSection?> ReadNextSectionAsync(CancellationToken cancellationToken = new CancellationToken())
8885
{
86+
if (_currentStream == null)
87+
{
88+
// Only occurs on first call
89+
// This stream will drain any preamble data and remove the first boundary marker.
90+
_currentStream = new MultipartReaderStream(_stream, _boundary) { LengthLimit = HeadersLengthLimit };
91+
}
92+
8993
// Drain the prior section.
9094
await _currentStream.DrainAsync(cancellationToken);
9195
// If we're at the end return null

src/Http/WebUtilities/src/MultipartReaderStream.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,15 @@ private int UpdatePosition(int read)
147147
_observedLength = _position;
148148
if (LengthLimit.HasValue && _observedLength > LengthLimit.GetValueOrDefault())
149149
{
150-
throw new InvalidDataException($"Multipart body length limit {LengthLimit.GetValueOrDefault()} exceeded.");
150+
// If we hit the limit before the first boundary then we're using the header length limit, not the body length limit.
151+
if (_boundary.BeforeFirstBoundary())
152+
{
153+
throw new InvalidDataException($"Multipart header length limit {LengthLimit.GetValueOrDefault()} exceeded. Too much data before the first boundary.");
154+
}
155+
else
156+
{
157+
throw new InvalidDataException($"Multipart body length limit {LengthLimit.GetValueOrDefault()} exceeded.");
158+
}
151159
}
152160
}
153161
return read;

src/Http/WebUtilities/test/MultipartReaderTests.cs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,43 @@ public async Task MultipartReader_HeadersLengthExceeded_Throws()
147147
Assert.Equal("Line length limit 17 exceeded.", exception.Message);
148148
}
149149

150+
[Fact]
151+
public async Task MultipartReader_HeadersLengthExceeded_LargePreamble()
152+
{
153+
var body = $"preamble {new string('a', 17000)}\r\n" +
154+
"--9051914041544843365972754266\r\n" +
155+
"\r\n" +
156+
"text default\r\n" +
157+
"--9051914041544843365972754266--\r\n";
158+
var stream = MakeStream(body);
159+
var reader = new MultipartReader(Boundary, stream);
160+
161+
var exception = await Assert.ThrowsAsync<InvalidDataException>(() => reader.ReadNextSectionAsync());
162+
Assert.Equal("Multipart header length limit 16384 exceeded. Too much data before the first boundary.", exception.Message);
163+
}
164+
165+
[Fact]
166+
public async Task MultipartReader_HeadersLengthLimitSettable_LargePreamblePasses()
167+
{
168+
var body = $"preamble {new string('a', 100_000)}\r\n" +
169+
"--9051914041544843365972754266\r\n" +
170+
"\r\n" +
171+
"text default\r\n" +
172+
"--9051914041544843365972754266--\r\n";
173+
var stream = MakeStream(body);
174+
var reader = new MultipartReader(Boundary, stream)
175+
{
176+
HeadersLengthLimit = 200_000,
177+
};
178+
179+
var section = await reader.ReadNextSectionAsync();
180+
Assert.NotNull(section);
181+
182+
var buffer = new MemoryStream();
183+
await section.Body.CopyToAsync(buffer);
184+
Assert.Equal("text default", Encoding.ASCII.GetString(buffer.ToArray()));
185+
}
186+
150187
[Fact]
151188
public async Task MultipartReader_ReadSinglePartBodyWithTrailingWhitespace_Success()
152189
{

0 commit comments

Comments
 (0)