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

C# Client Posting Multipart/Form-Data To C# API Endpoint Throws InvalidDataException #10503

Closed
ideoclickVanessa opened this issue May 23, 2019 · 12 comments
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates investigate
Milestone

Comments

@ideoclickVanessa
Copy link

Describe the bug

I have a C# API endpoint that accepts files for processing, defined like so:

//POST api/Ticket/Attachment
[HttpPost("Attachment")]
[ProducesResponseType(StatusCodes.Status200OK)]
public async Task<IActionResult> PostAttachment(int ticketId, [FromForm]IFormFileCollection files)

When I call this endpoint from Postman, the endpoint successfully receives / processes the files.
However, I receive an InvalidDataException: Multipart body length limit 16384 exceeded. exception when I call the endpoint from a C# client using this code:

public async Task PostAttachmentAsync(int ticketID, IFormFileCollection files, CancellationToken cancellationToken = default)
{
	using (HttpRequestMessage request = new HttpRequestMessage())
	{
		string boundary = $"{Guid.NewGuid().ToString()}";
		MultipartFormDataContent requestContent = new MultipartFormDataContent(boundary);
		requestContent.Headers.Remove("Content-Type");
		// The two dashes in front of the boundary are important as the framework includes them when serializing the request content.
		requestContent.Headers.TryAddWithoutValidation("Content-Type", $"multipart/form-data; boundary=--{boundary}");

		foreach (IFormFile file in files)
		{
			StreamContent streamContent = new StreamContent(file.OpenReadStream());
			requestContent.Add(streamContent, file.Name, file.FileName);
			streamContent.Headers.ContentDisposition.FileNameStar = "";
		}

		request.Content = requestContent;
		request.Method = new HttpMethod("POST");
		request.RequestUri = new Uri($"api/v3/Services/WorkItem/Ticket/Attachment?ticketID={ticketID}", UriKind.Relative);

		HttpClient client = await CreateHttpClientAsync(cancellationToken).ConfigureAwait(false);

		using (HttpResponseMessage response =
			await client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false)
		)
		{
			if (response == null)
				throw new PlatformAPIClientException("Could not retrieve a response.");

			Dictionary<string, IEnumerable<string>> responseHeaders = Enumerable.ToDictionary(response.Headers, h => h.Key, h => h.Value);
			if (response.Content?.Headers != null)
			{
				Dictionary<string, IEnumerable<string>> contentHeaders = Enumerable.ToDictionary(response.Content.Headers, h => h.Key, h => h.Value);
				responseHeaders.Concat(contentHeaders);
			}

			int responseStatusCode = (int)response.StatusCode;
			switch (responseStatusCode)
			{
				case StatusCodes.Status200OK:
				case StatusCodes.Status204NoContent:
					return;

				case StatusCodes.Status400BadRequest:
					string badRequestResponse = response.Content == null ? "No Response Content" : 
						await response.Content.ReadAsStringAsync().ConfigureAwait(false);
					ValidationProblemDetails validationResult = default;
					try
					{
						validationResult = JsonConvert.DeserializeObject<ValidationProblemDetails>(badRequestResponse, _settings.Value);
					}
					catch(Exception ex)
					{
						throw new PlatformAPIClientException("Could not deserialize the response body.", responseStatusCode, badRequestResponse, 
							responseHeaders, ex);
					}
					throw new PlatformAPIClientException<ValidationProblemDetails>("A server side error occurred.", responseStatusCode, 
						badRequestResponse, responseHeaders, validationResult, innerException: null);

				case StatusCodes.Status401Unauthorized:
				case StatusCodes.Status403Forbidden:
				case StatusCodes.Status500InternalServerError:
					string errorResponse = response.Content == null ? "No Response Content" : 
						await response.Content.ReadAsStringAsync().ConfigureAwait(false);
					ProblemDetails problemResult = default;
					try
					{
						problemResult = JsonConvert.DeserializeObject<ProblemDetails>(errorResponse, _settings.Value);
					}
					catch(Exception ex)
					{
						throw new PlatformAPIClientException("Could not deserialize the response body.", responseStatusCode, errorResponse, 
							responseHeaders, ex);
					}
					throw new PlatformAPIClientException<ProblemDetails>("A server side error occurred.", responseStatusCode,
						errorResponse, responseHeaders, problemResult, innerException: null);

				default:
					string unexpectedResponse = response.Content == null ? "No Response Content" :
						await response.Content.ReadAsStringAsync().ConfigureAwait(false);
					throw new PlatformAPIClientException($"The HTTP status code ({responseStatusCode}) of the response was not expected.",
						responseStatusCode, unexpectedResponse, responseHeaders, innerException: null);
			}
		}
	}
}

Google / Stack Overflow suggest that the exception should be resolved by using either the attribute [RequestFormLimits(MultipartBodyLengthLimit = long.MaxValue)] or configured globally in Startup:

services.Configure<FormOptions>(options =>
{
	options.MultipartBodyLengthLimit = long.MaxValue;
});

However, neither suggested solution prevents the InvalidDataException from being thrown when the endpoint is called by the C# client.

Can you please provide guidance as to whether this is a bug in the framework or my C# client code?

To Reproduce

Steps to reproduce the behavior:

  1. Using this version of ASP.NET Core '2.2'

Expected behavior

The C# client successfully posts the request without triggering the InvalidDataException.

Additional context

dotnet-info.txt
testfile.jpg
successful-postman-request.txt
failed-csharp-request.txt

@Eilon Eilon added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label May 23, 2019
@pranavkm
Copy link
Contributor

cc @Tratcher

@Tratcher
Copy link
Member

Notes:

I'm pretty sure these parts are unnecessary:

requestContent.Headers.Remove("Content-Type");
		// The two dashes in front of the boundary are important as the framework includes them when serializing the request content.
		requestContent.Headers.TryAddWithoutValidation("Content-Type", $"multipart/form-data; boundary=--{boundary}");

Those leading dashes are not logically part of the boundary value.

@Tratcher
Copy link
Member

Yes, it's your boundary that's the problem.
Multipart bodies have a preamble section before the first boundary marker. We drain that using a fixed limit buffer (also 16kb):
https://github.com/aspnet/AspNetCore/blob/4c79e7fdc017be6ec578f2098168d2646d1cd48d/src/Http/WebUtilities/src/MultipartReader.cs#L48-L50. Because it can't properly match the boundary you sent it reads all the way to the end of the message and hits the limit.

@ideoclickVanessa
Copy link
Author

I needed the boundary workaround:

requestContent.Headers.Remove("Content-Type");
// The two dashes in front of the boundary are important as the framework includes them when serializing the request content.
requestContent.Headers.TryAddWithoutValidation("Content-Type", $"multipart/form-data; boundary=--{boundary}");

otherwise, the file won't make it to the endpoint.

As you can see in the newly attached failed request, the boundary in the Content-Type is set as Content-Type: multipart/form-data; boundary="0ff7b36f-2353-4fbf-8158-3954a54c102e" while the serialized body header has the boundary listed as --0ff7b36f-2353-4fbf-8158-3954a54c102e. This results in the endpoint thinking there were no files (IFormFilesCollection request param from OP) passed to it via this code:

if ((files?.Count ?? 0) == 0)
{
	ValidationProblemDetails validationProblem = new ValidationProblemDetails();
	validationProblem.Errors["files"] = new[] { "No files found." };

	return StatusCode(StatusCodes.Status400BadRequest, validationProblem);
}

failed-csharp-request-no-boundary-workaround.txt
failed-csharp-response-no-boundary-workaround.txt

@Tratcher
Copy link
Member

You're solving the wrong problem. Look at your postman example. The boundary field is
"--------------------------848155269508837432319981", and then the pre-boundary marker is
"----------------------------848155269508837432319981" (note the two extra leading dashes)
and the post boundary is:
"----------------------------848155269508837432319981--" (note the two extra leading and trailing dashes). Those extra dashes are part of the multipart formatting. Your workaround is just causing it to fail earlier with a different error, it's not fixing anything. Note we have tests using MultipartFormDataContent that don't require any customization.

The real question here is why do you get {"errors":{"files":["No files found."]},"title":"One or more validation errors occurred.","status":400} when you leave the boundary alone. Something else is wrong with the message.

How about removing IFormFileCollection as the method parameter and calling HttpContext.Request.ReadFormAsync() directly? Then inspect the resulting form to see how it was parsed.

@ideoclickVanessa
Copy link
Author

Updating the endpoint to:

[HttpPost("Attachment")]
[ProducesResponseType(StatusCodes.Status200OK)]
public async Task<IActionResult> PostAttachment(int ticketId/*, [FromForm]IFormFileCollection files*/)
{
	IFormCollection form = await HttpContext.Request.ReadFormAsync();
	IFormFileCollection files = form?.Files;

	if ((files?.Count ?? 0) == 0)
	{
		ValidationProblemDetails validationProblem = new ValidationProblemDetails();
		validationProblem.Errors["files"] = new[] { "No files found." };

		return StatusCode(StatusCodes.Status400BadRequest, validationProblem);
	}

	// Proccess attachments

	return Ok();
}

did in fact work successfully when called via the C# client.

However, removing the [FromForm]IFormFileCollection files parameter means that it's no longer shown in the generated swagger document. This also means that it wouldn't exist in the generated C# client from said swagger document; thus, forcing me to write / maintain a custom client method for this endpoint.

Can you please provide more detail into why the [FromForm]IFormFileCollection files parameter doesn't work as expected; particularly since this would be the ideal way to define the endpoint?

@Tratcher
Copy link
Member

Tratcher commented May 24, 2019

@pranavkm?

The examples I've seen used IEnumerable<IFormFile>, not IFormFileCollection.

@ideoclickVanessa
Copy link
Author

FWIW: Switching from IFormFileCollection to IEnumerable<IFormFile> as the request parameter has the same issue where the files cannot be found. Issue aside, this makes sense since IFormFileCollection implements IEnumerable<IFormFile>.

@pranavkm
Copy link
Contributor

@ideoclickVanessa I tried this with our most recent 3.0 builds and it works just fine. We fixed a couple of form file related issues in 3.0, perhaps that could be it.

The other possible reason could be that model binding requires that the name of all of the file instances must match the action parameter name. In this case, MVC would expect all of the individual multi-part entries to be named files going by [FromForm]IFormFileCollection files. I don't think that's with the way you construct your request:

foreach (IFormFile file in files)
{
    StreamContent streamContent = new StreamContent(file.OpenReadStream());
    requestContent.Add(streamContent, file.Name, file.FileName);
    streamContent.Headers.ContentDisposition.FileNameStar = "";
}

Regardless, closing this since this issue seems to be resolved with the most recent builds.

@ideoclickVanessa
Copy link
Author

ideoclickVanessa commented Aug 27, 2019

The other possible reason could be that model binding requires that the name of all of the file instances must match the action parameter name. In this case, MVC would expect all of the individual multi-part entries to be named files going by [FromForm]IFormFileCollection files.

@pranavkm
If I understand this correctly: You are saying that if I have 3 files, they would all need to be named files since that's the name of my IFormFileCollection parameter. Is this correct?

If that is indeed what you are saying, why does the endpoint work successfully from Postman when using differently named files?

Also, could you update my foreach statement to reflect the changes that are needed so that I can better understand what you're trying to tell me?

@pranavkm
Copy link
Contributor

You are saying that if I have 3 files, they would all need to be named files since that's the name of my IFormFileCollection parameter. Is this correct?

Yup. By the way, these are names of the form file in the request, not the actual file names on disk or the file names in the content-disposition.

foreach (IFormFile file in files)
{
    StreamContent streamContent = new StreamContent(file.OpenReadStream());
    requestContent.Add(streamContent, name: "files", fileName: file.FileName);
}

@ideoclickVanessa
Copy link
Author

Ah.... ok. That makes much more sense. Thanks :)

@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates investigate
Projects
None yet
Development

No branches or pull requests

5 participants