[scotchmist]

Add SslStream::first_read_at, which returns when bytes were first read from the underlying stream. Servers can use it to time the TLS handshake from the start of the ClientHello rather than from TCP accept, so time spent waiting for the peer is not counted.

[clanky-the-clanker]

This pull request's reviews are posted as pull request reviews: see the review of dfd53534.

[fisherdarling]

@clanky-the-clanker review

[clanky-the-clanker]

Superseded by the review of ca2f16ad.

This review, of dfd53534

💬 Approved with comments — The implementation and the test are correct. The only comment is an optional clarification to the doc comment.

Findings: 1 info (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 1 info

The doc comment slightly overstates what the timestamp measures

Recommendation: 💬 Optional Nits

• [INFO] The doc comment says the timestamp is when the hello 'started arriving'. It is actually when the first read returned bytes. (tokio-boring/src/lib.rs:164)

View full review

• tokio-boring/src/lib.rs:164 describes the timestamp as when the peer's hello "started arriving". The code records Instant::now() when the first read returns bytes, which can be later than when the bytes actually arrived.

---

✅ All other checks passed

📋 Code Quality: ✅ LGTM

• first_read is set only when a read actually returns bytes. WouldBlock, errors and EOF don't set it.
• The bridge is the same object during the handshake and on the established stream, so the timestamp carries over once the handshake completes.
• The test takes its start time before the server is polled, and the client sleeps only after it has connected, so the assertion that the first read came at least the delay after the start is sound.

---

_{Reviewed commit: dfd53534}

[TheJokr]

clanky review

[clanky-the-clanker]

⚠️ Out of date: this review is of ca2f16ad; the pull request is now at d7dfb5c9. Someone with write access can comment clanky review (or clanker review or @clanky-the-clanker review) to review the new commits.

💬 Approved with comments — The implementation, the switch to tokio's Instant, and the test are correct. The only comment is an optional doc clarification for streams built with from_raw_parts.

Findings: 1 info (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 1 info

One optional clarification for streams created with from_raw_parts.

Recommendation: 💬 Optional Nits

• [INFO] The ClientHello/ServerHello wording only holds when tokio-boring performs the handshake itself, not for streams created with from_raw_parts. (tokio-boring/src/lib.rs:164)

View full review

• tokio-boring/src/lib.rs:164-165 says the first read is the peer's ClientHello or ServerHello. from_raw_parts builds a new bridge, with first_read: None, around a handshake that may have finished elsewhere. For such a stream, first_read_at() reflects a later application-data read.

---

✅ All other checks passed

📋 Code Quality: ✅ LGTM

• tokio::time::Instant needs tokio's time feature, and tokio-boring/Cargo.toml now enables it.
• first_read is set only when a read returns more than 0 bytes. WouldBlock, errors and EOF don't set it.
• The same bridge object is used during the handshake and on the established stream, so the timestamp carries over to SslStream::first_read_at.
• The test takes its start time before the server is polled, and the client sleeps only after it has connected. So the assertion that the first read came at least DELAY after the start is sound.

---

_{Reviewed commit: ca2f16ad}