Skip to content

Commit

Permalink
views: Don't munge the 'From' field of patches
Browse files Browse the repository at this point in the history
At the moment patchwork always uses the official submitter name (as
patchwork understands it) as the "From" for patches that you receive.
This isn't quite what users expect and has some unfortunate
consequences.

The biggest problem is that patchwork saves the "official" name for an
email address the first time it sees an email from them.  If that name
is wrong (or was missing) patchwork will be confused even if future
emails from this person are fixed.  There are similar problems if a
user changes his/her name (get married?).

It seems better to just have each patch report the actual "From" that
was used to send that patch.  We'll still return the submitter in
'X-Patchwork-Submitter' just in case someone wants it.

Reported-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Stephen Finucane <stephen@that.guru>
  • Loading branch information
dianders authored and stephenfin committed Dec 18, 2016
1 parent b6d3206 commit d365402
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 11 deletions.
34 changes: 25 additions & 9 deletions patchwork/tests/test_mboxviews.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,26 +82,30 @@ class MboxHeaderTest(TestCase):

"""Test the passthrough and generation of various headers."""

def test_header_passthrough_cc(self):
"""Validate passthrough of 'Cc' header."""
header = 'Cc: CC Person <cc@example.com>'
def _test_header_passthrough(self, header):
patch = create_patch(headers=header + '\n')
response = self.client.get(reverse('patch-mbox', args=[patch.id]))
self.assertContains(response, header)

def test_header_passthrough_cc(self):
"""Validate passthrough of 'Cc' header."""
header = 'Cc: CC Person <cc@example.com>'
self._test_header_passthrough(header)

def test_header_passthrough_to(self):
"""Validate passthrough of 'To' header."""
header = 'To: To Person <to@example.com>'
patch = create_patch(headers=header + '\n')
response = self.client.get(reverse('patch-mbox', args=[patch.id]))
self.assertContains(response, header)
self._test_header_passthrough(header)

def test_header_passthrough_date(self):
"""Validate passthrough of 'Date' header."""
header = 'Date: Fri, 7 Jun 2013 15:42:54 +1000'
patch = create_patch(headers=header + '\n')
response = self.client.get(reverse('patch-mbox', args=[patch.id]))
self.assertContains(response, header)
self._test_header_passthrough(header)

def test_header_passthrough_from(self):
"""Validate passthrough of 'From' header."""
header = 'From: John Doe <john@doe.com>'
self._test_header_passthrough(header)

def test_patchwork_id_header(self):
"""Validate inclusion of generated 'X-Patchwork-Id' header."""
Expand All @@ -116,6 +120,18 @@ def test_patchwork_delegate_header(self):
response = self.client.get(reverse('patch-mbox', args=[patch.id]))
self.assertContains(response, 'X-Patchwork-Delegate: %s' % user.email)

def test_patchwork_from_header(self):
"""Validate inclusion of generated 'X-Patchwork-From' header."""
email = 'jon@doe.com'
from_header = 'From: Jon Doe <%s>\n' % email

person = create_person(name='Jonathon Doe', email=email)
patch = create_patch(submitter=person, headers=from_header)
response = self.client.get(reverse('patch-mbox', args=[patch.id]))
self.assertContains(response, from_header)
self.assertContains(response, 'X-Patchwork-Submitter: %s <%s>' % (
person.name, email))

def test_from_header(self):
"""Validate non-ascii 'From' header.
Expand Down
4 changes: 2 additions & 2 deletions patchwork/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ def patch_to_mbox(patch):

mail = PatchMbox(body)
mail['Subject'] = patch.name
mail['From'] = email.utils.formataddr((
mail['X-Patchwork-Submitter'] = email.utils.formataddr((
str(Header(patch.submitter.name, mail.patch_charset)),
patch.submitter.email))
mail['X-Patchwork-Id'] = str(patch.id)
Expand All @@ -391,7 +391,7 @@ def patch_to_mbox(patch):
mail['Message-Id'] = patch.msgid
mail.set_unixfrom('From patchwork ' + patch.date.ctime())

copied_headers = ['To', 'Cc', 'Date']
copied_headers = ['To', 'Cc', 'Date', 'From']
orig_headers = HeaderParser().parsestr(str(patch.headers))
for header in copied_headers:
if header in orig_headers:
Expand Down

0 comments on commit d365402

Please sign in to comment.