'Content-Length' header not always returned when enumerating HttpContentHeaders

[mellinoe]

Duplicated from internal bug 122111. Creating this tracking issue here so that we can trace these through the markers in the code back to the original internal bug tracker discussions if necessary.

[karelz]

Next step: We need a repro (@davidsh you said you might have it - it is simple ~6 lines), then debug it and fix it.
Warning: It is complicated.

[mellinoe]

I have no context on this issue; I just made a tracking issue because we had references to an old bug tracker in the code, and I was worried we would forget about it entirely. This might not even be a problem anymore.

[davidsh]

This repros in .NET Framework (Desktop) and .NET Core. It is an old bug from .NET Framework and has been there since we first shipped HttpClient in .NET 4.5. Over time, this bug has become more annoying and impactful to developers.

[Fact]
public void ContentLength_CreateStringContent_HeaderPresentInToString()
{
    string data = "This is a test";
    var content = new StringContent(data);
    string headers = content.Headers.ToString();
    Console.WriteLine(headers);
    Assert.IsTrue(headers.Contains("Content-Length"));
}

[Fact]
public void ContentLength_CreateStringContentAndAccessPropertyHeader_HeaderPresentInToString()
{
    string data = "This is a test";
    var content = new StringContent(data);
    
    // Calling the 'getter' for the ContentLength property will reconcile the strings cache.
    Console.WriteLine("content.Headers.ContentLength={0}", content.Headers.ContentLength);
    string headers = content.Headers.ToString();

    Console.WriteLine(headers);

    Assert.IsTrue(headers.Contains("Content-Length"));
}

Test [2] always passes. This is good.
Test [1] should pass but fails because of this lazy initialization bug with ContentLength property.

[satano]

@karelz Hello. Can I take this?

[satano]

Info about how to fix this from @Tratcher during this PR.

A) There are some common HttpContent implementations that know their length up front such as ByteArrayContent, StringContent, FormUrlEncodedContent, etc.. These types could eagerly set ContentLength in their constructors rather than waiting for TryComputeLength to be called. This would fix header enumeration and ToString for those types. This wouldn't help dynamic content types like StreamContent, but it's not clear to me that it's important for ToString to include a lazily calculated header.

B) It's the transport's job to make the final determination on if the request should use Content-Length or Chunked. It performs that rationalization before serializing the request to the wire.
Proposal: Change TryComputeLength to a public API, call it directly from the transports, and set ContentLength manually before sending the request.

These changes could be taken independently or together. After these changes you may be able to remove the implicit ContentLength behavior.

[satano]

I am going to do the first part - option A.

[pawepaw]

any update on that? i'm still facing that issue. Without calling stringContent.Headers.ContentLength; i'm not getting content-length header.

[karelz]

Triage: Option A is worth doing - it should be simple.
Then close the issue -- skip part B until we have customers complaining, note: part B may be breaking for streaming scenarios, etc. We should document why it is the way it is then.

@satano looks like you're not working on it, so unassigning.