classification
Title: smtplib: add ability to bind to specific source IP address/port
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: giampaolo.rodola, paulos, python-dev, r.david.murray
Priority: normal Keywords: patch

Created on 2011-02-22 04:10 by paulos, last changed 2011-07-30 02:58 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
smtp_lib_source_address.patch paulos, 2011-02-23 01:32 add source_address arg to smtplib.xSMTP review
smtp_lib_source_address.patch paulos, 2011-02-28 13:24 Cleaned up patch review
smtp_lib_source_address.patch paulos, 2011-02-28 13:25 review
Messages (21)
msg129035 - (view) Author: Paulo Scardine (paulos) Date: 2011-02-22 04:10
In smtplib there is now way to bind to an specific source address on a machine with multiple interfaces; also, there no way to control the source port for the connection.

Since 2.7, socket.create_connection accepts a source_address parameter, a (host, port) tuple for the socket to bind to as its source address before connecting. If host or port are '' or 0 respectively the OS default behavior will be used.

I would like to add source_ip and source_port parameters to smtplib.SMTP, default to '' and 0 respectively.

It is a small change with no impact over existing code. Should I submit a patch?
msg129059 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-02-22 12:19
This sounds like a reasonable feature request.  If you would like to propose a patch against trunk (py3k, what will become 3.3), I will take a look at it.
msg129085 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2011-02-22 15:36
+1, this has been done for other modules such as ftplib as well and probably could be done for others such as httplib and poplib.
msg129092 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2011-02-22 16:17
> I would like to add source_ip and source_port parameters to 
> smtplib.SMTP, default to '' and 0 respectively.

It would be better to provide a unique source_address parameter defaulting to None, for consistency with socket.create_connection() expecting a (addr, port) tuple.
msg129098 - (view) Author: Paulo Scardine (paulos) Date: 2011-02-22 17:12
Seems like the signature of Lib.test.mock_socket.create_connection does not match socket.create_connection since 2.7.

I need help with the docs; the last argument is positional and optional, using the `[ ]' notation. What is the notation for the new keyword arguments?
msg129100 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2011-02-22 17:16
> Seems like the signature of Lib.test.mock_socket.create_connection does 
> not match socket.create_connection since 2.7.

You can add a fake parameter which does nothing.

> What is the notation for the new keyword arguments?

fun(a, b, c=None, d=None)
msg129101 - (view) Author: Paulo Scardine (paulos) Date: 2011-02-22 17:18
Also, it is my first patch and I have no clue about what I'm doing, I don't expect to get it right in the first try - please point any mistakes.
msg129103 - (view) Author: Paulo Scardine (paulos) Date: 2011-02-22 17:22
@Giampaolo: should I change the text in Doc/library/smtplib.rst or it is infered from the docstrings?
msg129106 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2011-02-22 17:37
Yes, you must update  Doc/library/smtplib.rst and write tests.
If you're not the one who's gonna commit the patch you can ignore Doc/whatsnew/3.3.srt and Misc/NEWS changes.

Updating the docstring is usually optional but I see smtplib module is already well documented so I'd update docstrings as well.

As for how to write a patch see:
http://www.python.org/dev/faq/#patches
...or search in the bug tracker for committed patches, such as:
http://bugs.python.org/issue8807
http://svn.python.org/view?view=revision&revision=84144
msg129108 - (view) Author: Paulo Scardine (paulos) Date: 2011-02-22 18:33
My first idea was to make the argument a tuple to match socket.create_connection(), but SMTP uses host and port arguments, for consistency it's better havin separate source_ip and source_port arguments. As a side effect, it makes easier to specify only source_port. 

The submited patch includes tests and updated docstrings; 

At the smtplib.rst, for example:

SMTP(host='', port=0, local_hostname=None[, timeout])

Would be ok to change it like:

SMTP(host='', port=0, local_hostname=None[, timeout], source_ip='', source_port=0)
msg129112 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2011-02-22 19:12
We already have 3 places where a tuple is used:

socket.socket.bind
socket.create_connection
http.client.HTTPConnection

Changing this notation in smtplib for such a rarely used feature is not worth the effort, imo.
Also, I would not add two new SMTP attributes. Passing a tuple to socket.create_connection() is just fine. If you want to know the source address afterwards you can use socket.getsockname().

PS - I modified smtplib.py in meantime. Please update your local copy.
msg129150 - (view) Author: Paulo Scardine (paulos) Date: 2011-02-23 01:32
Ok, I changed to a single parameter matching socket.create_connection().

I made my best to update tests and docs, but I don't have a clue if it is right.
msg129152 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2011-02-23 01:49
Follow my comments:

> +                 source_address=('', 0)):

Make that default to None instead and pass it as-is to socket.create_connection().

> +        self.source_address = source_address

There's no need to store the source address as an instance attribute. 
Just pass it as-is to socket.create_connection() in __init__ and connect methods and then get rid of it.


I think source_address no longer makes sense in UNIX sockets / LMTP class. 
Or at least, this is what I get if I try to bind() a UNIX socket:

>>> import socket
>>> sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
>>> sock.bind(("", 0))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 1, in bind
TypeError: argument must be string or read-only character buffer, not tuple
>>>
msg129153 - (view) Author: Paulo Scardine (paulos) Date: 2011-02-23 02:58
> There's no need to store the source address as an instance attribute.
> Just pass it as-is to socket.create_connection() in __init__ and 
> connect methods and then get rid of it.

Ok, what if user initialize with something like smtplib.SMTP(source_address=('200.165.12.1', 25)) and calls connect without source_address?
msg129447 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2011-02-25 22:31
You are right, I haven't thought about that.
msg129496 - (view) Author: Paulo Scardine (paulos) Date: 2011-02-26 03:44
Giampaolo,

Thanks for your kind review, I will send a patch with suggested changes this weekend.

Grazie,
--
Paulo
msg129699 - (view) Author: Paulo Scardine (paulos) Date: 2011-02-28 13:24
Patch attached.
msg129701 - (view) Author: Paulo Scardine (paulos) Date: 2011-02-28 13:25
oops...
msg129703 - (view) Author: Paulo Scardine (paulos) Date: 2011-02-28 13:32
Sorry for the last post, I had the impression that I forgot to attach the patch. I'm slow this Monday.
msg139343 - (view) Author: Paulo Scardine (paulos) Date: 2011-06-28 05:07
Is there anything I should improve in order to get this patch commited?
msg141417 - (view) Author: Roundup Robot (python-dev) Date: 2011-07-30 02:58
New changeset 26839edf3cc1 by Senthil Kumaran in branch 'default':
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.
http://hg.python.org/cpython/rev/26839edf3cc1
History
Date User Action Args
2011-07-30 02:58:43python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg141417

resolution: remind -> fixed
stage: needs patch -> resolved
2011-06-28 05:07:12paulossetresolution: remind
messages: + msg139343
2011-02-28 13:32:09paulossetnosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129703
2011-02-28 13:25:14paulossetfiles: + smtp_lib_source_address.patch
nosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129701
2011-02-28 13:24:22paulossetfiles: + smtp_lib_source_address.patch
nosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129699
2011-02-26 03:44:14paulossetnosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129496
2011-02-25 22:31:33giampaolo.rodolasetnosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129447
2011-02-23 02:58:24paulossetnosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129153
2011-02-23 01:49:33giampaolo.rodolasetnosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129152
2011-02-23 01:33:03paulossetfiles: - patch.diff
nosy: giampaolo.rodola, r.david.murray, paulos
2011-02-23 01:32:33paulossetfiles: + smtp_lib_source_address.patch
nosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129150
2011-02-22 19:12:47giampaolo.rodolasetnosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129112
2011-02-22 18:33:39paulossetnosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129108
2011-02-22 17:37:18giampaolo.rodolasetnosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129106
2011-02-22 17:22:17paulossetnosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129103
2011-02-22 17:18:19paulossetnosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129101
2011-02-22 17:16:41giampaolo.rodolasetnosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129100
2011-02-22 17:12:22paulossetfiles: + patch.diff

messages: + msg129098
keywords: + patch
nosy: giampaolo.rodola, r.david.murray, paulos
2011-02-22 16:17:32giampaolo.rodolasetnosy: giampaolo.rodola, r.david.murray, paulos
messages: + msg129092
2011-02-22 15:36:43giampaolo.rodolasetnosy: + giampaolo.rodola
messages: + msg129085
2011-02-22 12:19:47r.david.murraysetversions: + Python 3.3, - Python 2.7
nosy: + r.david.murray

messages: + msg129059

type: enhancement
stage: needs patch
2011-02-22 04:10:54pauloscreate