search

tomMoulard / fail2ban

Traefik plugin on fail2ban middleware
MIT License
191 stars 10 forks source link

Moved block logic into its own method so that the lock is released be… #68

Closed AnderssonPeter closed 9 months ago

AnderssonPeter commented 1 year ago

Moved block logic into its own method so that the lock is released before sending the response content

Attempt at fixing #67, might also solve #59.

!!!!Note that I have no knowledge or experience of GO so the code might not even compile!!!!

AnderssonPeter commented 1 year ago

@tomMoulard any chance this could be merged and released?

AnderssonPeter commented 1 year ago

@tomMoulard is there any thing I can improve to get this merged? as this is a show blocker for me.

tomMoulard commented 1 year ago

Hey @AnderssonPeter, I will look into this ASAP. Thanks for your interest in this Traefik Plugin !

Schlauer-Hax commented 1 year ago

@tomMoulard Did you have time to look into this yet? Would appreciate the fix!

fradeve commented 9 months ago

Hello everyone :wave: is there any progress on this one? I would have happily helped if I knew Go :sweat_smile:

tomMoulard commented 9 months ago

Just letting you know that this PR does not solve #67 nor #59, as it is only a cosmetic rewrite of the block.

The mutex will be unlocked after the end of the method call, whereas before, if was unlocked at the end of the ServeHTTP function. This is why I do not see a groundbreaking change in this PR, and I am not that enthusiastic in merging it.

If someone have another opinion, and can add tests to this PR to prove me wrong, I will happily and promptly move forward on this topic !

AnderssonPeter commented 9 months ago

First time writing go code as I said, so I believe you :) you can close the pr then :).

But it would be nice if you provided a fix of your own.

AnderssonPeter commented 9 months ago

The idea was that the lock taken inside CheckRequest should be released when it exits the function, as the function is done before we handle the request.

But I guess that I either misunderstand the issue or don't know how defer works in go.

Hope you find a alternative solution.

tomMoulard commented 9 months ago

yes indeed, but I would like to find a way to remove the lock entirely

AnderssonPeter commented 9 months ago

Sadly I can't help you there, I have no idea if go has any lockless data structures.

tomMoulard commented 9 months ago

As I fixed the deadlock issue on ws, I implemented your solution in #77, thanks a lot @AnderssonPeter !