Fix crash in Python API async_get_contents when terminal contains inline images #549
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes a crash that occurs when using the Python API's
async_get_contents()method to retrieve terminal content that includes inline images (e.g., displayed viaimgcat).Root Cause
In
protoStyleForCharacter:externalAttributes:, the code processesforegroundColorModeandbackgroundColorModeto set protobuf enum values. However, for image cells (c.image == 1), theforegroundColorandbackgroundColorfields store X/Y image coordinates, not actual colors.When an image cell happens to have color mode bits that match
ColorModeAlternate, and the coordinate values don't correspond to validITMAlternateColorenum values, the protobuf library throws an exception during enum validation inGPBSetEnumIvarWithFieldPrivate.Crash Stack Trace
GPBSetEnumIvarWithFieldPrivate + 116
-[PTYSession protoStyleForCharacter:externalAttributes:] + 280
-[PTYSession stringForLine:length:eaIndex:cppsArray:stylesArray:] + 336
__37-[PTYSession handleGetBufferRequest:]_block_invoke + 172
Fix
Wrap the foreground and background color mode processing in
if (!c.image)to skip color interpretation for image cells, since their color fields have different semantics.Also fixed a pre-existing copy-paste bug where
c.foregroundColorwas incorrectly used instead ofc.backgroundColorwhen checking the background color alternate mode.Test Plan
imgcatsession.async_get_contents()on lines containing the image