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

Thrift C++ runtime uses assert to prevent overflows, checks sanity only in debug builds

Attach filesAttach ScreenshotAdd voteVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Open
    • Major
    • Resolution: Unresolved
    • 0.10.0
    • None
    • C++ - Library
    • All

    Description

      Currently there is widespread use of assert in the thrift C++ runtime library. Some of the more disturbing cases are security related, for example checking header sizes. I recommend we eliminate assertions that are only checked in debug mode, and instead throw the appropriate exception, usually a TTransportException with CORRUPTED_DATA as the reason. If we're going to check for an overflow or a buffer overrun, we should do so in debug and release modes. Further, assertions are not easily tested whereas exceptions are.

      In THRIFT-3873 apache::thrift::transport::safe_numeric_cast was added, so I also suggest changing static_cast to safe_numeric_cast where appropriate throughout the transport code to catch any overflow errors.

      Another location where assert is used liberally is inside the posix Mutex implementation.

      Attachments

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            jking3 James E. King III
            jking3 James E. King III

            Dates

              Created:
              Updated:

              Slack

                Issue deployment