Considerations for Multipart support off of HttpRequest

[daniel-white]

I'm experimenting with adding Multipart support to HttpRequest. It will be done similarly to how forms are handled, using the same patterns and MultipartReader. The end goal is to allow MVC and Web API to define models as multipart.

I'm hoping for some feedback on the direction.

• How should MultipartSection be handled? Should Microsoft.AspnetCore.Http.Abstractions define its own MultipartSection? Or should we move it from Microsoft.AspnetCore.WebUtilities? There seems to be no clear reference graph for this.
• Should there be a MultipartValues which would work like StringValues?
• Should the MultipartSectionCollection be able to looked up by key and as a list? Not all parts have names.

Thoughts?

[davidfowl]

Are you planning to move to existing API? My high order bit would be to avoid synchronous APIs that donthr IO. That’s one mistake we made with the Form API.

Can you propose an API shape and the usage pattern you’d expect? That will help determine if this makes sense to make a first class API

[daniel-white]

I'm not sure at this point about moving APIs. I'm assuming you are referring to HttpRequest.Form as the offending synchronous API. I've omitted it from my example

Here's a concept API shape

class HttpRequest
{
     public virtual bool HasMultipartContent { get; }
     public virtual Task<IMultipartSectionCollection> ReadMultipartContent(CancellationToken);
}
interface IMultipartSectionCollection : IList<MultipartSection>
{
     MultipartValues this[string key];
     bool TryGetValues(string key, out MultipartValues values);
}
class MultipartSection
{
     string ContentType { get; }
     string ContentDisposition { get; }
     IHeaderDictionary Headers { get; }
     Stream Body { get; }
}

Using it:

HttpRequest request = ...
if (request.HasMultipartContent)
{
     IMultipartCollection collection = await request.ReadMultipartContent();
     ReadAsList(collection);
     // or
     ReadByName(collection);
}

void ReadAsList(IMultipartCollection collection)
{
     foreach (MultipartSection section in collection)
    {
        // do work
    }
}

void ReadByName(IMultipartCollection collection)
{
     if (collection.TryGetValues("name", out MultipartValues values)
    {
       // do work
    }
}

[Tratcher]

This isn't much of an improvement over HttpRequest.Form.File collection. Rather than adding another buffered collection that you can't even index, let's work to expose the unbuffered consumption pattern in a more first class way. This can be done with with new extension methods in Http.Extensions that call the existing WebUtilities APIs. That way you don't have to move anything around or create any new types.

Let's start with this sample:
https://github.com/aspnet/Entropy/blob/15ed884dbf4d7d7ffdb6fee905ddb0ecb831cbd9/samples/Content.Upload.Multipart/Startup.cs
IsMultipartContentType becomes an HttpRequest extension method bool IsMultipart().
The setup code (https://github.com/aspnet/Entropy/blob/15ed884dbf4d7d7ffdb6fee905ddb0ecb831cbd9/samples/Content.Upload.Multipart/Startup.cs#L29-L32) can all be hidden behind a new extension Task<MultipartSection> GetNextMultipartSectionAsync(); The MultipartReader can be created internally on the first call and cached in the feature collection.

I'm not worried about the nested multipart yet, though a similar approach could be applied.

[daniel-white]

Hm @Tratcher... Extension method would be nice. I was going for consistency. I'm not a fan of extension methods as properties. Where would the MultipartReader live since the extension method is in a static context?

[Tratcher]

It can be cached in HttpContext.Features

[aspnet-hello]

This issue was moved to dotnet/aspnetcore#2682