Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 prevent hang when HelloRetryRequest is returned #4036

Merged
merged 1 commit into from
May 22, 2024
Merged

Conversation

jaym
Copy link
Contributor

@jaym jaym commented May 20, 2024

It was possible for the following query to hang

tls.ciphers

This happened in cases where the server response with a HelloRetryRequest. This looks like a ServerHello until you check the random field of that message.

We ended up hanging because we thought the server replied successfully with a hello and were waiting for more data, and the server was waiting for us to write.

Copy link
Contributor

github-actions bot commented May 20, 2024

Test Results

2 996 tests  ±0   2 995 ✅ ±0   1m 28s ⏱️ +5s
  329 suites ±0       1 💤 ±0 
   23 files   ±0       0 ❌ ±0 

Results for commit 5fd3fce. ± Comparison against base commit d976b36.

♻️ This comment has been updated with latest results.

It was possible for the following query to hang
```
tls.ciphers
```

This happened in cases where the server response with a
`HelloRetryRequest`. This looks like a `ServerHello` until you check the
random field of that message.

We ended up hanging because we thought the server replied successfully
with a hello and were waiting for more data, and the server was waiting
for us to write.
Copy link
Contributor

@czunker czunker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice! Thanks @jaym

@czunker czunker merged commit 860eaeb into main May 22, 2024
15 checks passed
@czunker czunker deleted the jdm/helloretry branch May 22, 2024 13:13
@github-actions github-actions bot locked and limited conversation to collaborators May 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants