Skip to content

Fix convert_osc corrupting text with multiple OSC sequences - #426

Open
gaoflow wants to merge 1 commit into
tartley:masterfrom
gaoflow:fix/convert-osc-multiple-sequences
Open

Fix convert_osc corrupting text with multiple OSC sequences#426
gaoflow wants to merge 1 commit into
tartley:masterfrom
gaoflow:fix/convert-osc-multiple-sequences

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 24, 2026

Copy link
Copy Markdown

Problem

convert_osc mangles text when the input contains more than one OSC sequence.
The loop uses re.finditer on the original string, but then mutates text
on every iteration. After the first removal the second match's start/end
offsets are stale — they point into the already-shortened string — so the slice
lands in the wrong place and leaves behind a partial OSC escape or drops
trailing content.

Minimal reproducer:

from io import StringIO
from unittest.mock import patch
from colorama.ansitowin32 import AnsiToWin32

with patch('colorama.ansitowin32.winterm', None):
    stream = AnsiToWin32(StringIO(), strip=True, convert=False)
    text = 'Hello\033]0;First\aWorld\033]2;Second\aEnd'
    print(repr(stream.convert_osc(text)))
# Before fix: 'HelloWorld\x1b]0;Second'   (garbage)
# After fix:  'HelloWorldEnd'

The same code path also raises AttributeError: 'NoneType' object has no attribute 'set_title' on non-Windows when winterm is None, as reported in
#407.

Fix

Separate the two concerns:

  1. Side effects (calling winterm.set_title) — iterate all matches first,
    guarded by self.convert and winterm so non-Windows never touches winterm.
  2. Text stripping — use re.sub('', text) once, which correctly removes
    all matches in a single pass.

Also tighten params[0] in '02' (substring test, matches '' and '02')
to params[0] in ('0', '2') (tuple membership).

Tests

Two new tests added:

  • test_osc_multiple_sequences_stripped_correctly — asserts both OSC sequences
    are removed and surrounding text is preserved.
  • test_osc_strips_without_crash_when_winterm_none — asserts no crash when
    winterm is None (non-Windows with strip=True, convert=False).

All 40 tests pass (14 skipped, Windows-only).

The finditer loop mutated 'text' in-place on each iteration, but the
match positions came from the original string. After removing the first
OSC sequence the second match's start/end offsets pointed into the
already-shortened string, so slicing left garbage behind.

Fix: separate side effects (title calls) from text stripping; use
re.sub to remove all OSC sequences in one pass, which guarantees
correct results regardless of how many sequences appear.

Also guard the winterm.set_title() call with 'self.convert and winterm'
so that convert_osc no longer raises AttributeError on non-Windows
platforms where winterm is None, and tighten the params[0] check
from substring ('02') to tuple membership (('0', '2')).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant