Skip to content

Controller [FromBody] argument is null if project references "System.Data.SqlClient 4.1.0" #1605

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
another-guy opened this issue Jun 28, 2016 · 12 comments

Comments

@another-guy
Copy link

If my web api project references NuGet package System.Data.SqlClient 4.1.0 then a controller defined in the same project can't retrieve an argument passed in request body. It took me a while to find a diff (see below) that shows what code change introduces the bug in my project.

Here's a controller code I have

public class SqlTaskPersistence : ITaskPersistence
{
    public Task Save(OneLineTask oneLineTask) // <- This oneLineTask argument is always null :(
    {

Of course I have some service Startup tweaks (e.g. Dependency Injection configuration). I can provide this code too if needed. Is there a way for me to verify there are no errors/exceptions while service is being started? At the moment I test it only on a dev machine.

Could you give me a hint about what I am doing wrong?

bug

@rynowak
Copy link
Member

rynowak commented Jun 28, 2016

Can you please include your controller code (ok to omit the method bodies) and some information about what the request looks like? If this is in a public repo, just give us a pointer and we'll take a look.

@another-guy
Copy link
Author

@rynowak Here you go sir. The following commit fixes the [FromBody] argument = null issue:

https://github.com/another-guy/TheNextOne/commit/684ac93e75a4de7b7580835b61c252bdf1b0bb28

To test the API locally you can execute the following request in Fiddler/Postman:

Verb + url: POST http://localhost:5001/tasks
Headers: Content-Type:application/json
Body:

{
"id": "some id",
"description": "some description",
"created": "2014-01-01T02:00:00.123Z",
"status": "some status"
}

@rynowak
Copy link
Member

rynowak commented Jun 28, 2016

Thanks, I gave this a try and was able to see what you were seeing.

The issue here is that there was a breaking change between the 1.0.0-RC2 and 1.0.0 release to ValueTask<T> that impacted Kestrel. Referencing "System.Data.SqlClient": "4.1.0" pulls a framework package that isn't compatible with RC2 Kestrel and causes the break.

To fix this you can either upgrade your ASP.NET binaries to 1.0.0 or use "System.Data.SqlClient": "4.1.0-rc2-24027" for the time being (this is the release of SqlClient that corresponds with our RC2.

As far as why this isn't failing in a better way, I'm going to open an issue for this on MVC. The error coming from Kestrel was a MethodMissingException, but because it was bubbling up for a formatter, it was getting turned into a model state error. We need to reevaluate this.

@another-guy
Copy link
Author

@rynowak Thanks! I made my project use the rc2 version of System.Data.SqlClient and it indeed worked well for me.

It is funny, I saw the error in ModelState while debugging. I was unable to relate this error to a null reference issue I observed. Thank you for opening #4920. I hope improving this code will allow other developers to see their projects following the Fail Fast principle.

@codeConcussion
Copy link

I'm running into something similar. I upgraded from RC2 to Release and a weird error appeared. I post json to a controller method and [FromBody] works fine the first time. But with subsequent calls (that are exactly identical) I get a null value for the [FromBody] argument.

It appears to be Kestrel related because it works fine running locally with IISExpress or on our test server under IIS. Ideas?

@rynowak rynowak reopened this Jul 7, 2016
@rynowak
Copy link
Member

rynowak commented Jul 7, 2016

@codeConcussion did you look in ModelState? Are there any exceptions in there? Try dumping the contents to a log or console.

@codeConcussion
Copy link

codeConcussion commented Jul 7, 2016

@rynowak Just looked at it. No errors in ModelState. None on the first call that works (obviously) and none on the second call that fails.

It could be a completely different issue or just some boneheaded mistake I'm making but it sure seems strange. I'll try to create a slimmed down project from scratch and see if I can recreate it.

@rynowak
Copy link
Member

rynowak commented Jul 7, 2016

If you can try to create a repro we'd be happy to take a look.

@codeConcussion
Copy link

@rynowak - While creating the repro project I discovered the issue. It's unrelated to this issue even though the end result is the same.

We wrote middleware that is reading the HttpContext.Request.Body stream. The stream is a Microsoft.AspNetCore.Server.Kestrel.Internal.Http.FrameRequestStream which you can't reset (context.Request.Body.Position = 0 or context.Request.Body.Seek(0, SeekOrigin.Begin)) so instead we were just setting the body to a new MemoryStream which seemed to work. Now it causes issues - works the first time but not after.

I also tried doing context.Request.Body.CopyTo(newMemoryStream) and then reading the stream I copied it to instead of directly reading the request body, but that also causes the same issue.

Any suggestions on how to correctly read the request body's stream?

@davidfowl
Copy link
Member

See aspnet/KestrelHttpServer#940. You can work around it by resetting the stream back to the original after the response is written.

@rynowak
Copy link
Member

rynowak commented Jul 8, 2016

@codeConcussion you can see an example here, https://github.com/aspnet/HttpAbstractions/blob/3e69df87f89601a5ede32c6765b736ab922b8fee/src/Microsoft.AspNetCore.Http/Internal/BufferingHelper.cs

but as @davidfowl pointed out, you will currently have a bad time if you don't reset the stream to the original stream when unwinding.

@Tratcher did we ship the buffering middleware?

@Tratcher
Copy link
Member

Tratcher commented Jul 8, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants