DataGramSocket::read doesn't always fill in IP address

Hi, with the following call:

        const auto res = m_socket.read( buf, 1024, false, ip, port );

The ip address string sometimes contains 1.0.0.0, or “[inet_ntoa error” rather than the actual address.
The res return code is value and so is the buffer.

thx

Running into the same issue today, this happens when reading a number of times continued in a loop. Getting either “[inet_ntoa error” or 8.0.0.0 here. However the content of the buffer received looks valid.

I just hacked an assertion into SocketHelpers::readSocket to see if the full error message contained a bit more information, but it doesn’t seem so:

I’m on macOS 10.15.6

Update after some investigation. Searching the internet for [inet_ntoa error] revealed the source code for the open source BSD implementation of inet_ntoa from opensource.apple.com

Afaik, this is the implementation that’s actually used on macOS and it looks like this:

/*
 * Convert network-format internet address
 * to base 256 d.d.d.d representation.
 */
char *
inet_ntoa(in)
	struct in_addr in;
{
	static char ret[18];

	strcpy(ret, "[inet_ntoa error]");
	(void) inet_ntop(AF_INET, &in, ret, sizeof ret);
	return (ret);
}

We can see that the char buffer is initialised with that string and then inet_ntop gets called – without checking the return value of that function. At least the Linux man pages for inet_ntop state that

inet_ntop extends the inet_ntoa(3) function to support multiple address families, inet_ntoa(3) is now considered to be deprecated in favor of inet_ntop)

Now looking back at the Apple implementation, I see that the char buffer used there is static, which makes somewhat sense, since the function only returns a char pointer, but this also means that this function is not thread safe. So my assumption is that this is actually a thread safety issue here.

Modifying the juce::SocketHelpers::readSocket implementation to use inet_ntop with a local char buffer like this

solved the problem. According to git annotate, this code has not been touched for a few years, so I really wonder why others did not run into this problem before. I’m curious to hear what the JUCE team thinks about this?