[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes  added the comment:

Yes, the fix works well enough.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Nathaniel Smith]

Nathaniel Smith  added the comment:

Am I right that this is considered fixed (or at least as fixed as it can be), 
and the issue should be closed?

Also, small note in case anyone stumbles across this in the future and is 
confused: Python does *not* handle this correctly on Windows. I suspect 
Christian was confused because there's an undocumented features on Windows 
where if you pass fileno=, then that 
correctly reinstantiates the socket object. But fileno= 
doesn't seem to do any special autodetection of type/family/proto.

--
nosy: +njs

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[YoSTEALTH]

YoSTEALTH  added the comment:

It would be nice if "python" accounted for such low level os things. None the 
less client.detach() method works fine.

I really did enjoy your talk, kinda bummed it was short and didn't get into 
more details.

Thanks for your help and patience Christian :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes  added the comment:

File descriptors are a advanced features and expose low level operating system 
resources. You really have to understand how the OS works. They cannot be 
reference counted. In fact they *are* the reference to entries in the Kernel's 
global open file table. I gave a talked about FDs at PyCon US two years ago, 
maybe 
https://speakerdeck.com/tiran/pycon-2016-file-descriptors-unix-sockets-and-other-posix-magic
 will help you understand fds better.

You can make your example work with 
https://docs.python.org/3/library/socket.html#socket.socket.detach

def accept(sock):
client, addr = sock.accept()
inside = socket(fileno=client.fileno())
client.detach()
print(inside)
return inside
# after return, the client socket is closed by due to detach the fd isn't 
no longer close

The feature works as intended. It's designed to turn an inherited file 
descriptor (e.g. systemd socket activation) or transfered fds (e.g. through 
AF_UNIX SCM_RIGHTS). In both cases the fd is already a duplicated fd.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[YoSTEALTH]

YoSTEALTH  added the comment:

Christian thank you for your reply, i really appreciate it.

Lets analyze this a bit:
- Technically speaking i can "return client" directly and it would NOT close 
the socket.
- Shouldn't "inside" having reference to same fd mean +1 to reference count. 
Considering its a new object?
id(client): 140340037397192
id(inside): 140340010863560

I could understand making a duplicate of the fd if it were being passed across 
thread/process but in its original thread making a duplicate! This behavior is 
extremely odd.

I fell like everyone that uses socket(fileno) will run into days wasted and 
frustration just to figure out these solutions.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes  added the comment:

The problem is fixed and your example behaves as expected.

The laddr string is the result of inside.getsockname() and raddr string is the 
result of inside.getpeername(). In your example, inside and client share the 
same file descriptor. When the function exits client goes out of scope, the 
reference count of client drops to 0, the object gets deallocated and Python 
closes the shared file descriptor. In outside, the shared fd is gone and 
outside.getsockname() and outside.getpeername() fail. Basically the outside 
socket is dead because its fd has been closed.

You have to duplicate the fd or detach the socket to keep the fd open.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[YoSTEALTH]

YoSTEALTH  added the comment:

I am using 3.7.0b1 i don't think this issue is fixed!

# simple mockup:
# --
def accept(sock):
client, addr = sock.accept()
inside = socket(fileno=client.fileno())
print(inside)
# <__main__.Socket fd=5, family=AddressFamily.AF_INET, 
type=SocketKind.SOCK_STREAM, proto=6, laddr=('127.0.0.1', 8000), 
raddr=('127.0.0.1', 42532)>
return inside

outside = accept(sock)
print(outside)
# <__main__.Socket fd=5, family=AddressFamily.AF_INET, 
type=SocketKind.SOCK_STREAM, proto=6>


So the "laddr" and "raddr" goes missing the second its out of the function???

This has wasted days of my time, to even get to this point of figuring out 
whats going on wasn't easy! extremely frustrating.

--
nosy: +YoSTEALTH

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes  added the comment:


New changeset 2e0ecde8d74f5fc0e3e3e39216975cc70efc4796 by Christian Heimes in 
branch 'master':
bpo-28134: Ignore proto in unknown socket test (GH-5435)
https://github.com/python/cpython/commit/2e0ecde8d74f5fc0e3e3e39216975cc70efc4796


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Change by Christian Heimes :


--
pull_requests: +5268

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes  added the comment:

The test for proto isn't super critical. It's mostly ignored any way. I'll 
submit a band-aid.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[STINNER Victor]

STINNER Victor  added the comment:

x86-64 Sierra 3.x is grumpy:

http://buildbot.python.org/all/#/builders/14/builds/659

==
FAIL: test_uknown_socket_family_repr (test.test_socket.GeneralModuleTests)
--
Traceback (most recent call last):
  File 
"/Users/buildbot/buildarea/3.x.billenstein-sierra/build/Lib/test/test_socket.py",
 line 1645, in test_uknown_socket_family_repr
self.assertEqual(s.proto, 23)
AssertionError: 0 != 23

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes  added the comment:

I'm leaving the ticket open to remind me that I have to add a whatsnew entry 
and maybe consider a backport.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes  added the comment:


New changeset b6e43af669f61a37a29d8ff0785455108e6bc29d by Christian Heimes in 
branch 'master':
bpo-28134: Auto-detect socket values from file descriptor (#1349)
https://github.com/python/cpython/commit/b6e43af669f61a37a29d8ff0785455108e6bc29d


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes  added the comment:

Issue #32454 adds socket.close(fd) function.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes  added the comment:

Yuri,

* The patch is purely about Python's view of the socket. The actual behavior of 
the OS socket fd is not influenced by socket.socket()'s family, type and 
protocol. However the especially the family is critical for Python because a 
lot of socket code uses the family to decide how to format its address or the 
peer address.

* On some platforms and/or for some socket types, it is not possible to get the 
address family or type unless connect() or bind() have been called. On Linux it 
seems to work for both new sockets and bound/connected sockets. On Windows it 
never works for a fresh socket.

* It looks like Windows doesn't have protocol dedicated and always uses 0 as 
protocols.

* Using the wrong type is less of an issue. Using the wrong family is really 
really bad, see https://bugs.python.org/issue28134#msg285972.

* I don't care much about validating the values. I'm concerned to have correct 
values by default. Validation can be implemented in a separate PR. In that case 
we want to add socket.closefd(fd: int). On Windows os.close() can't be used for 
socket fds.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Yury Selivanov]

Yury Selivanov  added the comment:

Let's quickly iterate over what's possible first:

* It's possible to check the type of the FD using "getsockopt(SOL_SOCKET, 
SO_TYPE)" on all platforms.

* It's possible to check family/proto of the FD using "getsockopt(SOL_SOCKET, 
SO_DOMAIN/SO_PROTOCOL)" on Linux.

Given the above I propose the following:

1. Passing a wrong type can be considered as a serious error.  SOCK_STREAM is 
fundamentally different from SOCK_DGRAM.  Because we have a way to validate 
socket type on all platforms, I propose to do this validation when we create a 
socket from an FD, and raise an error if the passed socket type isn't correct.

2. When Python is running with '-X dev' (new dev mode added by Victor), I 
propose to also validate socket.family and socket.proto on Linux, and raise a 
RuntimeWarning when they don't match.

--
nosy: +pitrou, vstinner, yselivanov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Changes by Christian Heimes :


--
pull_requests: +1461

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes added the comment:

Here is another example of how broken and dangerous fileno argument is. 
getpeername() is neither a valid IPv4 tuple nor a valid IPv6 tuple. It's all 
messed up:

>>> import socket
>>> s = socket.create_connection(('www.python.org', 443))
>>> s

>>> socket.socket(fileno=s.fileno())


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Changes by Christian Heimes :


--
assignee:  -> christian.heimes
components: +Extension Modules
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes added the comment:

I'm well aware that it does not work in all cases. However it works good enough 
in most cases. Right now the fileno argument must be considered broken because 
it leads to wrong results. It is a problem and possible security issue for a 
couple of use cases, e.g. passing of sockets through AF_UNIX AUX data or 
systemd socket activation.

On Windows it is less problematic because socket(filno) works correctly with 
WSAPROTOCOL_INFO. It's only broken for integer fd.

I have considered to set type, family and proto to 0 (unspec) when the 
getsockopt and getsockname fail.

I have a differnt ticket for the protocol issue, #27816.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Martin Panter]

Martin Panter added the comment:

Personally, I’m not too enthusiastic, because it is rather magical, and does 
not work in all cases. It seems more like a feature than a bug fix. But I have 
rarely used the fileno=... parameter, and it shouldn’t have much negative 
impact, so I’m not too fussed.

According to Issue 27377, these are some cases where parts won’t work:

* Windows and OS X (and older versions of Linux and BSD) don’t have SO_PROTOCOL
* getsockname() not guaranteed to work on unbound sockets, especially on 
Windows, and Free BSD with SCTP sockets

Also, if we are going to read SO_PROTOCOL when fileno=... is given, why not 
also read it in the normal case when proto=0 (unspecified) is given?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes added the comment:

How about we fix the code and only document the limitations instead? :) After 
all it works fine on Windows and it is documented to work on all operating 
systems. Since it's a bug we can fix it in 3.5, too.

My patch implements a best-effort to get type, family and proto from the 
socket. It ignores any errors.

--
keywords: +patch
Added file: http://bugs.python.org/file44687/sock_initobj_types.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Martin Panter]

Martin Panter added the comment:

I agree the doc is far from perfect. The bit I was going off is just above 
, saying 
“these (read-only) attributes that correspond to the values given to the socket 
constructor”.

My instinct would be to clarify that for existing versions 2.7, 3.5, etc, that 
the constructor arguments are _not_ ignored and should correspond to the file 
descriptor. Then in the next Python version we can make it more automatic using 
the getsockopt() techniques.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes added the comment:

Martin, the documentation says "If fileno is specified, the other arguments are 
ignored, causing the socket with the specified file descriptor to return." It's 
a direct quote from Python 3's socket library documentation. For a non-native 
speaker like me, this sentence implies that socket.socket(fileno) not only 
ignores the arguments (which it does not) but that the constructor uses the 
fileno to set family, type and proto.

The socket module uses self->sock_family in several places to calculate the 
addr len or when it handles arguments and address information. Just look at 
this simple example:

>>> import socket
>>> uds = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
>>> s = socket.socket(fileno=uds.fileno())
>>> s.bind('/tmp/sock')
Traceback (most recent call last):
  File "", line 1, in 
TypeError: getsockaddrarg: AF_INET address must be tuple, not str
>>> uds.bind('/tmp/sock')

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Martin Panter]

Martin Panter added the comment:

The documentation says that the family, type and proto attributes correspond to 
the constructor arguments. Although it is unfortunate and quirky, I think your 
behaviour does match the documentation.

Do the mismatched settings cause any serious problems with socket methods, or 
just affect the Python-level attributes and repr()?

Even without using fileno=..., you could argue that the proto attribute is not 
ideal:
>>> s = socket()
>>> s.proto
0
>>> s.getsockopt(SOL_SOCKET, SO_PROTOCOL)
6
>>> IPPROTO_TCP
6

Perhaps the way forward is to deprecate fileno=... in favour of Neil’s 
fromfd2() function. That would avoid any confusion about conflicting socket() 
constructor arguments and defaults. I.e. what does socket(AF_INET, SOCK_STREAM, 
fileno=unix_datagram_fd) mean, and is it equivalent to 
socket(filno=unix_datagram_fd)?

--
nosy: +martin.panter

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[R. David Murray]

R. David Murray added the comment:

See also issue 27377?

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Christian Heimes added the comment:

I expected that socket.socket(fileno) would fill in all socket options like my 
own implementation 
https://github.com/tiran/socketfromfd/blob/master/socketfromfd.py

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

Changes by Christian Heimes :


--
stage:  -> needs patch
type:  -> behavior

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28134] socket.socket(fileno=fd) does not work as documented

[Christian Heimes]

New submission from Christian Heimes:

Documentation of socket.socket(fileno) 
https://docs.python.org/3/library/socket.html#socket.socket says:

> If fileno is specified, the other arguments are ignored, causing the socket 
> with the specified file descriptor to return.

The feature does not work. fileno does not infer the values for family, type 
and proto from the fd. Instead if uses the other arguments. I don't see how 
this feature should have ever worked on POSIX. There are no calls to 
getsockopt() with SO_DOMAIN, SO_TYPE and SO_PROTOCOL.

$ ./python 
Python 3.7.0a0 (default:6bcedf96d25f, Sep 13 2016, 20:48:50) 
[GCC 6.1.1 20160621 (Red Hat 6.1.1-3)] on linux
Type "help", "copyright", "credits" or "license" for more information.

>>> import socket
>>> uds = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
>>> uds

>>> s = socket.socket(fileno=uds.fileno())
>>> s

>>> s.family == uds.family
False
>>> s2 = socket.socket(type=socket.SOCK_DGRAM, fileno=uds.fileno())
>>> s2


--
messages: 276327
nosy: christian.heimes
priority: normal
severity: normal
status: open
title: socket.socket(fileno=fd) does not work as documented
versions: Python 3.5, Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com