Skip to content
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.

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

MultipartReader cannot detect boundary if the value of the boundary parameter is enclosed in quotation marks #31648

Closed
aetos382 opened this issue Apr 9, 2021 · 4 comments
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue.

Comments

@aetos382
Copy link

aetos382 commented Apr 9, 2021

Describe the bug

When using MultipartReader to read a request, the ReadNextSectionAsync method will throw an exception if the boundary parameter value in the Content-Type header is enclosed in double-quotes.

This is probably because MultipartReader strictly compares the boundary in the request body with the value of the boundary parameter. Since they do not match, it fails to detect the boundary in the body.

If you use the MultipartContent class to construct a multipart request, it will always enclose the boundary parameter value in double-quotes. So, the MultipartReader will not be able to process requests generated by MultipartContent.

RFC 2045 §5.1 specifies that the value of the boundary parameter is the same regardless of the enclosing quotation marks.

Note that the value of a quoted string parameter does not include the quotes.
That is, the quotation marks in a quoted-string are not a part of the value of the parameter, but are merely used to delimit that parameter value.
In addition, comments are allowed in accordance with RFC 822 rules for structured header fields.
Thus the following two forms

  Content-type: text/plain; charset=us-ascii (Plain text)

  Content-type: text/plain; charset="us-ascii"

are completely equivalent.

To Reproduce

You can get the reproduction code in this repository.

Exceptions

Error Message:
   System.IO.IOException : Unexpected end of Stream, the content may have already been read by another component.
  Stack Trace:
     at Microsoft.AspNetCore.WebUtilities.MultipartReaderStream.ReadAsync(Byte[] buffer, Int32 offset, Int32 count, CancellationToken cancellationToken)
   at Microsoft.AspNetCore.WebUtilities.StreamHelperExtensions.DrainAsync(Stream stream, ArrayPool`1 bytePool, Nullable`1 limit, CancellationToken cancellationToken)
   at Microsoft.AspNetCore.WebUtilities.MultipartReader.ReadNextSectionAsync(CancellationToken cancellationToken)
   at MultipartReaderIssue.MultipartReaderTest.<>c__DisplayClass0_0.<<DoTest>b__1>d.MoveNext() in /mnt/c/Users/aetos/Documents/Source/GitHub/aetos382/MultipartReaderIssue/MultipartReaderIssue/MultipartReaderTest.cs:line 42
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.TestHost.HttpContextBuilder.<>c__DisplayClass23_0.<<SendAsync>g__RunRequestAsync|0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.TestHost.ClientHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.SendAsyncCore(HttpRequestMessage request, HttpCompletionOption completionOption, Boolean async, Boolean emitTelemetryStartStop, CancellationToken cancellationToken)
   at MultipartReaderIssue.MultipartReaderTest.DoTest(Boolean removeQuotes) in /mnt/c/Users/aetos/Documents/Source/GitHub/aetos382/MultipartReaderIssue/MultipartReaderIssue/MultipartReaderTest.cs:line 62
--- End of stack trace from previous location ---

Workarounds

When creating an instance of MultipartReader, remove the double-quotes from the value pass to its constructor's boundary parameter, and it will work.

For example:

var reader = new MultipartReader(boundary.Trim('"'), request.Body);

Further technical details

  • ASP.NET Core version
    5.0

  • Include the output of dotnet --info

.NET SDK (reflecting any global.json):
 Version:   5.0.202
 Commit:    db7cc87d51

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.21354
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\5.0.202\

Host (useful for support):
  Version: 5.0.5
  Commit:  2f740adc14

.NET SDKs installed:
  5.0.201 [C:\Program Files\dotnet\sdk]
  5.0.202 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 3.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.13 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download
  • The IDE (VS / VS Code/ VS4Mac) you're running on, and its version
    • VS2019 16.9.3
    • VSCode 1.55.1
@aetos382 aetos382 changed the title MultipartReaderStream will fail to detect boundaries if the boundary parameter is enclosed in quotation marks MultipartReader cannot detect boundary if the value of the boundary parameter is enclosed in quotation marks Apr 9, 2021
@Tratcher
Copy link
Member

Tratcher commented Apr 9, 2021

This is by design, the quotes are not part of the boundary, they're part of the header formatting and need to be removed before calling this API.

How are you getting the boundary from the request? There are a few helper apis for that.

@Tratcher Tratcher added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Apr 9, 2021
@aetos382
Copy link
Author

aetos382 commented Apr 10, 2021

I understand.
If so, I would like a note to that effect to be added to the MultipartReader constructor documentation page.

@Tratcher
Copy link
Member

That content comes from here:

/// <summary>
/// Initializes a new instance of <see cref="MultipartReader"/>.
/// </summary>
/// <param name="boundary">The multipart boundary.</param>
/// <param name="stream">The <see cref="Stream"/> containing multipart data.</param>
public MultipartReader(string boundary, Stream stream)
: this(boundary, stream, DefaultBufferSize)
{
}
/// <summary>
/// Initializes a new instance of <see cref="MultipartReader"/>.
/// </summary>
/// <param name="boundary">The multipart boundary.</param>
/// <param name="stream">The <see cref="Stream"/> containing multipart data.</param>
/// <param name="bufferSize">The minimum buffer size to use.</param>

Feel free to send a clarifying PR.

@dotnet dotnet locked as resolved and limited conversation to collaborators May 10, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue.
Projects
None yet
Development

No branches or pull requests

4 participants