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

The introduction of FormFileValueProvider broke the documented large file upload #14518

Closed
Tragetaschen opened this issue Sep 27, 2019 · 5 comments
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Docs This issue tracks updating documentation

Comments

@Tragetaschen
Copy link
Contributor

Tragetaschen commented Sep 27, 2019

Describe the bug

This bug tries to tie a couple of issues and provide some search terms.

#9510 and #12847 introduced FormFileValueProviderFactory in the ResourceExecutingContext's ValueProviderFactories.

On the other hand, the 2.2 documentation for streaming file upload describes which ValueProviderFactories to remove in order to make large file uploads work.

The documentation team is already updating the docs: dotnet/AspNetCore.Docs#13344

Should this be part of the breaking changes in 3.0?

To Reproduce

Run the documentation's 2.2 code on 3.0 and notice that the request fails with

System.IO.InvalidDataException: Multipart body length limit 134217728 exceeded.
   at Microsoft.AspNetCore.WebUtilities.MultipartReaderStream.UpdatePosition(Int32 read)
   at Microsoft.AspNetCore.WebUtilities.MultipartReaderStream.ReadAsync(Byte[] buffer, Int32 offset, Int32 count, CancellationToken cancellationToken)
   at Microsoft.AspNetCore.WebUtilities.FileBufferingReadStream.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.Http.Features.FormFeature.InnerReadFormAsync(CancellationToken cancellationToken)
   at Microsoft.AspNetCore.Mvc.ModelBinding.FormFileValueProviderFactory.AddValueProviderAsync(ValueProviderFactoryContext context, HttpRequest request)
   at Microsoft.AspNetCore.Mvc.ModelBinding.CompositeValueProvider.CreateAsync(ActionContext actionContext, IList`1 factories)
   at Microsoft.AspNetCore.Mvc.ModelBinding.CompositeValueProvider.CreateAsync(ControllerContext controllerContext)
   at Microsoft.AspNetCore.Mvc.Controllers.ControllerBinderDelegateProvider.<>c__DisplayClass0_0.<<CreateBinderDelegate>g__Bind|0>d.MoveNext()
[…]

Expected behavior

Streaming file uploads should work.

To fix

RemoveType the FormFileValueProviderFactory in the resource filter attribute as well.

@pranavkm
Copy link
Contributor

Thanks @Tragetaschen. Does updating the filter to remove the additional value provider suffice?

@pranavkm pranavkm added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Sep 27, 2019
@Tragetaschen
Copy link
Contributor Author

Yes, removing this additional factory

         {
             var factories = context.ValueProviderFactories;
             factories.RemoveType<FormValueProviderFactory>();
+            factories.RemoveType<FormFileValueProviderFactory>();
             factories.RemoveType<JQueryFormValueProviderFactory>();
         }

re-enables my software update (~230MiB) upload again.

@pranavkm
Copy link
Contributor

@guardrex any chance you could incorporate this after you're done merging your other PR?

@guardrex
Copy link
Contributor

Yes ... it was left as a comment on dotnet/AspNetCore.Docs#13344 (comment) for the 3.0 updates to the File Upload topic.

@pranavkm
Copy link
Contributor

pranavkm commented Oct 2, 2019

Let's use the other docs issue to track addressing this. This is a known change, and updating the docs would be the way to go.

@pranavkm pranavkm closed this as completed Oct 2, 2019
@pranavkm pranavkm added Docs This issue tracks updating documentation and removed investigate labels Oct 2, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 2, 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 Docs This issue tracks updating documentation
Projects
None yet
Development

No branches or pull requests

4 participants