## Motivation
Currently, the `tracing` spans for the client and server handshakes
contain the name of the I/O type. In some cases, where nested I/O types
are in use, these names can be quite long; for example, in Linkerd, we
see log lines like this:
```
2022-03-07T23:38:15.322506670Z [ 10533.916262s] DEBUG ThreadId(01) inbound:accept{client.addr=192.168.1.9:1227}:server{port=4143}:direct:gateway{dst=server.echo.svc.cluster.local:8080}:server_handshake{io=hyper::common::io:⏪:Rewind<linkerd_io::either::EitherIo<linkerd_io::sensor::SensorIo<linkerd_io::prefixed::PrefixedIo<linkerd_io::either::EitherIo<tokio_rustls::server::TlsStream<linkerd_io::either::EitherIo<linkerd_io::scoped::ScopedIo<tokio::net::tcp::stream::TcpStream>, linkerd_io::prefixed::PrefixedIo<linkerd_io::scoped::ScopedIo<tokio::net::tcp::stream::TcpStream>>>>, linkerd_io::either::EitherIo<linkerd_io::scoped::ScopedIo<tokio::net::tcp::stream::TcpStream>, linkerd_io::prefixed::PrefixedIo<linkerd_io::scoped::ScopedIo<tokio::net::tcp::stream::TcpStream>>>>>, linkerd_transport_metrics::sensor::Sensor>, linkerd_io::sensor::SensorIo<linkerd_io::either::EitherIo<tokio_rustls::server::TlsStream<linkerd_io::either::EitherIo<linkerd_io::scoped::ScopedIo<tokio::net::tcp::stream::TcpStream>, linkerd_io::prefixed::PrefixedIo<linkerd_io::scoped::ScopedIo<tokio::net::tcp::stream::TcpStream>>>>, linkerd_io::either::EitherIo<linkerd_io::scoped::ScopedIo<tokio::net::tcp::stream::TcpStream>, linkerd_io::prefixed::PrefixedIo<linkerd_io::scoped::ScopedIo<tokio::net::tcp::stream::TcpStream>>>>, linkerd_transport_metrics::sensor::Sensor>>>}:FramedWrite::buffer{frame=Settings { flags: (0x0), initial_window_size: 65535, max_frame_size: 16384 }}: h2::codec::framed_write: send frame=Settings { flags: (0x0), initial_window_size: 65535, max_frame_size: 16384 }
```
which is kinda not great.
## Solution
This branch removes the IO type's type name from the spans for the
server and client handshakes. In practice, these are not particularly
useful, because a given server or client instance is parameterized over
the IO types and will only serve connections of that type.
h2::Error now knows whether protocol errors happened because the user
sent them, because it was received from the remote peer, or because
the library itself emitted an error because it detected a protocol
violation.
It also keeps track of whether it came from a RST_STREAM or GO_AWAY
frame, and in the case of the latter, it includes the additional
debug data if any.
Fixes#530
This commit adds accessors to `client::Connection` and
`server::Connection` that return the current value of the
`SETTINGS_MAX_CONCURRENT_STREAMS` limit that has been sent by this peer
and acknowledged by the remote.
This is analogous to the `max_concurrent_send_streams` methods added in
PR #513. These accessors may be somewhat less useful than the ones for
the values negotiated by the remote, since users who care about this
limit are probably setting the builder parameter. However, it seems
worth having for completeness sake --- and it might be useful for
determining whether or not a configured concurrency limit has been acked
yet...
Part of #512
* refactor: Extract FramedWrite::buffer to a less generic function
Should cut out another 23 KiB (since I see it duplicated)
* refactor: Extract some duplicated code to a function
* refactor: Extract part of flush into a less generic function
* refactor: Extract a less generic part of connection
* refactor: Factor out a less generic part of Connection::poll2
* refactor: Extract a non-generic part of handshake2
* refactor: Don't duplicate Streams code on Peer (-3.5%)
The `P: Peer` parameter is rarely used and there is already a mechanism
for using it dynamically.
* refactor: Make recv_frame less generic (-2.3%)
* Move out part of Connection::poll
* refactor: Extract parts of Connection
* refactor: Extract a non-generic part of reclaim_frame
* comments
This PR adds accessors to `client::Connection` and `server::Connection`
that return the send stream concurrency limit on that connection, as
negotiated by the remote peer. This is part of issue #512.
I think we probably ought to expose similar accessors for other
settings, but I thought it was better to add each one in a separate,
focused PR.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
We've adopted `tracing` for diagnostics, but currently, it is just being
used as a drop-in replacement for the `log` crate. Ideally, we would
want to start emitting more structured diagnostics, using `tracing`'s
`Span`s and structured key-value fields.
A lot of the logging in `h2` is already written in a style that imitates
the formatting of structured key-value logs, but as textual log
messages. Migrating the logs to structured `tracing` events therefore is
pretty easy to do. I've also started adding spans, mostly in the read
path.
Finally, I've updated the tests to use `tracing` rather than
`env_logger`. The tracing setup happens in a macro, so that a span for
each test with the test's name can be generated and entered. This will
make the test output easier to read if multiple tests are run
concurrently with `--nocapture`.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
The old `tokio-core` crate is deprecated in favour of the `tokio` crate,
which also provides the new multithreaded Tokio runtime. Although `h2`
is generic over the runtime, the examples do depend on `tokio`.
This branch update the example code to use the new `tokio` library
rather than `tokio-core`. It also updates the examples in `RustDoc`.
For the most part, this was pretty trivial --- simply replacing
`handle.spawn(...)` with `tokio::spawn(...)` and `core.run(...)` with
`tokio::run(...)`. There were a couple of cases where error and item
types from spawned futures had to be mapped to `(), ()`. Alternatively,
this could have been avoided by using the single-threaded runtime and
calling `block_on` instead to await the value of those futures, but I
thought it was better to reduce the amount of tokio-specific code in the
examples.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Problem:
Applications may want to access the underlying h2 stream ID for
diagnostics, etc. Stream IDs were not previously exposed in public APIs.
Solution:
Added a new public `share::StreamId` type, which has a more restricted
API than the internal `frame::StreamId` type. The public API types
`SendStream`, `RecvStream`, `ReleaseCapacity`,
`client::ResponseFuture`, and `server::SendResponse` now all have
`stream_id` methods which return the stream ID of the corresponding
stream.
Closes#289.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Because `self.pending` doesn't necessarily get cleaned up in a timely fashion -
rather, only when the user calls `poll_ready()` - it was possible for it to
refer to a stream that has already been closed. This would lead to a panic the
next time that `poll_ready()` was called.
Instead, use an `OpaqueStreamRef`, bumping the refcount.
A change to an existing test is included which demonstrates the issue.
There is currently no way to configure the initial target window size
for connections. The `Builder::initial_connection_window_size` utilities
make this configurable so that all new connections have this target
window size set.
When all Streams are dropped / finished, the Connection was held
open until the peer hangs up. Instead, the Connection should hang up
once it knows that nothing more will be sent.
To fix this, we notify the Connection when a stream is no longer
referenced. On the Connection poll(), we check that there are no
active, held, reset streams or any references to the Streams
and transition to sending a GOAWAY if that is case.
The specific behavior depends on if running as a client or server.
This, uh, grew into something far bigger than expected, but it turns out, all of it was needed to eventually support this correctly.
- Adds configuration to client and server to set [SETTINGS_MAX_HEADER_LIST_SIZE](http://httpwg.org/specs/rfc7540.html#SETTINGS_MAX_HEADER_LIST_SIZE)
- If not set, a "sane default" of 16 MB is used (taken from golang's http2)
- Decoding header blocks now happens as they are received, instead of buffering up possibly forever until the last continuation frame is parsed.
- As each field is decoded, it's undecoded size is added to the total. Whenever a header block goes over the maximum size, the `frame` will be marked as such.
- Whenever a header block is deemed over max limit, decoding will still continue, but new fields will not be appended to `HeaderMap`. This is also can save wasted hashing.
- To protect against enormous string literals, such that they span multiple continuation frames, a check is made that the combined encoded bytes is less than the max allowed size. While technically not exactly what the spec suggests (counting decoded size instead), this should hopefully only happen when someone is indeed malicious. If found, a `GOAWAY` of `COMPRESSION_ERROR` is sent, and the connection shut down.
- After an oversize header block frame is finished decoding, the streams state machine will notice it is oversize, and handle that.
- If the local peer is a server, a 431 response is sent, as suggested by the spec.
- A `REFUSED_STREAM` reset is sent, since we cannot actually give the stream to the user.
- In order to be able to send both the 431 headers frame, and a reset frame afterwards, the scheduled `Canceled` machinery was made more general to a `Scheduled(Reason)` state instead.
Closes#18Closes#191
This patch renames a number of types and functions making
the API more consistent.
* `Server` -> `Connection`
* `Client` -> `SendRequest`
* `Respond` -> `SendResponse`.
It also moves the handshake fns off of `Connection` and make
them free fns in the module. And `Connection::builder` is removed
in favor of `Builder::new`.
This patch adds checks for the request URI and rejects invalid URIs. In
the case of forwarding an HTTP 1.1 request with a path, an "http" pseudo
header is added to satisfy the HTTP/2.0 spec.
Closes#179
