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.

Sign up for GitHub

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

Jump to bottom

Handle TCP reset during handshake #251

Merged
merged 1 commit into from
Oct 18, 2017
Merged

Handle TCP reset during handshake #251

merged 1 commit into from
Oct 18, 2017

Conversation

ocheron
Copy link
Contributor

@ocheron ocheron commented Oct 11, 2017

Here catching only IOException looks preferable as the goal is simply to ignore.

@ocheron
Handle TCP reset during handshake
a92b413 
Sending the TLS alert to the peer may fail and should not replace and
hide the original exception we're trying to wrap.

Fixes #250.
@kazu-yamamoto
Copy link
Collaborator

Is this PR trying to fix #250?

@ocheron
Copy link
Contributor Author

ocheron commented Oct 12, 2017

Yes, this allows the HandshakeFailed exception to be returned instead of a second exception coming from sendPacket.

@kazu-yamamoto
Copy link
Collaborator

@ocheron Thank you.
@boris-stepanov Would you try this to fix your problem?

@boris-stepanov
Copy link

boris-stepanov commented Oct 16, 2017 •
edited
Loading

@kazu-yamamoto, @ocheron, well, it passed the first case with empty message, so I'm already satisfied :) I'm currently preparing more tests for my project and give you a report if will find something else. Thanks.

@ocheron
Copy link
Contributor Author

ocheron commented Oct 16, 2017

@boris-stepanov Thank you, I guess empty message means HandshakeFailed is now thrown and intercepted.

@kazu-yamamoto To put things in context: when working on previous issues I noticed sendPacket had its own catch in recvData.terminate and not in handshake exception handling. This looked suspicious but I didn't know how to generate a failure at that time.

@kazu-yamamoto
Copy link
Collaborator

@ocheron Would you like to merge this by yourself? Or shall I?

@kazu-yamamoto kazu-yamamoto self-requested a review October 18, 2017 06:23
kazu-yamamoto
kazu-yamamoto approved these changes Oct 18, 2017
View reviewed changes
Copy link
Collaborator

@kazu-yamamoto kazu-yamamoto left a comment

Choose a reason for hiding this comment

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

LGTM

@ocheron ocheron merged commit a92b413 into haskell-tls:master Oct 18, 2017
ocheron added a commit that referenced this pull request Oct 18, 2017
@ocheron
Merge pull request #251 from ocheron/tcp-reset
5870f27 
Handle TCP reset during handshake
ocheron added a commit that referenced this pull request Oct 18, 2017
@ocheron
Add #251 to CHANGELOG
7ea7a05
@ocheron ocheron deleted the tcp-reset branch October 18, 2017 19:08
@ocheron
Copy link
Contributor Author

ocheron commented Oct 18, 2017

Merged, thanks for reviewing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kazu-yamamoto kazu-yamamoto kazu-yamamoto approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ocheron @kazu-yamamoto @boris-stepanov