smtplib: add ability to bind to specific source IP address/port

[paulos]

BPO	11281
Nosy	@giampaolo, @bitdancer
Files	smtp_lib_source_address.patch: add source_address arg to smtplib.xSMTP smtp_lib_source_address.patch: Cleaned up patch smtp_lib_source_address.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2011-07-30.02:58:43.538>
created_at = <Date 2011-02-22.04:10:54.767>
labels = ['type-feature', 'library']
title = 'smtplib: add ability to bind to specific source IP address/port'
updated_at = <Date 2011-07-30.02:58:43.536>
user = 'https://bugs.python.org/paulos'

bugs.python.org fields:

activity = <Date 2011-07-30.02:58:43.536>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2011-07-30.02:58:43.538>
closer = 'python-dev'
components = ['Library (Lib)']
creation = <Date 2011-02-22.04:10:54.767>
creator = 'paulos'
dependencies = []
files = ['20854', '20944', '20945']
hgrepos = []
issue_num = 11281
keywords = ['patch']
message_count = 21.0
messages = ['129035', '129059', '129085', '129092', '129098', '129100', '129101', '129103', '129106', '129108', '129112', '129150', '129152', '129153', '129447', '129496', '129699', '129701', '129703', '139343', '141417']
nosy_count = 4.0
nosy_names = ['giampaolo.rodola', 'r.david.murray', 'paulos', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue11281'
versions = ['Python 3.3']

[paulos]

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?

[bitdancer]

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.

[giampaolo]

+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.

[giampaolo]

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.

[paulos]

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?

[giampaolo]

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)

[paulos]

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.

[paulos]

@giampaolo: should I change the text in Doc/library/smtplib.rst or it is infered from the docstrings?

[giampaolo]

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

[paulos]

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)

[giampaolo]

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.

[paulos]

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.

[giampaolo]

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
>>>

[paulos]

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?

[giampaolo]

You are right, I haven't thought about that.

[paulos]

Giampaolo,

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

Grazie,

Paulo

[paulos]

Patch attached.

[paulos]

oops...

[paulos]

Sorry for the last post, I had the impression that I forgot to attach the patch. I'm slow this Monday.

[paulos]

Is there anything I should improve in order to get this patch commited?

[python-dev]

New changeset 26839edf3cc1 by Senthil Kumaran in branch 'default':
Fix closes bpo-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.
http://hg.python.org/cpython/rev/26839edf3cc1