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

improper use of singleflight #130

Closed
nvx opened this issue Aug 20, 2021 · 2 comments · Fixed by #131
Closed

improper use of singleflight #130

nvx opened this issue Aug 20, 2021 · 2 comments · Fixed by #131
Assignees
Labels
bug Something isn't working

Comments

@nvx
Copy link
Contributor

nvx commented Aug 20, 2021

singleflight is used to deduplicate requests to issue a new certificate

ch := c.issueGroup.DoChan("issue", func() (interface{}, error) {

Unfortunately the key used is a static string "issue" which if simultaneous requests come in for different common names that are not cached the wrong result may be returned.

Changing the key from the fixed string to the same key used for the cache (name in this context) would prevent the issue from occurring.

I haven't actually run into this issue myself (my usage of certify only ever has the one common name being issued) but stumbled across it while looking at the code for unrelated reasons so I thought I'd open a bug report rather than a trivial PR just in case I've misunderstood the logic being used.

@johanbrandhorst
Copy link
Owner

I think you're entirely right, good spot! I've opened #131 to fix it.

@nvx
Copy link
Contributor Author

nvx commented Aug 21, 2021

That looks like it'd do the trick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants