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

Thread-safety #78

Open
eliank opened this issue Jun 29, 2023 · 0 comments
Open

Thread-safety #78

eliank opened this issue Jun 29, 2023 · 0 comments

Comments

@eliank
Copy link

eliank commented Jun 29, 2023

I was looking into expanding our API logging when I stumbled upon this Gem. When exploring its inner workings I found an issue with your middleware, it is not thread-safe because instance variables are used and passed whilst the middleware is used in a concurrent manner. See:
https://github.com/aserafin/grape_logging/blob/6e562a278864a382bf5cf37cda7bad8924f0ad29/lib/grape_logging/middleware/request_logger.rb#L60C34-L60C38

The @env can be overwritten during the call! causing the request and responds to mismatch, effectively sending a response that was meant for another client.

You could add this disclaimer to the readme, patch it or yank the gem altogether as it seems to no longer be maintained. Since I am not a user I won't be submitting a PR to fix it, but thank you for creating the gem regardless, its a nice insight to see how you solved it.

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

No branches or pull requests

1 participant