aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

FormFeature forces uploaded files to specify a filename directive in Content-Disposition header

Open andy1547 opened this issue 1 year ago • 6 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues - I couldn't add my input to it due to it being locked down to just collaborators, see https://github.com/dotnet/aspnetcore/issues/19295

Describe the bug

We're running into issues uploading files that lack a filename directive, as they get treated as form data instead of files. This causes them to be buffered into memory which can cause performance issues for large files, whereas when they contain a filename directive they are buffered to disk at ASPNETCORE_TEMP if they exceed 64KB. We don't have control over the clients uploading these files, so we have to cater to both cases.

We currently use InputFormatters to parse the request before it reaches the controller, trying to support file uploads that lack the filename directive would require special handling such as:

const string sectionName = "fileUpload";
var provider = await HttpContext.Request.ReadFormAsync(HttpContext.RequestAborted);
if (provider.Files[sectionName] is { } file) // filename directive supplied
{
	await using var stream = file.OpenReadStream();
	// do something
}
else if (provider.TryGetValue(sectionName, out var values)) // filename directive not supplied, get from form data
{
	var value = values.First();
       // TODO: convert to byte stream...
}

The opinionated line of code is https://github.com/dotnet/aspnetcore/blob/2ce54c68b2abfa66974a4e75cf80e61203180ce4/src/Http/Http/src/Features/FormFeature.cs#L219

To workaround this, we've disabled the in-built behaviour, and essentially copied the FormFeature implementation, with a change so all sections are treated as IFormFile with readable Streams.

The RFC 7578 spec states that a filename isn't mandatory for file uploads see https://datatracker.ietf.org/doc/html/rfc7578#section-4.2. Likewise the MDN doc states it's optional https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition#filename

Expected Behavior

IFormCollection.Files to contain files uploaded without the filename directive.

.NET Version

v8.0.100

andy1547 avatar Jan 08 '24 10:01 andy1547

How do you distinguish between form fields and files without the filename parameter? Or do you assume all fields are files?

Tratcher avatar Jan 08 '24 18:01 Tratcher

Luckily our application doesn't need to process form fields, we only take advantage of WebAPI features rather than the full MVC featureset. Our typical requests contain 1-MANY file parts, with an associated JSON array part contain metadata about each file. At this time I don't have an API change suggestion that would easily cover this edge case.

andy1547 avatar Jan 09 '24 15:01 andy1547

Try this: https://learn.microsoft.com/en-us/aspnet/core/mvc/models/file-uploads?view=aspnetcore-8.0#upload-large-files-with-streaming

Tratcher avatar Jan 09 '24 17:01 Tratcher

Try this: https://learn.microsoft.com/en-us/aspnet/core/mvc/models/file-uploads?view=aspnetcore-8.0#upload-large-files-with-streaming

We originally used that documentation page as inspiration (e.g. the DisableFormValueModelBindingAttribute, so we could disable the in-built behavior and roll our own copy of FormFeature with the small caveat noted above).

I've created this Github issue to bring attention to the API not properly supporting file uploads that lack the optional filename directive, and to discuss whether future enhancements can be made (perhaps configurable in FormOptions class) to cater to this use-case.

andy1547 avatar Jan 09 '24 18:01 andy1547

The trouble is that without the file name it's ambiguous if the content is a file or a field. I'm not sure there's anything we could do about that, the developer would have to write code to decide which is which regardless.

Tratcher avatar Jan 10 '24 19:01 Tratcher

But wouldn't it be possible to parse the controllers signature and if there is a parameter of type IFormFile, parse it as a file?

craffael avatar Aug 26 '24 09:08 craffael