Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Unable to use IFormFile with ApiController in 2.1 #8311

Closed
Eilon opened this issue Aug 22, 2018 · 17 comments
Closed

Unable to use IFormFile with ApiController in 2.1 #8311

Eilon opened this issue Aug 22, 2018 · 17 comments
Assignees
Labels
3 - Done bug cost: XS Will take up to half a day to complete

Comments

@Eilon
Copy link
Member

Eilon commented Aug 22, 2018

From @mitchellj on Thursday, 09 August 2018 20:06:10

When we are trying to use IFormFile it works perfectly when we are not using [ApiController], as soon as we introduce ApiController one of two things happens

  1. If we do not prefix with [FromForm] we get a Serialization exception which generates a Bad Request response.
  2. If we do prefix FromForm to the IFormFile then it becomes null

In order to get IFormFile to work we need to remove ApiController completely in which case the IFormFile becomes completely propagated correctly.

Copied from original issue: dotnet/aspnetcore#3403

@mkArtakMSFT
Copy link
Member

Thanks for contacting us, @mitchellj.
This is because we do not special case the IFormFile type and treat it as a complex type. ApiController conventions for complex types are that by default they're treated as FromBody.
@pranavkm, seems this is a good candidate to be special-cased.

@mkArtakMSFT mkArtakMSFT added bug 1 - Ready PRI: 1 - Required Must be handled in a reasonable time labels Aug 22, 2018
@mkArtakMSFT mkArtakMSFT added this to the 2.2.0-preview2 milestone Aug 22, 2018
@pranavkm
Copy link
Contributor

  1. Do you have your compatibility version set to 2.1? See https://docs.microsoft.com/en-us/aspnet/core/migration/20_21?view=aspnetcore-2.1#changes-to-startup
  2. We had an issue specifically with collection of IFormFile - BindingSource for IEnumerable<IFormFile> ApiController parameters on is incorrectly inferred as FromBody #7770 that was fixed in 2.2

@pranavkm pranavkm added investigate and removed 1 - Ready PRI: 1 - Required Must be handled in a reasonable time bug labels Aug 23, 2018
@Schmaga
Copy link

Schmaga commented Aug 25, 2018

I ran into the same problem. And strangely, when not using any controller parameter and trying to access or iterate Request.Form.Files the thread would just die without ever responding to the browser or even entering any exception handling blocks that are around the code. Feels kind of weird. My ASP.NET Core version was 2.1.2.

Until this is solved, I was able to help myself with a workaround by explicitly stating the name of the form parameter using the [FromForm] attribute like this:

public async Task<IActionResult> UploadLogo([FromForm(Name = "file")] IFormFile file)        
{
     ...
}

After that for some reason the binding worked. I discovered this behavior by accident after having a hard time getting the file upload from an angular 6 application to work. Hope it helps someone else. The link to the corresponding stackoverflow answer is: https://stackoverflow.com/a/52017637/8007572

@pranavkm
Copy link
Contributor

@Schmaga did the compat switch work for you?

@mitchellj
Copy link

Sorry I was out of the country for a while, I will be back in a couple of days and try the compat switches and report back. Compatibility was NOT set previously so that is most likely the issue will confirm though.

@pranavkm
Copy link
Contributor

@mitchellj feel free to reopen this issue if you still see it after changing the compat version.

@Schmaga
Copy link

Schmaga commented Aug 30, 2018 via email

@Schmaga
Copy link

Schmaga commented Sep 5, 2018

@pranavkm I returned from vacation and checked with the compat switch set to 2.1.

It now works, but I also have to remove the [FromForm] Attribute completely from the controller's IFormFile parameter, otherwise it is still null.

I think it might make sense to better document what exactly the compat switch affects, because I would not have thought it would produce such strange behavior as described in my above post.

@pranavkm
Copy link
Contributor

pranavkm commented Sep 5, 2018

It now works, but I also have to remove the [FromForm] Attribute completely from the controller's IFormFile parameter, otherwise it is still null.

That isn't intended. Do you have a repro app for this?

@pranavkm pranavkm reopened this Sep 5, 2018
@Schmaga
Copy link

Schmaga commented Sep 5, 2018

Not yet, but I could extract it from our project for you. What format is best? Will it be enough to paste the relevant snippets here or do you need a zip file with a project or something?

This is my first .NET OSS issue, so sorry if the answer might be obvious. And I didn't find anything specific about providing example code in the contribution guidelines :)

@pranavkm
Copy link
Contributor

pranavkm commented Sep 5, 2018

A sample repo on github works really well to share code. If that's difficult, any of your suggestions work fine.

@Eilon Eilon modified the milestones: 2.2.0-preview2, 2.2.0-preview3 Sep 5, 2018
@Eilon
Copy link
Member Author

Eilon commented Sep 5, 2018

Moved to preview3 for now.

@mitchellj
Copy link

Sorry back now, I can confirm as @pranavkm has found as well that

  • Decorating the controller with [ApiController] and the IFormFile with [FromForm] gives us a null IFormFile.
  • Decorating the controller with [ApiController] and not decorating the IFormFile parameter binds the IFormFile correctly

@Schmaga
Copy link

Schmaga commented Sep 6, 2018

Thanks @pranavkm. Will provide a repo then, hopefully in the next few days.

@pranavkm
Copy link
Contributor

pranavkm commented Sep 7, 2018

Looks like yet another case that we need to special case in https://github.com/aspnet/Mvc/blob/release/2.2/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs#L215-L233. FormFileModelBinder requires the model name to match which the provider messes with.

For 2.1, I'd recommend not specifying the attribute or if you do specify the attribute, also specify the name .e.g [FromForm("file")].

@pranavkm pranavkm added the bug label Sep 7, 2018
@pranavkm pranavkm added 1 - Ready cost: XS Will take up to half a day to complete and removed investigate labels Sep 7, 2018
@Schmaga
Copy link

Schmaga commented Sep 7, 2018

@pranavkm here is the link to the repositry: https://github.com/Schmaga/aspnet_issue_8311

You can play around with removing the compat switch and/or the attribute a bit.

A bit late, but I hope it still helps.

@ranouf
Copy link

ranouf commented Oct 9, 2018

I have made a repo to reproduce the problem and thanks to @dougbu we have found the solution. Let take a look there: https://github.com/ranouf/TestMultipleUpload

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3 - Done bug cost: XS Will take up to half a day to complete
Projects
None yet
Development

No branches or pull requests

6 participants