JsonMediaTypeFormatter doesn't work when Request is chunked

[felipepessoto]

When the request transfer-encoding is chunked, the request.ContentLength is 0 instead of null, then JsonMediaTypeFormatter return the default value (null for classes) in this line:

AspNetWebStack/src/System.Net.Http.Formatting/Formatting/BaseJsonMediaTypeFormatter.cs

Line 209 in 1a987f8

return GetDefaultValueForType(type);

The workaround is: https://gist.github.com/jayoungers/0b39b66c49bf974ba73d83943c4b218b but it is just a workaround, and we don't know if it breaks other scenarios.

[dougbu]

I seem to remember an issue with chunking that may have been fixed recently. Which ASP.NET version are you using? And, which host e.g. IIS are you testing with?

[felipepessoto]

I tried with the latest version, 5.2.6

[dougbu]

Which host? A repro project, preferrably hosted in a GitHub repo, may help.

[felipepessoto]

@dougbu , the repro is here: https://github.com/felipepessoto/AspNetWebStack_197
It is just a clean project where I changed the ValuesController to receive a complex object and updated the Nuget Packages.

The workaround is in Global.asax.

To reproduce you can use the ChunkedRequest.raz file with Fiddler, or create a request like this:

Headers:

User-Agent: Fiddler
Host: localhost:52841
Content-Type: application/json
Transfer-Encoding: chunked

Body:

D
{Name: "Joe"}
0

Be aware of the two line breaks in the end of body.

Thanks

[dougbu]

This issue looks like a holdover from HTTP 1.0. The code in this repo special cases Content-Length when a Transfer-Encoding is specified. But, that's not done everywhere and the formatters are all broken in this respect.

The relevant HTTP 1.1 text is in https://tools.ietf.org/html/rfc2616#section-4.3:

Messages MUST NOT include both a Content-Length header field and a non-identity transfer-coding. If the message does include a non-identity transfer-coding, the Content-Length MUST be ignored.

So, a chunked message containing Content-Length is invalid but our formatters are required to accept it.

@danroth27 @mkArtakMSFT this looks like a straight-up bug in our formatters (again, not just JsonMediaTypeFormatter). Should we fix it in 3.2.7?

---

PS. Where our code handles messages with both Content-Length and Transfer-Encoding headers, the handling is specific to Transfer-Encoding: chunked. @Tratcher is, say, a Transfer-Encoding: gzip header handled (if supported) and removed before it reaches ASP.NET e.g. by Owin, IIS or IIS Express? (Nothing in ASP.NET itself handles gzip, compress or deflate transfer encodings.)

[Tratcher]

No, there's no support for Transfer-Encoding: gzip. (it would be Content-Encoding: gzip anways).

Content-Length vs Chunked should be a non-issue for high level components like formatters. Only the server should need to worry about the distinction.

[dougbu]

Only the server should need to worry about the distinction.

What about the case where the formatters are used in a client application?

Separately, I looked again and the Content-Length special case is specific to when ASP.NET hosts are creating responses. They should also special-case requests containing both headers.

[Tratcher]

On the client side as well, it's not the formatters job to validate content-length vs chunked. At most when they're generating a body they could avoid setting both.

[dougbu]

@Tratcher looks like something at a lower level than our web host adds Content-Length: 0 to the content headers whenever the request is chunked. Where is that likely happening? (I can't find any code in this repo that sets Request.Content.Headers.ContentLength.)

This happens whether or not the request has a non-empty body. Tested using Fiddler, with the following request bodies:

D
{Name: "Joe"}
0

and

0

[dougbu]

@felipepessoto the workaround described is incorrect because it unconditionally resets a 0 ContentLength value to null. If done at all, this should only be done if the request is chunked.

[felipepessoto]

Yeah, I implemented it using a different approach in my real project, deriving from JsonMediaTypeFormatter and looking the headers.

This gist is what many people are using though, because it is what you found in stackoverflow when search for this problem

That's why I think it is important to fix it instead of workaround it.

PS: if I remember well, the same IF exists in XML formatter as well

[mkArtakMSFT]

Thanks for the investigation, @dougbu!
As this seems to be caused by an external (lower-level) aspect, please use this issue to track the workaround.

[dougbu]

@Tratcher I'm leaving the investigate label here only for the option of filing a bug on the underlying component that's setting the incorrect ContentLength value. See comments above.

[Tratcher]

Let's look at the repro together next week. We should be able to narrow down the layering quickly.

[dougbu]

Removing investigate label. Root cause here is that SeekableBufferedRequestStream.Length value may be incorrect while its inner Stream's CanSeek == false. Fixing that should fix the problem.

Alternate fix of changing the HtmlContext classes in web host to never guess ContentLength based on the body Stream could perhaps be more efficient. But, that would be a messier, larger change.

[dougbu]

523f3aa

[dougbu]

Note we went with the "alternate fix" I mentioned earlier, avoiding sync I/O

[felipepessoto]

The next release will include this fix? Do you already have an approximate release date?

Thanks

[dougbu]

The next release will include this fix. @danroth27 have we announced a specific date for 3.2.7?