[Bug]: critical error handling can be corrupted by logging callbacks (primarily windows)

[mitchcapper]

Contact Details

gh

Version

master

Description

It may be worth trying to translate errors before logging error messages. If a custom callback is used and errno is set it clears WSAGetLastError() which the code needs. This includes if the callback tries to be safe and stores errno and restores it back, as restoring back doesn't set the WSA error. It is possible to store and restore the WSA error (WSAGetLastError/WSASetLastError), but clearly this cold be missed. While sometimes this only causes an issue for error messages there are times where it breaks functionality for example:

https://github.com/wolfSSL/wolfssl/blob/master/src/wolfio.c#L290-L293 when wolfIO_Recv returns with -1 it logs a message then returns translateIOError. Here if errno has been reset then the last error call in translate: https://github.com/wolfSSL/wolfssl/blob/master/src/wolfio.c#L127 which then calls WSAGetLastError() will return 0. If however it should have returned SOCKET_EWOULDBLOCK for example a different return value is set: https://github.com/wolfSSL/wolfssl/blob/master/src/wolfio.c#L135 and that in turn would have a different outcome: https://github.com/wolfSSL/wolfssl/blob/master/src/internal.c#L9852

As this can break internal flow handling it in the library is probably best. If it is decided to put the onus on the caller to adds the log handler I would recommend adding a disclaimer to the documentation talking about the logging callback to ensure they do not set errno (even to its current value).

Reproduction steps

install a logger callback that sets errno=somevalue this should instantly clear the error in WSAGetLastError and break any downloads that log things like need more data.

Relevant log output

No response