This simplifies the code that's used for wiring up the http.Handler-based
transport for testing. It's not used outside of tests.
http2.Server.ServeConn was added somewhat recently for other reasons and
I just realized it would simplify this code as well.
The http.Handler-based transport body reader was returning error types
not understood by the recvMsg parser. See #557 for some background and
examples.
Fix the http.Handler transport and add tests. I copied in a subset of
the http2 package's serverTest type, adapted slightly to work with
grpc. In the process of adding tests, I discovered that
ErrUnexpectedEOF was also not handled by the regular server
transport. Document the rules and fix that crash as well.
Unrelated stuff in this CL:
* make tests listen on localhost:0 instead of :0, to avoid Mac firewall
pop-up dialogs.
* rename parser.s field to parser.r, to be more idiomatic that it's an
io.Reader and not anything fancier. (it's not acting like type
stream, even if that's the typical concrete type)
* move 5 byte temp buffer into parser, rather than allocating it for
each new message. (drop in the bucket improvement in garbage; more
to do later)
* rename http2RSTErrConvTab to http2ErrConvTab, per Qi's earlier
CL. Also add the HTTP/1.1-required error mapping for completeness,
not that it should ever arise with gRPC, also per Qi's earlier CL
referenced in #557.
This removes serverSetUp and clientSetUp. They had too many positional
parameters and weren't readable at the call sites.
With this change, it's now much more obvious how a test differs from
others just be looking at what is tinkered with after newTest.
Change-Id: I59bb06f8029af166002033f2c3f7b8f0b2d20940
Filter expected log output by default, unless a flag is provided, or a
test fails.
This makes it possible to see unexpected things. Having noisy tests
makes it too easy to miss actual problems.
It wasn't closing the recvBuffer body in all cases during shutdown.
This change also:
* adds a new test with concurrent streams doing their own serial sends.
This test was part of earlier debugging, but exists now to add more
test coverage around concurrency.
* starts a cleanup of the end2end testing code, to be continued later.
but the cleanup was necessary when writing the new test to be clean
and not add more positional parameters.
* documents the concurrency expectations of the ServerTransport
interface, cleaning up some other nearby documentation in the
process.
* speeds up TestCancelNoIO to cancel some contexts once no longer
needed, adding some comments about what the test is doing, adds some
TODOs, and reduces some overly-long sleeps.
This adds new http.Handler-based ServerTransport in the process,
reusing the HTTP/2 server code in x/net/http2 or Go 1.6+.
All end2end tests pass with this new ServerTransport.
Fixesgrpc/grpc-go#75
Also:
Updates grpc/grpc-go#495 (lets user fix it with middleware in front)
Updates grpc/grpc-go#468 (x/net/http2 validates)
Updates grpc/grpc-go#147 (possible with x/net/http2)
Updates grpc/grpc-go#104 (x/net/http2 does this)
The grpc-http2 transport doesn't validate hpack-decoded field names to be
valid http2 field names before checking their first byte. Had it verified
first and found that the empty string is illegal, this crash wouldn't happen,
but currently a malicious request can crash a gRPC server by sending an empty
hpack string.
The go race detector has a limit on how many goroutines it can track.
This test is only barely acceptable and the presence of any new
goroutines push it over the edge. Shorten this test is race mode.
Brad Fitzpatrick
Qi Zhao
Paul Querna
Yang Gao
David Symonds
Tamir Duberstein
Eric Anderson