emencia / emencia-django-newsletter

An app for sending newsletter by email to a contact list.
189 stars 72 forks source link

Avoid a ValueError in SMTPServer #25

Closed DNX closed 13 years ago

DNX commented 13 years ago

I propose to add a try block on the line 84 in newsletter/models.py

so, the code:

    @property
    def custom_headers(self):
    if self.headers:
        headers = {}
        for header in self.headers.split('\r\n'):
            if header:
                key, value = header.split(':')
                headers[key.strip()] = value.strip()
        return headers
    return {}

becomes:

    @property
    def custom_headers(self):
    if self.headers:
        headers = {}
        for header in self.headers.split('\r\n'):
            if header:
                try:
                    key, value = header.split(':')
                    headers[key.strip()] = value.strip()
                except ValueError:
                    pass
        return headers
    return {}

also it can be useful to validate custom_headers on saving SMTPServer.

Without this precautions, when we run send_newsletter with a wrong format of custom_headers we get:

denis@denis-linux:~/workspace/dnxproject$ python manage.py send_newsletter
Starting sending newsletters...
Start emailing New service
2 emails will be sent
- Processing First Email | first email tags
Traceback (most recent call last):
  File "manage.py", line 14, in <module>
    execute_manager(settings)
  File "/usr/local/lib/python2.6/dist-packages/django/core/management/__init__.py", line 438, in execute_manager
    utility.execute()
  File "/usr/local/lib/python2.6/dist-packages/django/core/management/__init__.py", line 379, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python2.6/dist-packages/django/core/management/base.py", line 191, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/usr/local/lib/python2.6/dist-packages/django/core/management/base.py", line 220, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python2.6/dist-packages/django/core/management/base.py", line 351, in handle
    return self.handle_noargs(**options)
  File "/home/denis/workspace/dnxproject/apps/emencia/django/newsletter/management/commands/send_newsletter.py", line 28, in handle_noargs
    mailer.run()
  File "/home/denis/workspace/dnxproject/apps/emencia/django/newsletter/mailer.py", line 72, in run
    message = self.build_message(contact)
  File "/home/denis/workspace/dnxproject/apps/emencia/django/newsletter/mailer.py", line 115, in build_message
    for header, value in self.newsletter.server.custom_headers.items():
  File "/home/denis/workspace/dnxproject/apps/emencia/django/newsletter/models.py", line 84, in custom_headers
    key, value = header.split(':')
ValueError: need more than 1 value to unpack

Thanks.

Fantomas42 commented 13 years ago

Hi denis,

thank you for the report.

Rather than implementing a Try...Except in the mailer, I prefer implement a custom validator of the format directly in the admin interface.

Regards.

DNX commented 13 years ago

Perfect, I totally agree with you. Thanks

Fantomas42 commented 13 years ago

Implemented in Fantomas42/emencia-django-newsletter@603b8f3729aef8e418bc2946c74d88acdef283b1