[merge][#109613] don't set bugs property unless bugs are specified

[John Arbash Meinel]

John Arbash Meinel has voted +1.
Status is now: Approved
Comment:
Well technically it is "bundle reader correctly handles empty 
properties." Not bundles :).

I did look over the bundle code, and the code seems to do:
(bzrlib.bundle.bundle_data.RevisionInfo.as_revision)
             for property in self.properties:
                 key_end = property.find(': ')
                 assert key_end is not None
                 key = property[:key_end].encode('utf-8')
                 value = property[key_end+2:].encode('utf-8')
                 rev.properties[key] = value

Which looks correct.

And the writing code 
(bzrlib.bundle.serializer.v08.BundleSerializerV08._write_revision) seems 
to do a correct thing, too:
         if rev.properties:
             self._write('properties', None, indent=1)
             for name, value in rev.properties.items():
                 self._write(name, value, indent=3)

Now, self._write is writing:
if not value:
   f.write(':\n')
...

ah, so maybe that is it. The *reading* code is doing:
                 key_end = property.find(': ')
                 assert key_end is not None
But there is no ': ' because we have ':\n'. And the assert isn't 
trapping what it thinks it is, because key_end would be '-1', not None.

And that ends up generating

key = property[:-1].encode('utf-8') # Which should accidentally work
value = property[-1+2:] # Which is something like 'ugs:'

So yes, there is a bug for empty properties in the old serializer.


For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3Ce01316480704242256s5f4ef739u2c47ea9dc6d7a8c6%40mail.gmail.com%3E