Bools often corrupt when written using Compact

[terwilliger42]

Looks like if you have a bool in your struct and you're writing using Compact format, it's very likely the resulting message will be corrupt. See attached testcase.

This occurs due to this piece of code -- the "else" clause does not write out the field ID.

    def write_bool(self, bool):
        if self._bool_fid and self._bool_fid > self._last_fid \
                and self._bool_fid - self._last_fid <= 15:
            if bool:
                ctype = CompactType.TRUE
            else:
                ctype = CompactType.FALSE
            self._write_field_header(ctype, self._bool_fid)
        else:
            if bool:
                self.write_byte(CompactType.TRUE)
            else:
                self.write_byte(CompactType.FALSE)

In a practical sense, this may occur if the last field ID written out is more than 15 IDs away, but consider this:

1. Fields are not necessarily written out in ID order (maybe that's a separate bug?)
2. If the previous fields were default values, then they are not written out at all -- thus do not contribute to the ID closeness required to not trigger the else clause.

corrupt_bool_testcase.txt

[ethe]

Modify it to 1: bool some_bool will fix it, I think it's not a bug

[terwilliger42]

@ethe: In our case we have many thrift messages which all need to share a some common set of fields -- we give those IDs 1, 2, 3, 4, 5 etc... and then require the fields which are not common to start at 20, so that there aren't conflicts.

AFAIK the thrift spec doesn't say that you can't skip IDs. If I change the type to an int16 or any other type other than bool, it will work just fine. If this is not allowed, an exception should be raised rather than causing corrupt messages.

We use thriftpy, I appreciate all of the work that has been done thus far and am glad to see it will get a little more attention going forward.