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

Incorrect handling of sequence numbers that wrap to negative

    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.11.0
    • Fix Version/s: 0.13.0
    • Component/s: Test Suite
    • Labels:
      None
    • Environment:

      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

              • Assignee:
                jking3 James E. King III
                Reporter:
                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