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

Remove heartbeats completely #1928

Closed

Conversation

levitte
Copy link
Member

@levitte levitte commented Nov 15, 2016

Checklist
  • CLA is signed
Description of change

@levitte levitte added the 1.2.0 label Nov 15, 2016
@levitte levitte mentioned this pull request Nov 16, 2016
@mattcaswell mattcaswell added this to the 1.2.0 milestone Jan 24, 2018
@richsalz richsalz modified the milestones: 1.2.0, Post 1.1.0 May 6, 2018
@richsalz richsalz modified the milestones: Post 1.1.0, Assessed May 6, 2018
@levitte levitte force-pushed the remove-heartbreaks-completely-120 branch from a2448fd to 58d71c7 Compare September 30, 2018 18:06
@levitte levitte force-pushed the remove-heartbreaks-completely-120 branch from 58d71c7 to 1fbc4df Compare March 29, 2019 11:15
@levitte levitte changed the title Remove heartbeats completely (future 1.2.0) Remove heartbeats completely Mar 29, 2019
@levitte
Copy link
Member Author

levitte commented Mar 29, 2019

Re-awakening an old PR

@levitte levitte added branch: master Merge to master branch and removed after 1.1.1 labels Mar 29, 2019
@levitte levitte modified the milestones: Assessed, 3.0.0 Mar 29, 2019
@richsalz
Copy link
Contributor

This should probably have a CHANGES or NEWS entry.

@levitte levitte force-pushed the remove-heartbreaks-completely-120 branch from 44f69fc to 96c08b9 Compare March 29, 2019 12:02
@levitte
Copy link
Member Author

levitte commented Mar 29, 2019

Entries added. Also, it turns out that this fixes #4856, so I added a reference to that issue as well in the squash commit

@levitte levitte requested a review from t-j-h March 29, 2019 12:04
@levitte
Copy link
Member Author

levitte commented Mar 29, 2019

Merged.

558ea84 Remove heartbeats completely

@levitte levitte closed this Mar 29, 2019
levitte added a commit that referenced this pull request Mar 29, 2019
Fixes #4856

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from #1928)
@mattcaswell
Copy link
Member

Why wasn't the configure option moved to the list of deprecated Configure options. Now if someone explicitly compiles with "no-heartbeat" they will get an error rather than it being ignored - which seems to introduce unnecessary breakage.

@@ -468,7 +468,6 @@ static const ssl_trace_tbl ssl_exts_tbl[] = {
{TLSEXT_TYPE_srp, "srp"},
{TLSEXT_TYPE_signature_algorithms, "signature_algorithms"},
{TLSEXT_TYPE_use_srtp, "use_srtp"},
{TLSEXT_TYPE_heartbeat, "tls_heartbeat"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Was it really necessary to remove this trace code ?

Copy link
Member Author

Choose a reason for hiding this comment

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

If there's no heartbeat code, what good does that trace do you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants