-
-
Notifications
You must be signed in to change notification settings - Fork 367
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
fix: remove redundant map access #775
Conversation
733ad37
to
f024721
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that this will panic if the key does not exist
I don't think this is right. See https://go.dev/ref/spec#Index_expressions
Here is a Go Playground example https://go.dev/play/p/udUlbIUWIfq |
Yeah this will not panic and will return exactly the same output. I would imagine there is a test in this scenario or one could be easily added to prove unchanged function. |
BTW @aeneasr if the session itself is nil then a call to Anyway, I've added a test to show this does not panic. |
Thank you for the follow up - I think I was confused because it would panic if we had a pointer and would try to access it. Since we don't have either, this is fine! :) |
Remove a redundant check for whether an element is present in a map. Since we return the zero value of a
time.Time
anyway, it's simpler to use the single return value version to access the map.Checklist
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact security@ory.sh) from the maintainers to push the changes.
I have not added any documentation or tests for this since it is such a small fix.