Merge lp:~bhavesh-goyal093/postorius/fixed-invalid-email-acceptance into lp:postorius

Proposed by Bhavesh Goyal
Status: Merged
Merged at revision: 216
Proposed branch: lp:~bhavesh-goyal093/postorius/fixed-invalid-email-acceptance
Merge into: lp:postorius
Diff against target: 49 lines (+13/-14)
1 file modified
src/postorius/views/list.py (+13/-14)
To merge this branch: bzr merge lp:~bhavesh-goyal093/postorius/fixed-invalid-email-acceptance
Reviewer Review Type Date Requested Status
Terri Approve
Review via email: mp+255447@code.launchpad.net

Description of the change

Earlier The Mass Subscription also allowed some invalid email addresses such as ..<email address hidden>, ...@gmail.com to get subscribed to a mailing list, which could have led to Server Errors in long run while sending mass mails.

Now The Emails are successfully validated using Django EmailValidators and all the invalid emails including the ones ignored earlier are now rejected by throwing a Email Validation Error.

To post a comment you must log in.
Revision history for this message
Terri (terriko) wrote :

I *think* that invalid email addresses are probably rejected by Mailman Core anyhow, but this should let us provide better error messages. I'm running some tests against the code now in preparation for merging.

If someone could set up a few tox tests for this so it'll be tested going forwards, that would be most excellent.

Revision history for this message
Terri (terriko) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/postorius/views/list.py'
--- src/postorius/views/list.py 2015-02-09 14:35:44 +0000
+++ src/postorius/views/list.py 2015-04-07 23:12:00 +0000
@@ -24,6 +24,8 @@
24from django.core.urlresolvers import reverse24from django.core.urlresolvers import reverse
25from django.shortcuts import render_to_response, redirect25from django.shortcuts import render_to_response, redirect
26from django.template import RequestContext26from django.template import RequestContext
27from django.core.validators import validate_email
28from django.core.exceptions import ValidationError
27from django.utils.decorators import method_decorator29from django.utils.decorators import method_decorator
28from django.utils.translation import gettext as _30from django.utils.translation import gettext as _
29from urllib2 import HTTPError31from urllib2 import HTTPError
@@ -249,25 +251,22 @@
249 else:251 else:
250 emails = request.POST["emails"].splitlines()252 emails = request.POST["emails"].splitlines()
251 for email in emails:253 for email in emails:
252 parts = email.split('@')254 try:
253 if len(parts) != 2 or '.' not in parts[1]:255 validate_email(email)
256 self.mailing_list.subscribe(address=email)
257 messages.success(request,
258 'The address %s has been subscribed to %s.' %
259 (email, self.mailing_list.fqdn_listname))
260 except MailmanApiError:
261 return utils.render_api_error(request)
262 except HTTPError, e:
263 messages.error(request, e)
264 except ValidationError:
254 messages.error(request,265 messages.error(request,
255 'The email address %s is not valid.' %266 'The email address %s is not valid.' %
256 email)267 email)
257 else:
258 try:
259 self.mailing_list.subscribe(address=email)
260 messages.success(
261 request,
262 'The address %s has been subscribed to %s.' %
263 (email, self.mailing_list.fqdn_listname))
264 except MailmanApiError:
265 return utils.render_api_error(request)
266 except HTTPError, e:
267 messages.error(request, e)
268 return redirect('mass_subscribe', self.mailing_list.list_id)268 return redirect('mass_subscribe', self.mailing_list.list_id)
269269
270
271def _get_choosable_domains(request):270def _get_choosable_domains(request):
272 try:271 try:
273 domains = Domain.objects.all()272 domains = Domain.objects.all()

Subscribers

People subscribed via source and target branches