gh-107898: Do not ignore address and flags parameters in socket.{send,recv}_fds

[GalaxySnail]

fix 8d120f7

• Issue: address and flags are ignored in socket.send_fds and socket.recv_fds #128881

• Issue: socket.send_fds and socket.recv_fds ignore optional arguments ('flags' and 'address'). #107898

[GalaxySnail]

I have no idea what's happening on macOS. Should we just skip it on macOS?

[Zheaoli]

MSG_PEEK may got different action on macOS/FreeBSD, let's figure it out.

BTW, we can skip the macOS test first

[picnixz]

Can we also have a test that combine both address and flags as well for send_fds please? and see if we can have test coverage for Windows (just using a 0 flag for instance would be sufficient).

According to https://docs.python.org/3/library/socket.html#socket.send_fds, send_fds and recv_fds are is available on Windows and Unix, so we should be able to have tests for them as well.

[GalaxySnail]

I found that FreeBSD doesn't seem to support receiving only 1 file descriptor with recvmsg. Using CMSG_LEN(sizeof(int)) leads to MSG_CTRUNC, while CMSG_LEN(2 * sizeof(int)) is required to successfully receive a single file descriptor.

Here is a POC: https://fars.ee/Ih-8/c

$ curl https://fars.ee/Ih-8 -o foo.c
$ cc -Wall -Wextra -Wpedantic -std=c17 foo.c && truss -o out.txt ./a.out && ggrep -E 'sendmsg|recvmsg' out.txt
sendmsg(3,{NULL,0,[{"Hello world!",12}],1,{{level=SOL_SOCKET,type=SCM_RIGHTS,data={0x01,0x00,0x00,0x00}}},20,0},0) = 12 (0xc)
recvmsg(4,{NULL,0,[{"Hello world!",12}],1,{},0,MSG_CTRUNC},0) = 12 (0xc)
$ gsed -i 's|fds\[\] = {1}|fds[] = {1, 2}|' foo.c
$ cc -Wall -Wextra -Wpedantic -std=c17 foo.c && truss -o out.txt ./a.out && ggrep -E 'sendmsg|recvmsg' out.txt
sendmsg(3,{NULL,0,[{"Hello world!",12}],1,{{level=SOL_SOCKET,type=SCM_RIGHTS,data={0x01,0x00,0x00,0x00,0x02,0x00,0x00,0x00}}},24,0},0) = 12 (0xc)
recvmsg(4,{NULL,0,[{"Hello world!",12}],1,{{level=SOL_SOCKET,type=SCM_RIGHTS,data={0x05,0x00,0x00,0x00,0x06,0x00,0x00,0x00}}},24,0},0) = 12 (0xc)

[picnixz]

We are close to the end!

[picnixz]

Let's rename the result flags as msg_flags to be consistent with the signature (don't forget to update return ...)

[GalaxySnail]

Done.

[picnixz]

Suggested change

	assert len(msg) < 512
	os.write(wfd, msg)
	data = os.read(rfd, 512)
	self.assertEqual(data, msg)
	os.write(wfd, msg)
	data = os.read(rfd, len(msg))
	self.assertEqual(data, msg)

I think we can do it like this right?

[GalaxySnail]

man 7 pipe says:

POSIX.1 says that writes of less than PIPE_BUF bytes must be atomic: the output data is written to the pipe as a contiguous sequence. [...] POSIX.1 requires PIPE_BUF to be at least 512 bytes.

I hardcoded 512 here, which may look confusing. I've updated it to make it more clear.

[picnixz]

Above, the test for sendmsg only enabled it for @skipWithClientIf(sys.platform not in {"linux", "android", "freebsd"}. I think it's better if we check this here as well because there might be more platforms than {"linux", "android", "freebsd", "darwin"} in the future.

Or: we change the test above with @skipWithClientIf(sys.platform not in ("darwin",) instead and add @requireAttrs(socket, "MSG_DONTWAIT")

[GalaxySnail]

According to PEP 11, it should be safe to replace @skipWithClientIf(sys.platform not in {"linux", "android", "freebsd"}) with @requireAttrs(socket, "MSG_DONTWAIT") and @skipWithClientIf(sys.platform not in ("darwin",)) on all supported platforms. If this test fails on any other platform, we can update it as needed.

[picnixz]

I'm back here, but does it work on freebsd?

[GalaxySnail]

Unfortunately no. On FreeBSD recvmsg with MSG_PEEK returns random bytes in the ancillary data, which looks like some kind of uninitialized memory.

[picnixz]

LGTM. I'll let @gpshead have a look

[GalaxySnail]

Gentle ping. Would it be possible to get this PR merged before Python 3.15.0 beta 1 freeze? Is there anything left we should do?

[picnixz]

I do not think it matters as this will be backported (it is a bugfix) so the feature freeze deadline will not apply