* native-tls: fix use of non-fmt panic in tests
* fix some misc. clippy lints
This branch fixes a number of lints. The most important one was the use
of a non-`format_args!` expression in a `panic!` macro, which generates
a compiler warning in recent Rust toolchains, which is breaking the CI
`cargo check` run on PR #64.
While I was here, I also fixed some miscellaneous Clippy lints, mostly
in tests. These include:
* Use of `clone()` on `SocketAddr`s (which implement `Copy`)
* Unnecessary single-path-segment imports (which probably used to be
`extern crate`s in earlier Rust?)
* `'static` lifetimes in `const` type annotations (`const`s always have
the `'static` lifetime)
None of these were breaking the build on CI, but I figured I'd address
them while I was fixing other lints.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
After this commit, this crate will support using TLS streams in a
half-closed state. Note that the TLS 1.3 spec in RFC 8446
says this should be supported:
```
Each party MUST send a "close_notify" alert before closing its write
side of the connection, unless it has already sent some error alert.
This does not have any effect on its read side of the connection. Note
that this is a change from versions of TLS prior to TLS 1.3 in which
implementations were required to react to a "close_notify" by discarding
pending writes and sending an immediate "close_notify" alert of their
own. That previous requirement could cause truncation in the read side.
Both parties need not wait to receive a "close_notify" alert before
closing their read side of the connection, though doing so would
introduce the possibility of truncation.
```
https://tools.ietf.org/html/rfc8446#page-87
The `rustls` crate raises such a clean closure of a
[`ClientSession`](https://docs.rs/rustls/0.18.0/rustls/struct.ClientSession.html#impl-Read)
or
[`ServerSesson`](https://docs.rs/rustls/0.18.0/rustls/struct.ServerSession.html#impl-Read)
read-side with `ErrorKind::ConnectionAborted`.
This crate's `TlsState` struct already encodes support for the
half-closed states `TlsState::ReadShutdown` and
`TlsState::WriteShutdown`, in addition to `TlsState::FullyShutdown`.
However, the current behavior of the `AsyncRead` implementation is that
it unconditionally shuts-down the write-half of a connection after the
read-half closes cleanly with `ErrorKind::ConnectionAborted`.
This change removes the `stream.session.send_close_notify()` and
`this.state.shutdown_write()` calls from `poll_read()`. Note that
`stream.session.send_close_notify()` is still called in
`poll_shutdown()`, which the application calls to cleanly shutdown the
write-half.
I highly suspect the logic of this can be simplified and cleaned up
further. Minimally, the edited match statement now has two identical
branches which could be combined into one. Additionally, perhaps the
`Stream` implementation should simply return `Ok(0)` for this case in
its implementation of
[`tokio::io::AsyncRead`](https://docs.rs/tokio/0.2/tokio/io/trait.AsyncRead.html),
since that's the defined way to indicate clean closure with EOF from
`AsyncRead`. However, I want to make the minimal changes and have them
reviewed for logical correctness first.
Co-authored-by: Braden Ehrat <braden@cloudflare.com>
The `wants_read` only changes after `process_new_packets`,
which means that not immediately calling `process_new_packets` may cause rustls to cache too much data.
