Skip to content

Minimal API with IFormFile throws when request body is missing and Content-Type header is missing or empty #53630

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

Closed
1 task done
balazsmeszegeto opened this issue Jan 26, 2024 · 5 comments · Fixed by #53680
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc

Comments

@balazsmeszegeto
Copy link
Contributor

balazsmeszegeto commented Jan 26, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Having an endpoint with Minimal API and IFormFile:

app.MapPost("/upload", (IFormFile file) => "ok");

Sending a request with incorrect Content-Type, ie:

curl --request POST 'http://localhost:5000/upload' --header 'Content-Type: foo'

will return 415 status, as expected. However, missing or empty (spaces) Content-Type, ie:

curl --request POST 'http://localhost:5000/upload'
curl --request POST 'http://localhost:5000/upload' --header 'Content-Type:'
curl --request POST 'http://localhost:5000/upload' --header 'Content-Type:  '

will throw InvalidOperationException and return 500. I am not 100% familiar with the code in this area (yet), but my understanding is that routing constraint won't exclude the endpoint for missing/empty Content-type, and then parameter binding will throw, while attempting to read HttpRequest.Form. The exception thrown is related to this issue: #49096

Tried changing the signature, with attribute and optional, same behaviour:

app.MapPost("/upload", ([FromForm] IFormFile file) => "ok");
app.MapPost("/upload", ([FromForm] IFormFile? file) => "ok");
app.MapPost("/upload", (IFormFile? file) => "ok");

Expected Behavior

I expect that for endpoints with IFormFile (or its variations like IFormFileCollection) parameters, only valid Content-Type, ie. Content-Type: multipart/form-data; ... will be allowed, excluding missing or empty Content-Type headers. Only valid Content-Type would result in binding parameters from form

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

8.0.101

Anything else?

Checked this issue: #39430 but didn't find this explicitly.

I am happy to contribute, but first would need confirmation that this is indeed a bug and not a design decision I am unaware of 😃

@ghost ghost added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jan 26, 2024
@davidfowl
Copy link
Member

We should return 400 if the form isn't a valid content type rather than throwing (as part of the minimal API codegen).

@balazsmeszegeto
Copy link
Contributor Author

balazsmeszegeto commented Jan 26, 2024

We should return 400 if the form isn't a valid content type rather than throwing (as part of the minimal API codegen).

Just to make it clear, you mean 400, not 415? Because non-empty but invalid already returns 415. Should missing/empty be treated differently?

@davidfowl
Copy link
Member

Actually, now I'm wondering why this isn't working:

if (!httpContext.Request.HasFormContentType)
{
Log.UnexpectedNonFormContentType(httpContext, httpContext.Request.ContentType, throwOnBadRequest);
httpContext.Response.StatusCode = StatusCodes.Status415UnsupportedMediaType;
return (null, false);
}

@balazsmeszegeto
Copy link
Contributor Author

Actually, now I'm wondering why this isn't working:

if (!httpContext.Request.HasFormContentType)
{
Log.UnexpectedNonFormContentType(httpContext, httpContext.Request.ContentType, throwOnBadRequest);
httpContext.Response.StatusCode = StatusCodes.Status415UnsupportedMediaType;
return (null, false);
}

I don't think that's being used here, since it'd also accept Content-Type: application/x-www-form-urlencoded. But if I try

curl --request POST 'http://localhost:5000/upload' --header 'Content-Type: application/x-www-form-urlencoded'

so I get 415, as expected.

balazsmeszegeto added a commit to balazsmeszegeto/aspnetcore that referenced this issue Jan 28, 2024
@balazsmeszegeto balazsmeszegeto changed the title Minimal API with IFormFile throws when Content-Type header is missing or empty Minimal API with IFormFile throws when request body is missing and Content-Type header is missing or empty Jan 28, 2024
@balazsmeszegeto
Copy link
Contributor Author

balazsmeszegeto commented Jan 28, 2024

Update: the exception only happens if both of these are true:

  • no request body
  • Content-Type header is missing or empty

I've updated the issue's title.

The bug is in TryReadFormAsync, which would return successful if IHttpRequestBodyDetectionFeature.CanHaveBody is false. I've added a PR fixing it

if (feature?.CanHaveBody == true)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc
Projects
None yet
2 participants