fix: broken unclosed files on windows

[jabdr]

This patch fixes tests and log file behaviour on windows. The big problem here is, that windows does not allow to delete files with open file descriptors.

To solve this problem I had to implement the following changes:

• close temp files created during the tests
• close opened log files
• Add io.Closer to the WAF interface to allow closing log files

This was originally intended as a small fix, but now it requires an API change. Technically, it is not required to Close the log files on linux, but I think it is still a good best practice to not rely on GC alone.

I removed the os.Remove calls from testing/auditlog_test.go and replaced them with Close, as go test already cleans them up, but only if they are properly closed (on windows).

[jptosso]

Unfortunately this is a breaking change. We cannot update the public waf interface.
But this is a v4 TODO

[jabdr]

I thought so already. Please let me know if I can do anything to help getting this merged to v4.

[jcchavezs]

This looks good to me. Just remove the closer from the interface and keep the others.

…

On Sat, Aug 24, 2024 at 7:45 PM jabdr ***@***.***> wrote: I thought so already. Please let me know if I can do anything to help getting this merged to v4. — Reply to this email directly, view it on GitHub <#1139 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXOYAS2NXLCDPK7WWOCZILZTDBFLAVCNFSM6AAAAABNBXVCLWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBYGQ3DSNJQGE> . You are receiving this because your review was requested.Message ID: ***@***.***>

[jcchavezs]

Actually giving a second thought we should only close those closeables we open so let's carefully review the cases. On Sun, Aug 25, 2024 at 10:25 AM José Carlos Chávez < ***@***.***> wrote:

…

This looks good to me. Just remove the closer from the interface and keep the others. On Sat, Aug 24, 2024 at 7:45 PM jabdr ***@***.***> wrote: > I thought so already. Please let me know if I can do anything to help > getting this merged to v4. > > — > Reply to this email directly, view it on GitHub > <#1139 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAXOYAS2NXLCDPK7WWOCZILZTDBFLAVCNFSM6AAAAABNBXVCLWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBYGQ3DSNJQGE> > . > You are receiving this because your review was requested.Message ID: > ***@***.***> >

[jabdr]

@jcchavezs

This looks good to me. Just remove the closer from the interface and keep the others.

I can do that. Should I create a seperate PR for v4 that includes the close on the public interface?

Actually giving a second thought we should only close those closeables we open so let's carefully review the cases.

Do you mean the debuglog Close? Let me think about it.

[jabdr]

@jcchavezs @jptosso

I removed the changes to debuglog itself. You are correct, it should not close the debug file handle, as it only receives the io.Writer from WAF. Instead I choose to add a field to the internal WAF struct. So the file creator/opener is also closing the file in the end.

I removed the public WAF interface change from this PR, but I recommend to add it to the next major version.

[jcchavezs]

Please rebase your PR on top of gh-windows