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/ (+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:

Description of the change

Earlier The Mass Subscription also allowed some invalid email addresses such as ..<email address hidden>, 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
1=== modified file 'src/postorius/views/'
2--- src/postorius/views/ 2015-02-09 14:35:44 +0000
3+++ src/postorius/views/ 2015-04-07 23:12:00 +0000
4@@ -24,6 +24,8 @@
5 from django.core.urlresolvers import reverse
6 from django.shortcuts import render_to_response, redirect
7 from django.template import RequestContext
8+from django.core.validators import validate_email
9+from django.core.exceptions import ValidationError
10 from django.utils.decorators import method_decorator
11 from django.utils.translation import gettext as _
12 from urllib2 import HTTPError
13@@ -249,25 +251,22 @@
14 else:
15 emails = request.POST["emails"].splitlines()
16 for email in emails:
17- parts = email.split('@')
18- if len(parts) != 2 or '.' not in parts[1]:
19+ try:
20+ validate_email(email)
21+ self.mailing_list.subscribe(address=email)
22+ messages.success(request,
23+ 'The address %s has been subscribed to %s.' %
24+ (email, self.mailing_list.fqdn_listname))
25+ except MailmanApiError:
26+ return utils.render_api_error(request)
27+ except HTTPError, e:
28+ messages.error(request, e)
29+ except ValidationError:
30 messages.error(request,
31 'The email address %s is not valid.' %
32 email)
33- else:
34- try:
35- self.mailing_list.subscribe(address=email)
36- messages.success(
37- request,
38- 'The address %s has been subscribed to %s.' %
39- (email, self.mailing_list.fqdn_listname))
40- except MailmanApiError:
41- return utils.render_api_error(request)
42- except HTTPError, e:
43- messages.error(request, e)
44 return redirect('mass_subscribe', self.mailing_list.list_id)
47 def _get_choosable_domains(request):
48 try:
49 domains = Domain.objects.all()


People subscribed via source and target branches