Skip to content

Conversation

@maskit
Copy link
Member

@maskit maskit commented Jun 8, 2020

A possible fix for the crash on TLS resumption.

The crash seems to be caused by a small change made on ##6736. On the PR, I added this->_setSSLCurveNID(SSLGetCurveNID(ssl)); after a call for _setSSLSessionCacheHit for consistency, but I realized it doesn't make sense in _getSessionInformation.

So I removed the line and added a code similar to fetch_ssl_curve, which was removed on #6736.

I couldn't reproduce the crash by myself and CI tests don't catch the crash somehow, so please ran this locally if you can reproduce the crash.

@maskit maskit added the TLS label Jun 8, 2020
@maskit maskit added this to the 10.0.0 milestone Jun 8, 2020
@maskit maskit requested review from a-canary and shinrich June 8, 2020 05:48
@maskit maskit self-assigned this Jun 8, 2020
@shinrich
Copy link
Member

shinrich commented Jun 8, 2020

I'll give this change a try.

@shinrich
Copy link
Member

shinrich commented Jun 8, 2020

Verified it addressed the issue. Will kick autest again in the hopes that the compress failure was transient. If not will try to get that test running locally.

@shinrich
Copy link
Member

shinrich commented Jun 8, 2020

[approve ci autest]

2 similar comments
@shinrich
Copy link
Member

shinrich commented Jun 8, 2020

[approve ci autest]

@shinrich
Copy link
Member

shinrich commented Jun 8, 2020

[approve ci autest]

@maskit maskit merged commit 02a60b2 into apache:master Jun 9, 2020
@zwoop
Copy link
Contributor

zwoop commented Jun 9, 2020

Cherry-picked to v9.0.x branch.

I assume we don't need this for 8.1.x ?

@zwoop zwoop modified the milestones: 10.0.0, 9.0.0 Jun 9, 2020
@maskit
Copy link
Member Author

maskit commented Jun 10, 2020

You're right. We don't need this for 8.1.x as long as we don't backport #6736.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants