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: Closed (submitted)

Change Summary:

Pushed to release-5.x (0a38537)
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (0a38537)
Loading...