• 
      

    Add typing to OutputWrapper and fix string mismatch issues.

    Review Request #13035 — Created May 10, 2023 and submitted

    Information

    RBTools
    release-5.x

    Reviewers

    OutputWrapper's design led to easy-to-introduce string mismatch
    issues, which could cause crashes or data corruption. This is because it
    was meant to be used with either a Unicode or byte string buffer, and
    calls to .write() failed to handle all cases of mismatches.

    This change fixes this all through a combination of type hints and
    smarter string conversions.

    OutputWrapper is now a generic, available in OutputWrapper[bytes] or
    OutputWrapper[str] form. The form used dictates the type of upstream
    buffer this can work with, and the types of string values allowd when
    writing.

    To protect against bad calls, we now use force_bytes() or
    force_unicode(), depending on the type. This happens automatically for
    all writes. This also allows us to simplify all the internal logic.

    Since OutputWrapper.write() defaults to using a newline as the end
    value, and we can't hard-code a string type here, we now use a wrapping
    object that symbolizes a newline. The prior hard-coded default is likely
    why the write() logic was the way it was before.

    Unit tests have been added for the class testing both bytes and str
    variations.

    Unit tests pass.

    Tested command output from a few different commands, utilizing both
    Unicode and byte streams. Didn't encounter any issues.

    Summary ID
    Add typing to OutputWrapper and fix string mismatch issues.
    `OutputWrapper`'s design led to easy-to-introduce string mismatch issues, which could cause crashes or data corruption. This is because it was meant to be used with either a Unicode or byte string buffer, and calls to `.write()` failed to handle all cases of mismatches. This change fixes this all through a combination of type hints and smarter string conversions. `OutputWrapper` is now a generic, available in `OutputWrapper[bytes]` or `OutputWrapper[str]` form. The form used dictates the type of upstream buffer this can work with, and the types of string values allowd when writing. To protect against bad calls, we now use `force_bytes()` or `force_unicode()`, depending on the type. This happens automatically for all writes. This also allows us to simplify all the internal logic. Since `OutputWrapper.write()` defaults to using a newline as the `end` value, and we can't hard-code a string type here, we now use a wrapping object that symbolizes a newline. The prior hard-coded default is likely why the `write()` logic was the way it was before. Unit tests have been added for the class testing both `bytes` and `str` variations.
    d4947a7ad9b4a4c582ac4d9569ec9a02581efc23
    david
    1. Ship It!
    2. 
        
    chipx86
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-5.x (0a38537)
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to master (0a38537)