It's doing whatever the underlying io.Reader does.

If the underlying reader returns (non-zero, io.EOF), then it sends one 1 DATA frame.

(We tried to change the strings.Reader and bytes.Reader implementations several times in the past to do eager EOF returns but it broke too many tests to be worth it.)

But I suppose we could know better in the case where the request body's content-length was predeclared (as it is when you use http.NewRequest on a few known types like strings.Reader). Once we've read enough bytes then we could assume that no bytes will remain on that reader, but that's also where we currently double-check users' Reader implementations and abort the http2 stream if their declared request content-length doesn't match reality. But I suppose we could trust standard library types and only sanity check unknown types.

But that's a lot of special casing for minimal benefit.

The spec (https://http2.github.io/http2-spec/#malformed) says:

A request or response is also malformed if the value of a content-length header field does not equal the sum of the DATA frame payload lengths that form the body.
Malformed requests or responses that are detected MUST be treated as a stream error (Section 5.4.2) of type PROTOCOL_ERROR.

So we can do the following:

• If request does not have content-length header, do not change the behavior.
• Read from io.Reader until we get enough bytes according to content-length, then try to read one more byte.
• If we get EOF without the byte, we are good.
• If we get the byte with or without EOF, report PROTOCOL_ERROR.

Another way is that we can wrap io.Reader into a new io.Reader with content-length constraint. This can be done as a separate package outside x/net, which will have minimal side-effects.

@bradfitz Which way looks good to you? I can do the implementation.

I see that io.LimitReader will do the trick.

I reopened the issue because io.LimitedReader does not do the trick, as it also returns EOF lazily.
I propose implementing a modified version of LimitedReader in http2/transport.go so that it returns eager EOF, as long as content-length is specified.

But then how/when do we report an error if the user's Request.Body Reader returns more bytes than the declared ContentLength?

@bradfitz A simplified solution would be to drop the extra bytes silently without producing any actual error except a debug log.

A complete solution would be the following:

1. Implement a limitedReader with eager EOF that can also produce a special ErrUnexpectedData when it meets extra bytes.
2. In ClientConn.roundTrip(): wrap body with limitedReader, if contentLen>=0
3. In clientStream.writeRequestBody(): after body.Read, if err==ErrUnexpectedData, write a stream reset frame with PROTOCOL_ERROR.

Does these sounds OK?

That doesn't sound okay to me. I'm not even sure how (1) would work, and even if it did, it'd be silently sending truncated data to the peer, rather than aborting the stream which is meant to prevent them fro working with corrupted data.

Fair enough.
Then we can go for (2). What's your concern? Is it the idea of addressing some fundamental issues (i.e. lazy EOF) in an upper level less elegant? 😊

Change https://golang.org/cl/181157 mentions this issue: http2: end stream eagerly after sending the request body

@bradfitz Hi Brad, I thought it may be easier to just talk about the code, so I filed https://golang.org/cl/181157

What I did is also simpler than discussed before. It fixes the issue, while still allows sending any body without interruption or truncation, even if the actual length does not match specified content-length. This is exactly the behavior before.

I thought it is the receiving peer's responsibility to handle the malformed request body rather than the sending peer.

@dmitshur Since Brad is on leave, can you please review https://golang.org/cl/181157 ? Thanks!

ping

Change https://golang.org/cl/200102 mentions this issue: net/http: update bundled x/net/http2