cpython: Fix closes Issue11281 - smtplib.STMP gets source_address parameter, which adds

http://hg.python.org/cpython/rev/26839edf3cc1 changeset: 71617:26839edf3cc1 parent: 71613:018e14a46454 user: Senthil Kumaran <senthil@uthcode.com> date: Sat Jul 30 10:56:50 2011 +0800 summary: Fix closes Issue11281 - smtplib.STMP gets source_address parameter, which adds the ability to bind to specific source address on a machine with multiple interfaces. Patch by Paulo Scardine. files: Doc/library/smtplib.rst | 33 +++++++++++++++++----- Lib/smtplib.py | 40 ++++++++++++++++++--------- Lib/test/mock_socket.py | 3 +- Lib/test/test_smtplib.py | 17 +++++++++++ Misc/NEWS | 4 ++ 5 files changed, 75 insertions(+), 22 deletions(-) diff --git a/Doc/library/smtplib.rst b/Doc/library/smtplib.rst --- a/Doc/library/smtplib.rst +++ b/Doc/library/smtplib.rst @@ -20,7 +20,7 @@ Protocol) and :rfc:`1869` (SMTP Service Extensions). -.. class:: SMTP(host='', port=0, local_hostname=None[, timeout]) +.. class:: SMTP(host='', port=0, local_hostname=None[, timeout], source_address=None) A :class:`SMTP` instance encapsulates an SMTP connection. It has methods that support a full repertoire of SMTP and ESMTP operations. If the optional @@ -29,7 +29,12 @@ raised if the specified host doesn't respond correctly. The optional *timeout* parameter specifies a timeout in seconds for blocking operations like the connection attempt (if not specified, the global default timeout - setting will be used). + setting will be used). The optional source_address parameter allows to bind to some + specific source address in a machine with multiple network interfaces, + and/or to some specific source tcp port. It takes a 2-tuple (host, port), + for the socket to bind to as its source address before connecting. If + ommited (or if host or port are '' and/or 0 respectively) the OS default + behavior will be used. For normal use, you should only require the initialization/connect, :meth:`sendmail`, and :meth:`quit` methods. An example is included below. @@ -48,8 +53,10 @@ .. versionchanged:: 3.3 Support for the :keyword:`with` statement was added. + .. versionadded:: 3.3 + source_address parameter. -.. class:: SMTP_SSL(host='', port=0, local_hostname=None, keyfile=None, certfile=None[, timeout], context=None) +.. class:: SMTP_SSL(host='', port=0, local_hostname=None, keyfile=None, certfile=None[, timeout], context=None, source_address=None) A :class:`SMTP_SSL` instance behaves exactly the same as instances of :class:`SMTP`. :class:`SMTP_SSL` should be used for situations where SSL is @@ -62,18 +69,28 @@ keyfile and certfile must be None. The optional *timeout* parameter specifies a timeout in seconds for blocking operations like the connection attempt (if not specified, the global default timeout setting - will be used). + will be used). The optional source_address parameter allows to bind to some + specific source address in a machine with multiple network interfaces, + and/or to some specific source tcp port. It takes a 2-tuple (host, port), + for the socket to bind to as its source address before connecting. If + ommited (or if host or port are '' and/or 0 respectively) the OS default + behavior will be used. .. versionchanged:: 3.3 *context* was added. + .. versionadded:: 3.3 + source_address parameter. -.. class:: LMTP(host='', port=LMTP_PORT, local_hostname=None) + +.. class:: LMTP(host='', port=LMTP_PORT, local_hostname=None, source_address=None) The LMTP protocol, which is very similar to ESMTP, is heavily based on the - standard SMTP client. It's common to use Unix sockets for LMTP, so our :meth:`connect` - method must support that as well as a regular host:port server. To specify a - Unix socket, you must use an absolute path for *host*, starting with a '/'. + standard SMTP client. It's common to use Unix sockets for LMTP, so our + :meth:`connect` method must support that as well as a regular host:port + server. The optional parameters local_hostname and source_address has the + same meaning as that of SMTP client.To specify a Unix socket, you must use + an absolute path for *host*, starting with a '/'. Authentication is supported, using the regular SMTP mechanism. When using a Unix socket, LMTP generally don't support or require any authentication, but your diff --git a/Lib/smtplib.py b/Lib/smtplib.py --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -215,7 +215,8 @@ default_port = SMTP_PORT def __init__(self, host='', port=0, local_hostname=None, - timeout=socket._GLOBAL_DEFAULT_TIMEOUT): + timeout=socket._GLOBAL_DEFAULT_TIMEOUT, + source_address=None): """Initialize a new instance. If specified, `host' is the name of the remote host to which to @@ -223,11 +224,16 @@ By default, smtplib.SMTP_PORT is used. An SMTPConnectError is raised if the specified `host' doesn't respond correctly. If specified, `local_hostname` is used as the FQDN of the local host. By default, - the local hostname is found using socket.getfqdn(). + the local hostname is found using socket.getfqdn(). The + `source_address` parameter takes a 2-tuple (host, port) for the socket + to bind to as its source address before connecting. If the host is '' + and port is 0, the OS default behavior will be used. """ self.timeout = timeout self.esmtp_features = {} + self.source_address = source_address + if host: (code, msg) = self.connect(host, port) if code != 220: @@ -276,10 +282,11 @@ # This makes it simpler for SMTP_SSL to use the SMTP connect code # and just alter the socket connection bit. if self.debuglevel > 0: - print('connect:', (host, port), file=stderr) - return socket.create_connection((host, port), timeout) + print('connect: to', (host, port), self.source_address, file=stderr) + return socket.create_connection((host, port), timeout, + self.source_address) - def connect(self, host='localhost', port=0): + def connect(self, host='localhost', port=0, source_address=None): """Connect to a host on a given port. If the hostname ends with a colon (`:') followed by a number, and @@ -290,6 +297,7 @@ specified during instantiation. """ + if source_address: self.source_address = source_address if not port and (host.find(':') == host.rfind(':')): i = host.rfind(':') if i >= 0: @@ -829,7 +837,8 @@ """ This is a subclass derived from SMTP that connects over an SSL encrypted socket (to use this class you need a socket module that was compiled with SSL support). If host is not specified, '' (the local host) is used. If port is - omitted, the standard SMTP-over-SSL port (465) is used. keyfile and certfile + omitted, the standard SMTP-over-SSL port (465) is used. The optional + source_address takes a two-tuple (host,port) for socket to bind to. keyfile and certfile are also optional - they can contain a PEM formatted private key and certificate chain file for the SSL connection. context also optional, can contain a SSLContext, and is an alternative to keyfile and certfile; If it is specified both @@ -840,7 +849,8 @@ def __init__(self, host='', port=0, local_hostname=None, keyfile=None, certfile=None, - timeout=socket._GLOBAL_DEFAULT_TIMEOUT, context=None): + timeout=socket._GLOBAL_DEFAULT_TIMEOUT, + source_address=None, context=None): if context is not None and keyfile is not None: raise ValueError("context and keyfile arguments are mutually " "exclusive") @@ -850,12 +860,14 @@ self.keyfile = keyfile self.certfile = certfile self.context = context - SMTP.__init__(self, host, port, local_hostname, timeout) + SMTP.__init__(self, host, port, local_hostname, timeout, + source_address) def _get_socket(self, host, port, timeout): if self.debuglevel > 0: print('connect:', (host, port), file=stderr) - new_socket = socket.create_connection((host, port), timeout) + new_socket = socket.create_connection((host, port), timeout, + self.source_address) if self.context is not None: new_socket = self.context.wrap_socket(new_socket) else: @@ -884,14 +896,16 @@ ehlo_msg = "lhlo" - def __init__(self, host='', port=LMTP_PORT, local_hostname=None): + def __init__(self, host='', port=LMTP_PORT, local_hostname=None, + source_address=None): """Initialize a new instance.""" - SMTP.__init__(self, host, port, local_hostname) + SMTP.__init__(self, host, port, local_hostname = local_hostname, + source_address = source_address) - def connect(self, host='localhost', port=0): + def connect(self, host='localhost', port=0, source_address=None): """Connect to the LMTP daemon, on either a Unix or a TCP socket.""" if host[0] != '/': - return SMTP.connect(self, host, port) + return SMTP.connect(self, host, port, source_address = source_address) # Handle Unix-domain sockets. try: diff --git a/Lib/test/mock_socket.py b/Lib/test/mock_socket.py --- a/Lib/test/mock_socket.py +++ b/Lib/test/mock_socket.py @@ -106,7 +106,8 @@ return MockSocket() -def create_connection(address, timeout=socket_module._GLOBAL_DEFAULT_TIMEOUT): +def create_connection(address, timeout=socket_module._GLOBAL_DEFAULT_TIMEOUT, + source_address=None): try: int_port = int(address[1]) except ValueError: diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py --- a/Lib/test/test_smtplib.py +++ b/Lib/test/test_smtplib.py @@ -72,6 +72,14 @@ smtp = smtplib.SMTP(HOST, self.port) smtp.close() + def testSourceAddress(self): + mock_socket.reply_with(b"220 Hola mundo") + # connects + smtp = smtplib.SMTP(HOST, self.port, + source_address=('127.0.0.1',19876)) + self.assertEqual(smtp.source_address, ('127.0.0.1', 19876)) + smtp.close() + def testBasic2(self): mock_socket.reply_with(b"220 Hola mundo") # connects, include port in host name @@ -206,6 +214,15 @@ smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3) smtp.quit() + def testSourceAddress(self): + # connect + smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3, + source_address=('127.0.0.1', 19876)) + self.assertEqual(smtp.source_address, ('127.0.0.1', 19876)) + self.assertEqual(smtp.local_hostname, 'localhost') + print(dir(smtp)) + smtp.quit() + def testNOOP(self): smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3) expected = (250, b'Ok') diff --git a/Misc/NEWS b/Misc/NEWS --- a/Misc/NEWS +++ b/Misc/NEWS @@ -246,6 +246,10 @@ Library ------- +- Issue #11281: smtplib.STMP gets source_address parameter, which adds the + ability to bind to specific source address on a machine with multiple + interfaces. Patch by Paulo Scardine. + - Issue #12464: tempfile.TemporaryDirectory.cleanup() should not follow symlinks: fix it. Patch by Petri Lehtinen. -- Repository URL: http://hg.python.org/cpython

Hi, On 30/07/2011 5.58, senthil.kumaran wrote:
http://hg.python.org/cpython/rev/26839edf3cc1 changeset: 71617:26839edf3cc1 parent: 71613:018e14a46454 user: Senthil Kumaran<senthil@uthcode.com> date: Sat Jul 30 10:56:50 2011 +0800 summary: Fix closes Issue11281 - smtplib.STMP gets source_address parameter, which adds the ability to bind to specific source address on a machine with multiple interfaces. Patch by Paulo Scardine.
files: Doc/library/smtplib.rst | 33 +++++++++++++++++----- Lib/smtplib.py | 40 ++++++++++++++++++--------- Lib/test/mock_socket.py | 3 +- Lib/test/test_smtplib.py | 17 +++++++++++ Misc/NEWS | 4 ++ 5 files changed, 75 insertions(+), 22 deletions(-)
diff --git a/Doc/library/smtplib.rst b/Doc/library/smtplib.rst --- a/Doc/library/smtplib.rst +++ b/Doc/library/smtplib.rst @@ -20,7 +20,7 @@ Protocol) and :rfc:`1869` (SMTP Service Extensions).
-.. class:: SMTP(host='', port=0, local_hostname=None[, timeout]) +.. class:: SMTP(host='', port=0, local_hostname=None[, timeout], source_address=None)
The "[, timeout]" now looks weird there, and it would be better to convert it to ", timeout=..." to match the other args. However I don't know what the value should be, since the real value is socket._GLOBAL_DEFAULT_TIMEOUT (i.e. object()) and I don't think it's a good idea to expose that. Maybe "None" can be used instead?
A :class:`SMTP` instance encapsulates an SMTP connection. It has methods that support a full repertoire of SMTP and ESMTP operations. If the optional @@ -29,7 +29,12 @@ raised if the specified host doesn't respond correctly. The optional *timeout* parameter specifies a timeout in seconds for blocking operations like the connection attempt (if not specified, the global default timeout - setting will be used). + setting will be used). The optional source_address parameter allows to bind to some + specific source address in a machine with multiple network interfaces, + and/or to some specific source tcp port. It takes a 2-tuple (host, port),
I think TCP should be uppercase.
+ for the socket to bind to as its source address before connecting. If + ommited (or if host or port are '' and/or 0 respectively) the OS default
s/ommited/omitted/ and s/''/``''``/
+ behavior will be used.
For normal use, you should only require the initialization/connect, :meth:`sendmail`, and :meth:`quit` methods. An example is included below. @@ -48,8 +53,10 @@ .. versionchanged:: 3.3 Support for the :keyword:`with` statement was added.
+ .. versionadded:: 3.3 + source_address parameter.
I think the convention is to use "versionadded" when the function/method/class/etc has been added, and "versionchanged" for all the changes, including new arguments.
-.. class:: SMTP_SSL(host='', port=0, local_hostname=None, keyfile=None, certfile=None[, timeout], context=None) +.. class:: SMTP_SSL(host='', port=0, local_hostname=None, keyfile=None, certfile=None[, timeout], context=None, source_address=None)
Ditto for "[, timeout]" and the typos/markup below.
A :class:`SMTP_SSL` instance behaves exactly the same as instances of :class:`SMTP`. :class:`SMTP_SSL` should be used for situations where SSL is @@ -62,18 +69,28 @@ keyfile and certfile must be None. The optional *timeout* parameter specifies a timeout in seconds for blocking operations like the connection attempt (if not specified, the global default timeout setting - will be used). + will be used). The optional source_address parameter allows to bind to some + specific source address in a machine with multiple network interfaces, + and/or to some specific source tcp port. It takes a 2-tuple (host, port), + for the socket to bind to as its source address before connecting. If + ommited (or if host or port are '' and/or 0 respectively) the OS default + behavior will be used.
.. versionchanged:: 3.3 *context* was added.
+ .. versionadded:: 3.3 + source_address parameter.
-.. class:: LMTP(host='', port=LMTP_PORT, local_hostname=None) + +.. class:: LMTP(host='', port=LMTP_PORT, local_hostname=None, source_address=None)
The LMTP protocol, which is very similar to ESMTP, is heavily based on the - standard SMTP client. It's common to use Unix sockets for LMTP, so our :meth:`connect` - method must support that as well as a regular host:port server. To specify a - Unix socket, you must use an absolute path for *host*, starting with a '/'. + standard SMTP client. It's common to use Unix sockets for LMTP, so our + :meth:`connect` method must support that as well as a regular host:port + server. The optional parameters local_hostname and source_address has the
s/has/have/? Also I prefer 'arguments' rather than 'parameters', the smtplib doc uses both, but 'arguments' seems to be used more.
+ same meaning as that of SMTP client.To specify a Unix socket, you must use
Missing space after the '.' (there should be two spaces, but here a single space is used consistently so it's fine).
+ an absolute path for *host*, starting with a '/'.
Authentication is supported, using the regular SMTP mechanism. When using a Unix socket, LMTP generally don't support or require any authentication, but your diff --git a/Lib/smtplib.py b/Lib/smtplib.py --- a/Lib/smtplib.py +++ b/Lib/smtplib.py @@ -215,7 +215,8 @@ [...]
Best Regards, Ezio Melotti

On Sat, Jul 30, 2011 at 11:11:08PM +0300, Ezio Melotti wrote:
-.. class:: SMTP(host='', port=0, local_hostname=None[, timeout]) +.. class:: SMTP(host='', port=0, local_hostname=None[, timeout], source_address=None)
The "[, timeout]" now looks weird there, and it would be better to convert it to ", timeout=..." to match the other args. However I don't know what the value should be, since the real value is socket._GLOBAL_DEFAULT_TIMEOUT (i.e. object()) and I don't think it's a good idea to expose that. Maybe "None" can be used instead?
I found that [, timeout] bit odd too. But is not mentioning that as timeout=None when it is timeout=socket._GLOBAL_DEFAULT_TIME technically inaccurate? FWIW, I see similar style (...,[,timeout], kw=val) adopted elsewhere in the library docs too. urllib, httplib, nntplib etc. Though it does not look okay, it is better than giving inaccurate information. While ftplib and poplib, has them as timeout=None, while they default to socket._GLOBAL_DEFAULT_TIMEOUT object. If we decide upon something, it can be made consistent. So, the question is, when the timeout argument refers to socket._GLOBAL_DEFAULT_TIME, how should we write the docs. 1. def somesocketmethod(arg1,arg2, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, ...) - I don't see anything is wrong with this. 2. def somesocketmethod(arg1,arg2, timeout=None, ...) - And explain that it actually points to a socket default timeout object, which is odd and can lead to user errors. 3. def somesocketmethod(arg1,arg2[,timeout], kwarg=value) - That's how it is currently explained at some places. If this style is okay, we can change the places were it refers to None to be above. Thanks for your review comments, I have address the remaining ones. -- Senthil

On Sun, 31 Jul 2011 09:26:33 +0800 Senthil Kumaran <senthil@uthcode.com> wrote:
I found that [, timeout] bit odd too. But is not mentioning that as timeout=None when it is timeout=socket._GLOBAL_DEFAULT_TIME technically inaccurate?
FWIW, I see similar style (...,[,timeout], kw=val) adopted elsewhere in the library docs too. urllib, httplib, nntplib etc. Though it does not look okay, it is better than giving inaccurate information.
Indeed, we don't want to document the private sentinel object.
While ftplib and poplib, has them as timeout=None, while they default to socket._GLOBAL_DEFAULT_TIMEOUT object.
This is wrong; someone may pass None thinking it will trigger the default behaviour, and have nasty surprises.
3. def somesocketmethod(arg1,arg2[,timeout], kwarg=value)
However weird-looking, I think this is the correct solution here. Regards Antoine.

On 31 Jul 2011, at 02:26, Senthil Kumaran wrote:
On Sat, Jul 30, 2011 at 11:11:08PM +0300, Ezio Melotti wrote:
-.. class:: SMTP(host='', port=0, local_hostname=None[, timeout]) +.. class:: SMTP(host='', port=0, local_hostname=None[, timeout], source_address=None)
The "[, timeout]" now looks weird there, and it would be better to convert it to ", timeout=..." to match the other args. However I don't know what the value should be, since the real value is socket._GLOBAL_DEFAULT_TIMEOUT (i.e. object()) and I don't think it's a good idea to expose that. Maybe "None" can be used instead?
I found that [, timeout] bit odd too. But is not mentioning that as timeout=None when it is timeout=socket._GLOBAL_DEFAULT_TIME technically inaccurate?
It does mean that users will expect to be able to call with an explicit timeout=None and get the default behaviour. Just use the numeric value of the global timeout perhaps? MIchael Foord
FWIW, I see similar style (...,[,timeout], kw=val) adopted elsewhere in the library docs too. urllib, httplib, nntplib etc. Though it does not look okay, it is better than giving inaccurate information.
While ftplib and poplib, has them as timeout=None, while they default to socket._GLOBAL_DEFAULT_TIMEOUT object.
If we decide upon something, it can be made consistent. So, the question is, when the timeout argument refers to socket._GLOBAL_DEFAULT_TIME, how should we write the docs.
1. def somesocketmethod(arg1,arg2, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, ...)
- I don't see anything is wrong with this.
2. def somesocketmethod(arg1,arg2, timeout=None, ...)
- And explain that it actually points to a socket default timeout object, which is odd and can lead to user errors.
3. def somesocketmethod(arg1,arg2[,timeout], kwarg=value)
- That's how it is currently explained at some places. If this style is okay, we can change the places were it refers to None to be above.
Thanks for your review comments, I have address the remaining ones.
-- http://www.voidspace.org.uk/ May you do good and not evil May you find forgiveness for yourself and forgive others May you share freely, never taking more than you give. -- the sqlite blessing http://www.sqlite.org/different.html
participants (5)
-
Antoine Pitrou
-
Ezio Melotti
-
Michael Foord
-
Senthil Kumaran
-
senthil.kumaran