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

What does SecureContext::EnableTicketKeyCallback do? #13176

Closed
addaleax opened this issue May 23, 2017 · 1 comment
Closed

What does SecureContext::EnableTicketKeyCallback do? #13176

addaleax opened this issue May 23, 2017 · 1 comment
Labels
crypto Issues and PRs related to the crypto subsystem. question Issues that look for answers.

Comments

@addaleax
Copy link
Member

e83a41a introduced an internal enableTicketKeyCallback method on SecureContexts, without documentation or usage in JS land. It’s only mentioned in one of the test files, test/parallel/test-https-resume-after-renew.js. So … Should this functionality stay? Should it go? And generally, what is it?

I stumbled upon this while looking into #13142 (comment) to see whether SecureContext should inherit from AsyncWrap, but I can’t really tell how the MakeCallback there works and whether it should really be one, so I thought it might be good to just ask about it.

@indutny @nodejs/crypto

@addaleax addaleax added crypto Issues and PRs related to the crypto subsystem. question Issues that look for answers. labels May 23, 2017
@indutny
Copy link
Member

indutny commented May 23, 2017

It is needed only for testing as it is now. There is just no other way to write a regression test for that problem.

If someone will ever request an API for this, we should make it public. Otherwise, let's keep it for private use.

addaleax added a commit to addaleax/node that referenced this issue May 24, 2017
jasnell pushed a commit that referenced this issue May 28, 2017
Fixes: #13176
PR-URL: #13193
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 17, 2017
Fixes: #13176
PR-URL: #13193
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. question Issues that look for answers.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants