Extended headers in git diffs break patch parsing

Bug #1510337 reported by Colin Watson on 2015-10-27
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Launchpad itself
Critical
Colin Watson

Bug Description

https://oops.canonical.com/?oopsid=OOPS-6db37bf38de8f54f864dceb1fb48de26 happens when trying to send inline review comment mail on certain git-based merge proposals.

  MalformedPatchHeader: Malformed patch header. No orig name
'new file mode 100644\n'

    Traceback (most recent call last):
  Module lazr.jobrunner.jobrunner, line 194, in runJobHandleError
    self.runJob(job, fallback)
  Module lp.services.job.runner, line 302, in runJob
    super(BaseJobRunner, self).runJob(IRunnableJob(job), fallback)
  Module lazr.jobrunner.jobrunner, line 162, in runJob
    job.run()
  Module lp.code.model.branchmergeproposaljob, line 399, in run
    mailer = CodeReviewCommentMailer.forCreation(self.code_review_comment)
  Module lp.code.mail.codereviewcomment, line 87, in forCreation
    code_review_comment.message.rfc822msgid)
  Module lp.code.mail.codereviewcomment, line 77, in __init__
    self._generateBodyBits()
  Module lp.code.mail.codereviewcomment, line 113, in _generateBodyBits
    inline_comment.comments, inline_comment.previewdiff.text)
  Module lp.code.mail.codereviewcomment, line 192, in build_inline_comments_section
    diff_lines, allow_dirty=True, keep_dirty=True)
  Module lp.code.mail.patches, line 445, in parse_patches
    patch_lines['saved_lines'], allow_dirty),
  Module lp.code.mail.patches, line 334, in parse_patch
    (orig_name, mod_name) = get_patch_names(iter_lines)
  Module lp.code.mail.patches, line 45, in get_patch_names
    raise MalformedPatchHeader("No orig name", line)
MalformedPatchHeader: Malformed patch header. No orig name
'new file mode 100644\n'

The relevant bit of the patch looks something like:

  diff --git a/foo b/foo
  new file mode 100644

Anything with an extended header line that doesn't start with "index " will do this. git documents quite a few of these, in Documentation/diff-generate-patch.txt. I think we should ignore all such after "diff --git".

Related branches

Launchpad QA Bot (lpqabot) wrote :
tags: added: qa-needstesting
Changed in launchpad:
status: In Progress → Fix Committed
Colin Watson (cjwatson) on 2015-10-28
tags: added: qa-ok
removed: qa-needstesting
Colin Watson (cjwatson) on 2015-10-28
Changed in launchpad:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers