Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-4405

Incorrect handling of sequence numbers that wrap to negative

    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 0.11.0
    • 0.13.0
    • Test Suite
    • None
    • docker ubuntu-xenial

    Description

      The following tests were added:

      • The cpp client for cross test did not use sequence numbers, so I added a testing protocol layer for TestClient on top of all protocols that uses a sequence ID that starts at INT32_MAX - 10, and advances until it wraps around. This caught a number of negative value handling issues.
      • The cpp client verifies the sequence ID that returns matches what was sent.
      • The python client was changed to use pedantic sequence ID checking on all protocols.

      The following errors were identified and fixed:

      • In c_glib, thrift_stored_message_protocol was limiting the sequence ID to [0..G_INTMAX]. This was changed to allow any 32-bit value, matching other implementations.
      • In cpp, JSON protocol, when the server read the header, it used a uint64_t for processing; this interacted with a bugfix from 2017 (THRIFT-4138) that dropped boost::lexical_cast and switched to std::stringstream, and this corrupted the negative sequence ID.
      • In python, compact protocol, a negative sequence ID was not handled properly because it is read in and written out as a "var int" which is always positive.

      Documentation was added for sequence number handling.

      Attachments

        Issue Links

          Activity

            People

              jking3 James E. King III
              jking3 James E. King III
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 20m
                  20m