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

conn_is_closed should return 1 if get_last_sys_error is WSAECONNRESET #8590

Closed
wants to merge 1 commit into from

Conversation

paulmon
Copy link
Contributor

@paulmon paulmon commented Mar 26, 2019

CLA: trivial

conn_is_closed should check for WSAECONNRESET on Windows because it has a different value than ECONNRESET. Without this check ssl tests fail in the Python standard tests because the SSL_do_handshake() on the server side returns an error if the client closes the socket too quickly.

Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

I agree this is trivial

@mattcaswell
Copy link
Member

@paulmon please can you amend your commit message to include the "CLA: trivial" text in the commit description (to keep the CLA bot happy).

@mattcaswell mattcaswell added the approval: review pending This pull request needs review by a committer label Mar 27, 2019
@paulmon paulmon changed the title conn_is_closed should return 1 if get_last_sys_error is WSAECONNRESET CLA: trivial - conn_is_closed should return 1 if get_last_sys_error is WSAECONNRESET Mar 27, 2019
@paulmon paulmon changed the title CLA: trivial - conn_is_closed should return 1 if get_last_sys_error is WSAECONNRESET conn_is_closed should return 1 if get_last_sys_error is WSAECONNRESET Mar 27, 2019
@paulmon
Copy link
Contributor Author

paulmon commented Mar 27, 2019

I tried editing the commit message with git --amend, but the buildbot still doesn't like it.
I also put the CLA: trivial on a seperate line in the commit message like it says in CONTRIBUTING.
Is that what you were suggesting? Do I need to close this PR and create a new one or should that have worked if I did it right?

@mattcaswell
Copy link
Member

Close/reopen to kick CLA bot

@mattcaswell mattcaswell reopened this Mar 27, 2019
@mattcaswell mattcaswell removed the hold: cla required The contributor needs to submit a license agreement label Mar 27, 2019
@mattcaswell
Copy link
Member

Not sure why the bot doesn't like it. It looks ok to me. I removed it by hand for now.

Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

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

Agree that this is trivial.

@paulidale paulidale added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Mar 28, 2019
levitte pushed a commit that referenced this pull request Mar 28, 2019
CLA: trivial

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #8590)
levitte pushed a commit that referenced this pull request Mar 28, 2019
CLA: trivial

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #8590)

(cherry picked from commit 0b885f7)
@mattcaswell
Copy link
Member

Pushed. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants