gh-124944: Add socket.SO_ORIGINAL_DST

[Stevenjin8]

#124944

[ghost]

All commit authors signed the Contributor License Agreement.

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[rruuaanng]

Also, you need to test this attribute in the corresponding test.

[Stevenjin8]

@rruuaanng do you have any ideas on how to write a meaningful test?

The test should only run if the system has netfilter support.
We know this at build time through the HAVE_LINUX_NETFILTER_IPV4_H macro, but we don't have access to this when running the tests.
Other tests make similar checks by inspecting the attributes of socket.
For example, we check for bluetooth support by checking if socket.{AF_BLUETOOTH,BTPROTO_RFCOMM} are defined (amongst other things):

def _have_socket_bluetooth():
    """Check whether AF_BLUETOOTH sockets are supported on this host."""
    try:
        s = socket.socket(socket.AF_BLUETOOTH, socket.SOCK_STREAM, socket.BTPROTO_RFCOMM)
    except (AttributeError, OSError):
        return False
    else:
        s.close()
    return True
# ...
HAVE_SOCKET_BLUETOOTH = _have_socket_bluetooth()

And then we run tests only if there is bluetooth support

@unittest.skipUnless(HAVE_SOCKET_BLUETOOTH,
                     'Bluetooth sockets required for this test.')
class BasicBluetoothTest(unittest.TestCase):

    def testBluetoothConstants(self):
        socket.BDADDR_ANY
        socket.BDADDR_LOCAL
        socket.AF_BLUETOOTH
        socket.BTPROTO_RFCOMM
    # ...

So the bluetooth tests are a bit circular because we test for socket.{AF_BLUETOOTH,BTPROTO_RFCOMM} only if socket.{AF_BLUETOOTH,BTPROTO_RFCOMM} are defined.
But in the bluetooth case, the tests run other checks, so they are still meaningful.

However, in the case of SO_ORIGINAL_DESTINATION/netfilter, the only way to check whether there is netfilter support is by checking if socket.SO_ORIGINAL_DESTINATION is defined. If I were to model the test for socket.SO_ORIGINAL_DST after the existing ones, the test would effectively be something like

if hasattr(socket, "SO_ORIGINAL_DST"):
    assert hasattr(socket, "SO_ORIGINAL_DST")

which is dead code.

[rruuaanng]

if hasattr(socket, "SO_ORIGINAL_DST"):
    assert hasattr(socket, "SO_ORIGINAL_DST")

Actually, you can integrate this code into the existing tests for the sockets attribute, rather than writing a new test. It's meaning(Maybe).

Edit： And，It seems that SO_ORIGINAL_DST is unique to Unix-like system. Perhaps you can modify the test to skip it if the current platform is not Unix-like.

Edit: Maybe you should to see Lib/test/test_socket.py refer to the relevant unit test.

[Stevenjin8]

@rruuaanng My understanding is that SO_ORIGINAL_DST is only for linux as it is part of the netfilter project:

The netfilter project is a community-driven collaborative FOSS project that provides packet filtering software for the Linux 2.4.x and later kernel series

And although most Linux distributions have netfilter and related headers (I couldn't find one without), Linux does not imply netfilter/SO_ORIGINAL_DST. You can disable it if you're building your own kernel/distribution.
Thus, we can't do something like

if sys.platform.startswith('linux'):
   socket.SO_ORIGINAL_DST

as tests in Lib/test/test_socket.py do because we would require systems with linux to have netfilter.

After writing all that, I do think that we can check whether socket.SO_ORIGINAL_DST works as expected if its defined.

if hasattr(socket, "SO_ORIGINAL_DST"):
    s = # set up socket
    opt = getsockopt(s, socket.SOL_IP, socket.SO_ORIGINAL_DST)
    # check that `opt` makes sense

This way we don't assert that linux systems have netfilter, but we do say that if socket.SO_ORIGINAL_DST exists, it should work with setsockopt.

Let me know what you think

[rruuaanng]

You can actually add a description of this attribute in the documentation.
For example, you can state that it only applies to the linux. Since this is already explained in the documentation, there’s no need to modify other content.
You just need to include a check in the tests so that this particular test is only run on linux, as you demonstrated.

[picnixz]

I don't think there's need to test this since we don't test other non-crucial constants, nor is there a need to add a dedicated documented entry (the docs just gather most constants under SO_*). Only crucial constants such as socket.AF_INET are really tested for their existence.

If you want a test, you can just do something like this:

@unittest.skipUnless(sysconfig.get_config_var('HAVE_LINUX_NETFILTER_IPV4_H'), 
					 "requires <linux/netfilter_ipv4.h>")
def test_socket_linux_netfilter_ipv4(self):
	socket.SO_ORIGINAL_DST

[Stevenjin8]

going to ignore the docs error because it also happens for SO_REUSEPORT and SO_EXCLUSIVEADDRESS

[erlend-aasland]

Bénédict, are you happy with the changes? @picnixz

[picnixz]

LGTM.

[erlend-aasland]

Let's go! Thanks, @Stevenjin8.