Skip to content

Commit a9b75ca

Browse files
JerichoJericho
committed
Merge branch 'feature/fix_gh104' into develop
2 parents 77bca0e + 7407832 commit a9b75ca

2 files changed

Lines changed: 165 additions & 2 deletions

File tree

‎Source/HttpMultipartParser.UnitTests/ParserScenarios/StreamPositionHasBeenMoved.cs‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
using System.IO;
2+
using System.Linq;
3+
using System.Text;
4+
using System.Threading.Tasks;
5+
using Xunit;
6+
7+
namespace HttpMultipartParser.UnitTests.ParserScenarios
8+
{
9+
// These unit tests allow reproducing the "Value cannot be null. (Parameter 'source')" exception discussed here:
10+
// https://github.com/Http-Multipart-Data-Parser/Http-Multipart-Data-Parser/issues/104
11+
// https://github.com/Http-Multipart-Data-Parser/Http-Multipart-Data-Parser/issues/102
12+
// https://github.com/Http-Multipart-Data-Parser/Http-Multipart-Data-Parser/issues/66
13+
14+
// This problem was caused by the fact that developers would read the content of their stream prior to parsing their content
15+
// which had the side effect of moving the 'Position' to the end of the stream and therefore we would get a null value when
16+
// reading the content of the stream in StreamingMultipartFormDataParser.DetectBoundary (and DetectBoundaryAsync as well).
17+
// As of March 2022, this problem was resolved by throwing a more descriptive exception.
18+
public class StreamPositionHasBeenMoved
19+
{
20+
private static readonly TestData _testCase = new TestData(
21+
"Hello world",
22+
Enumerable.Empty<ParameterPart>().ToList(),
23+
Enumerable.Empty<FilePart>().ToList()
24+
);
25+
26+
public StreamPositionHasBeenMoved()
27+
{
28+
foreach (var filePart in _testCase.ExpectedFileData)
29+
{
30+
filePart.Data.Position = 0;
31+
}
32+
}
33+
34+
[Fact]
35+
public void End_of_stream_and_boundary_is_known()
36+
{
37+
using (Stream stream = TestUtil.StringToStream(_testCase.Request, Encoding.UTF8))
38+
{
39+
// Move the Position to the end of the stream
40+
var sr = new StreamReader(stream);
41+
var content = sr.ReadToEnd();
42+
43+
// When the developer provides the boundary, the parser does not have to determine the boundary
44+
// and therefore the DetectBoundary method is not invoked which avoids the problem altogether.
45+
// However, the parser is unable to find the provided boundary and throws a meaningful exception.
46+
Assert.Throws<MultipartParseException>(() => MultipartFormDataParser.Parse(stream, "MyBoundary"));
47+
}
48+
}
49+
50+
[Fact]
51+
public void End_of_stream_and_boundary_is_unknown()
52+
{
53+
using (Stream stream = TestUtil.StringToStream(_testCase.Request, Encoding.UTF8))
54+
{
55+
// Move the Position to the end of the stream
56+
var sr = new StreamReader(stream);
57+
var content = sr.ReadToEnd();
58+
59+
// As of March 2022, the problem was resolved by throwing a more descriptive exception
60+
Assert.Throws<MultipartParseException>(() => MultipartFormDataParser.Parse(stream));
61+
}
62+
}
63+
64+
[Fact]
65+
public void Middle_of_stream_and_boundary_is_known()
66+
{
67+
using (Stream stream = TestUtil.StringToStream(_testCase.Request, Encoding.UTF8))
68+
{
69+
// Move the Position to an arbitrary location
70+
stream.Position = 3;
71+
72+
// When the developer provides the boundary, the parser does not have to determine the boundary
73+
// and therefore the DetectBoundary method is not invoked which avoids the problem altogether.
74+
// However, the parser is unable to find the provided boundary and throws a meaningful exception.
75+
Assert.Throws<MultipartParseException>(() => MultipartFormDataParser.Parse(stream, "MyBoundary"));
76+
}
77+
}
78+
79+
[Fact]
80+
public void Middle_of_stream_and_boundary_is_unknown()
81+
{
82+
using (Stream stream = TestUtil.StringToStream(_testCase.Request, Encoding.UTF8))
83+
{
84+
// Move the Position to an arbitrary location
85+
stream.Position = 3;
86+
87+
// As of March 2022, the problem was resolved by throwing a more descriptive exception
88+
Assert.Throws<MultipartParseException>(() => MultipartFormDataParser.Parse(stream));
89+
}
90+
}
91+
92+
[Fact]
93+
public async Task End_of_stream_and_boundary_is_known_async()
94+
{
95+
using (Stream stream = TestUtil.StringToStream(_testCase.Request, Encoding.UTF8))
96+
{
97+
// Move the Position to the end of the stream
98+
var sr = new StreamReader(stream);
99+
var content = await sr.ReadToEndAsync().ConfigureAwait(false);
100+
101+
// When the developer provides the boundary, the parser does not have to determine the boundary
102+
// and therefore the DetectBoundary method is not invoked which avoids the problem altogether.
103+
// However, the parser is unable to find the provided boundary and throws a meaningful exception.
104+
await Assert.ThrowsAsync<MultipartParseException>(() => MultipartFormDataParser.ParseAsync(stream, "MyBoundary")).ConfigureAwait(false);
105+
}
106+
}
107+
108+
[Fact]
109+
public async Task End_of_stream_and_boundary_is_unknown_async()
110+
{
111+
using (Stream stream = TestUtil.StringToStream(_testCase.Request, Encoding.UTF8))
112+
{
113+
// Move the Position to the end of the stream
114+
var sr = new StreamReader(stream);
115+
var content = await sr.ReadToEndAsync().ConfigureAwait(false);
116+
117+
// As of March 2022, the problem was resolved by throwing a more descriptive exception
118+
await Assert.ThrowsAsync<MultipartParseException>(() => MultipartFormDataParser.ParseAsync(stream)).ConfigureAwait(false);
119+
}
120+
}
121+
122+
[Fact]
123+
public async Task Middle_of_stream_and_boundary_is_known_async()
124+
{
125+
using (Stream stream = TestUtil.StringToStream(_testCase.Request, Encoding.UTF8))
126+
{
127+
// Move the Position to an arbitrary location
128+
stream.Position = 3;
129+
130+
// When the developer provides the boundary, the parser does not have to determine the boundary
131+
// and therefore the DetectBoundary method is not invoked which avoids the problem altogether.
132+
// However, the parser is unable to find the provided boundary and throws a meaningful exception.
133+
await Assert.ThrowsAsync<MultipartParseException>(() => MultipartFormDataParser.ParseAsync(stream, "MyBoundary")).ConfigureAwait(false);
134+
}
135+
}
136+
137+
[Fact]
138+
public async Task Middle_of_stream_and_boundary_is_unknown_async()
139+
{
140+
using (Stream stream = TestUtil.StringToStream(_testCase.Request, Encoding.UTF8))
141+
{
142+
// Move the Position to an arbitrary location
143+
stream.Position = 3;
144+
145+
// As of March 2022, the problem was resolved by throwing a more descriptive exception
146+
await Assert.ThrowsAsync<MultipartParseException>(() => MultipartFormDataParser.ParseAsync(stream)).ConfigureAwait(false);
147+
}
148+
}
149+
}
150+
}

‎Source/HttpMultipartParser/StreamingMultipartFormDataParser.cs‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,14 @@ private static string DetectBoundary(RebufferableBinaryReader reader)
279279
{
280280
// Presumably the boundary is --|||||||||||||| where -- is the stuff added on to
281281
// the front as per the protocol and ||||||||||||| is the part we care about.
282-
string boundary = string.Concat(reader.ReadLine().Skip(2));
282+
var line = reader.ReadLine();
283+
284+
// The line must not be empty and must starts with "--".
285+
if (string.IsNullOrEmpty(line)) throw new MultipartParseException("Unable to determine boundary: unexpected end of stream");
286+
else if (!line.StartsWith("--")) throw new MultipartParseException("Unable to determine boundary: content does not start with a valid multipart boundary");
287+
288+
// Remove the two dashes
289+
string boundary = line.Substring(2);
283290

284291
// If the string ends with '--' it means that we found the "end" boundary and we
285292
// need to trim the two dashes to get the actual boundary
@@ -315,7 +322,13 @@ private static async Task<string> DetectBoundaryAsync(RebufferableBinaryReader r
315322
// Presumably the boundary is --|||||||||||||| where -- is the stuff added on to
316323
// the front as per the protocol and ||||||||||||| is the part we care about.
317324
var line = await reader.ReadLineAsync(cancellationToken).ConfigureAwait(false);
318-
string boundary = string.Concat(line.Skip(2));
325+
326+
// The line must not be empty and must starts with "--".
327+
if (string.IsNullOrEmpty(line)) throw new MultipartParseException("Unable to determine boundary: either the stream is empty or we reached the end of the stream");
328+
else if (!line.StartsWith("--")) throw new MultipartParseException("Unable to determine boundary: content is not a valid multipart boundary");
329+
330+
// Remove the two dashes
331+
string boundary = line.Substring(2);
319332

320333
// If the string ends with '--' it means that we found the "end" boundary and we
321334
// need to trim the two dashes to get the actual boundary.

0 commit comments

Comments
 (0)