4572: X-ReviewBoard-Diff-For headers are incomplete

ryan.beasley

What version are you running?

2.0.20

What's the URL of the page containing the problem?

n/a

What steps will reproduce the problem?

  1. Post a review where the diff includes plain edits to an existing file (i.e. does not involve creating, copying, or moving).
  2. Upon receipt of generated e-mail, examine X-ReviewBoard-Diff-For headers.
  3. Observe that X-ReviewBoard-Diff-For headers appear only for files named in the diffset which were created/moved. Edited files are not named.

What is the expected output? What do you see instead?

  • I expect X-ReviewBoard-Diff-For headers to name every file (up to the 8K limit) edited in a diffset.

What operating system are you using? What browser?

  • n/a

Please provide any additional information below.

If I'm reading the code correctly, this appears to be a bug from the original implementation which continues to the present. Any filediff for which none of filediff.{deleted,copied,moved,is_new} applies fails to generate an X-ReviewBoard-Diff-For header.

From release-2.0.20 (reviewboard/notifications/email.py):

601     if latest_diffset:
602         modified_files = set()
603
604         for filediff in latest_diffset.files.all():
605             if filediff.deleted or filediff.copied or filediff.moved:
606                 modified_files.add(filediff.source_file)
607
608             if filediff.is_new or filediff.copied or filediff.moved:
609                 modified_files.add(filediff.dest_file)
610
611         for filename in modified_files:
612             headers.appendlist('X-ReviewBoard-Diff-For', filename)

And from HEAD (reviewbaord/notifications/email/message.py):

181
182     if latest_diffset:
183         modified_files = set()
184
185         for filediff in latest_diffset.files.all():
186             if filediff.deleted or filediff.copied or filediff.moved:
187                 modified_files.add(filediff.source_file)
188
189             if filediff.is_new or filediff.copied or filediff.moved:
190                 modified_files.add(filediff.dest_file)
191
192         # The following code segment deals with the case where the client adds
193         # a significant amount of files with large names. We limit the number
194         # of headers; when more than 8192 characters are reached, we stop
195         # adding filename headers.
196         current_header_length = 0
197
198         for filename in modified_files:
...
212             headers.appendlist('X-ReviewBoard-Diff-For', filename)
david
#1 david
  • +EasyFix
#2 hmarceau

Claiming for easy fix

david
#3 david

Fixed in release-2.5.x (a25b589). This will ship in 2.5.17. Thanks!

  • -New
    +Fixed