Skip to content

Conversation

tessig
Copy link
Contributor

@tessig tessig commented Dec 12, 2024

Description

in combination with --auto-create-sessions options, the expiry check can not be performed since we get nil for the session on the initial incoming webhook

Checklist

  • [ x ] My code adheres to the style guidelines of this project
  • [ x ] I have conducted a self-review of my code
  • [ x ] I have added comments to my code, especially in areas that may be difficult to understand

in combination with `--auto-create-sessions` options, the expiry check can not be performed since we get nil for the session on the initial incoming webhook
@tessig tessig requested a review from tarampampam as a code owner December 12, 2024 09:55
@tarampampam
Copy link
Owner

Thank you for your PR! Could you clarify a bit - does this address a specific bug? If so, could you describe how to reproduce it? I'm a bit confused because if we're already checking for existence in the map using a boolean return value, why isn't that sufficient in this case?

@tessig
Copy link
Contributor Author

tessig commented Dec 13, 2024

Thanks for having a look.

With the --auto-create-sessions feature enabled, I would expect that I could post to an arbitrary session ID and it would be created.

For example: run curl -v -X POST --data '{"foo": "bar"}' http://localhost:8080/d9833717-484d-416c-beb0-1248325186fb on a fresh, empty instance.

When doing this, I get a nil-pointer dereference because the session load returns nil, false in that case.

@tarampampam tarampampam self-assigned this Dec 13, 2024
@tarampampam tarampampam added the type:fix 🛠 Fix label Dec 13, 2024
@tarampampam tarampampam merged commit cc8eae2 into tarampampam:master Dec 13, 2024
19 checks passed
@tarampampam
Copy link
Owner

@tessig Thank you, mate! You've done a great job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:fix 🛠 Fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants