dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.34k stars 9.98k forks source link

IFormFile binding error #28273

Open daping123 opened 3 years ago

daping123 commented 3 years ago

Describe the bug

I have a model with IFormFIle property and other simple type properties.It could pass to the controller with [ApiController] and [FromForm] successfully.But If I only pass IFormFile property in model,it could not pass data to controller.I find a solution,when I remove [ApiController] and [FromForm],it works.

To Reproduce

using asp.net core 3.1

Model:

    public class ProductImageDetailsDto
    {
        public IFormFile ProductImage { get; set; }

        public bool IsDefaultImage { get; set; }

    }

Controller:

    [ApiController]
    [Route("[controller]")]
    public class WeatherForecastController : ControllerBase
    {
        [HttpPut("UpdateProductImagesByProductId/{id}")]
        public async Task<IActionResult> UpdateProdutImagesByProductId(Guid id,[FromForm]List<ProductImageDetailsDto> productImages)
        {

            return Ok();
        }
    }

SO Reference:

https://stackoverflow.com/questions/65085522/how-to-upload-multiple-files-using-multipart-form-data-media-type-in-net-core-w/65087114

mkArtakMSFT commented 3 years ago

Thank you for filing this issue. In order for us to investigate this issue, please provide a minimalistic repro project (ideally a GitHub repo) that illustrates the problem.

daping123 commented 3 years ago

Thank you for filing this issue. In order for us to investigate this issue, please provide a minimalistic repro project (ideally a GitHub repo) that illustrates the problem.

@mkArtakMSFT I have create a github repo here: https://github.com/daping123/ModelBindingProj

When you post only ProductImage in postman,I will receive data count=0. When you post both ProductImage and IsDefaultImage,I will receive the correct data.

Why?Does IFormFile run the different pipeline in model binding?And my workaround is that,when I remove [ApiController] and [FromForm], I could pass ProductImage without IsDefaultImage.For my workaround,i am also confused,why i can't add [FromForm]?

And I find if I receive Model instead of List in backend.Using [FromForm] is OK.

ghost commented 3 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

coderealm commented 3 years ago

I have spent the last 3 days looking to find how others have solved this problem but nothing out there works. Guess it is bug. When will this be released to production please?

coderealm commented 3 years ago

When testing your solution, please include HttpPost as his is HttpPut. I had similar issue posting to the server A solution for a scenario like below would be nice !!!


[HttpPost]
public async Task<IActionResult> GetModelWithMediaUsingIFormFile(Employee employee)
{

    return Ok();
}

// Model for model binding

public class Employee
{
    public string FirstName {get; set;}
    public string LastName {get; set;}
    public IEnumerable<IFormFile> ChristmasPartyPhotosAndVideos { get; set;}
}
ghost commented 2 years ago

Thanks for contacting us. We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 2 years ago

Thanks for contacting us. We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

Danthar commented 10 months ago

I just spend way to long trying to solve this exact issue. Im on aspnetcore 7 at the moment.

I can confirm its a bug.

My example is practically the same as the example already provided in this issue. The only difference is that i use the [FromBody] binding attribute. But as far as i know it does not really matter.

Symptoms are:

Cause:

The default modelbinder implementation contains bugs that cause it to mishandle multipart/form POST requests. I tried to debug the modelbinders and find where it fails, but it seems to fail somewhere before it even gets to the ComplexObjectModelBinder. Which is where you would expect this to end up.

Possible solutions/workaround, and stuff that highlights the default modelbinders are at fault.

1: When you specify a custom Modelbinder, it properly invokes this modelbinder and then continues without issues. (obviously if your custom modelbinder has issues, you still end up with an empty model object)

2: If you remove the parameters from the ActionMethod, Then use the TryUpdateModelAsync it works like a charm.

Example:


public async Task<ActionResult<string>> MyUploadTest() {
  var model = new MyUploadTestModel();

await TryUpdateModelAsync(model);

//now the model is bound correctly and you can use the ModelState property to determine if there are validation issues or not

return "OK";
}

public class MyUploadTest() {
   public string SomeName {get;set;}
public string[] MoreOptionalValues {get;set;}

[Required]
[ModelBinder(typeof(JsonBinder))]
public MyOtherComplextModel ComplexParameter {get;set;}

//Using IFormFileCollection or this does not matter, both work
public List<IFormFile>  Files {get;set;}
}

Solution 2 is what im using currently. Because it allows me to have the framework deal with the binding logic. However because this works, it only proves my point that its somewhere in the default ModelBinding infrastructure that this is broken.

You would expect that the following example would work, since its practically the same (except it has the extra parameter name to bind into)

public async Task<ActionResult<string>> MyUploadTest([FromBody] MyUploadTestModel options) {
// this example always fails, usually with a HTTP statuscode 415
return "OK";
}