diff --git a/CHANGES b/CHANGES index 7a691b0cc9..40e5cf5d29 100644 --- a/CHANGES +++ b/CHANGES @@ -45,6 +45,33 @@ $ uvx --from 'libtmux' --prerelease allow python _Notes on the upcoming release will go here._ +### Fixes + +#### A newline in a format value no longer breaks every object listing + +A pane whose `pane_current_path` contained a newline — a directory whose +name has one — made `Server.panes` and `Server.windows` raise +`ValueError: zip() argument 2 is shorter than argument 1` for the +*entire* server, healthy panes included. `fetch_objs` iterated stdout +one line per object, so a value containing a newline split its record +across two lines and each fragment reached `parse_output` with too few +values for its strict `zip`. + +Because every pane row carries `pane_current_path`, and every +pane-targeting lookup enumerates panes, one directory took out +resolution for all of them. Which calls broke also depended on which +pane happened to be active, since session and window rows resolve +`pane_*` against the active pane — so the same server appeared to work +or fail as the user switched panes. + +Records are now regrouped on the field separator instead of on +newlines. The `-F` template terminates every field with a separator, so +a record holds exactly as many separators as it has fields and a +newline is never one of them; a value may now contain any number of +newlines in any position. A value that carries the separator itself no +longer corrupts the parse silently — it is reported as output that +could not be parsed. + ### Documentation #### Cleaner `from_env` examples (#719) diff --git a/src/libtmux/neo.py b/src/libtmux/neo.py index 98ece86fa5..a74c871fa8 100644 --- a/src/libtmux/neo.py +++ b/src/libtmux/neo.py @@ -1036,6 +1036,58 @@ def parse_output( return {k: v for k, v in formatter.items() if v} +def _split_records(stdout: list[str], field_count: int) -> list[str]: + """Regroup ``-F`` output into one string per object. + + tmux writes one record per line, but any format value may itself + contain a newline -- ``pane_current_path`` for a directory whose + name has one -- and that splits the record across output lines. + Iterating lines then hands :func:`parse_output` a fragment with too + few values, which its strict ``zip`` rejects, so one directory + breaks every object on the server rather than the one pane in it. + + Regrouping on the separator is exact rather than merely better: the + template from :func:`get_output_format` terminates *every* field + with a separator, so one record holds exactly ``field_count`` of + them and a newline is never one. Nothing is split on newlines, so a + value may contain any number of them, in any position. + + Raises + ------ + :exc:`~libtmux.exc.LibTmuxException` + If the values do not divide into whole records, which means a + value contained the separator itself. + """ + blob = "\n".join(stdout) + if not blob: + return [] + + values = blob.split(FORMAT_SEPARATOR) + # Every record ends with a separator, so the split always leaves one + # trailing empty for the final record. + if values and values[-1] == "": + values.pop() + + if field_count <= 0 or len(values) % field_count: + msg = ( + f"tmux output could not be parsed: {len(values)} values for " + f"{field_count} fields per record. A format value probably " + f"contains the field separator ({FORMAT_SEPARATOR!r})." + ) + raise exc.LibTmuxException(msg) + + records: list[str] = [] + for start in range(0, len(values), field_count): + chunk = values[start : start + field_count] + # The newline that terminated the previous record survives the + # join glued to this record's first value. It is a delimiter, + # not data. + if start and chunk[0].startswith("\n"): + chunk[0] = chunk[0][1:] + records.append(FORMAT_SEPARATOR.join(chunk) + FORMAT_SEPARATOR) + return records + + def fetch_objs( server: Server, list_cmd: ListCmd, @@ -1137,7 +1189,10 @@ def fetch_objs( raise_if_stderr(proc, list_cmd) - outputs = [parse_output(line, list_cmd, tmux_version) for line in proc.stdout] + outputs = [ + parse_output(record, list_cmd, tmux_version) + for record in _split_records(proc.stdout, len(_fields)) + ] if logger.isEnabledFor(logging.DEBUG): if cmd_str is None: diff --git a/tests/test_neo.py b/tests/test_neo.py index f67215e57b..e3a25bc7b4 100644 --- a/tests/test_neo.py +++ b/tests/test_neo.py @@ -13,12 +13,15 @@ import pytest +from libtmux import exc +from libtmux.formats import FORMAT_SEPARATOR from libtmux.neo import ( _CONTEXT_ONLY_TOKENS, FIELD_VERSION, SCOPES_BY_LIST_CMD, Obj, _is_target_not_found_error, + _split_records, _token_scope, get_output_format, ) @@ -258,3 +261,75 @@ def test_every_obj_field_classifies_to_known_scope() -> None: "(add them to _SCOPE_OVERRIDES, _SCOPE_PREFIXES, " f"_UNIVERSAL_TOKENS, or _CONTEXT_ONLY_TOKENS): {unclassified}" ) + + +class SplitRecordsFixture(t.NamedTuple): + """Test fixture for :func:`_split_records`.""" + + test_id: str + values: list[list[str]] + + +SPLIT_RECORDS_FIXTURES: list[SplitRecordsFixture] = [ + SplitRecordsFixture("single_clean_record", [["a", "b", "c"]]), + SplitRecordsFixture("two_clean_records", [["a", "b", "c"], ["d", "e", "f"]]), + SplitRecordsFixture("newline_in_first_field", [["a\nx", "b", "c"]]), + SplitRecordsFixture("newline_in_middle_field", [["a", "b\nx", "c"]]), + SplitRecordsFixture("newline_in_last_field", [["a", "b", "c\nx"]]), + SplitRecordsFixture("consecutive_newlines", [["a", "b\n\n\nx", "c"]]), + SplitRecordsFixture( + "poisoned_record_between_clean_ones", + [["a", "b", "c"], ["d", "e\npath", "f"], ["g", "h", "i"]], + ), + SplitRecordsFixture("empty_values", [["", "", ""]]), +] + + +@pytest.mark.parametrize( + SplitRecordsFixture._fields, + SPLIT_RECORDS_FIXTURES, + ids=[fixture.test_id for fixture in SPLIT_RECORDS_FIXTURES], +) +def test_split_records_round_trips_newlines( + test_id: str, + values: list[list[str]], +) -> None: + """A newline inside a value must not split its record. + + tmux emits one record per line, so a value containing a newline -- + ``pane_current_path`` under a directory whose name has one -- used + to arrive as two short fragments and fail ``parse_output``'s strict + ``zip``. Because every pane row carries ``pane_current_path``, that + broke enumeration for the whole server, not just the one pane. + """ + assert test_id + field_count = len(values[0]) + # Rebuild exactly what tmux writes: each record's fields, every one + # terminated by the separator, and records terminated by newlines. + stdout_text = "".join( + "".join(f"{value}{FORMAT_SEPARATOR}" for value in record) + "\n" + for record in values + ) + stdout = stdout_text.split("\n") + while stdout and stdout[-1] == "": + stdout.pop() + + records = _split_records(stdout, field_count) + + assert len(records) == len(values) + for record, expected in zip(records, values, strict=True): + parsed = record.split(FORMAT_SEPARATOR)[:-1] + assert parsed == expected + + +def test_split_records_reports_a_forged_separator() -> None: + """A value carrying the separator is named, not a ``zip`` message.""" + stdout = [f"a{FORMAT_SEPARATOR}b{FORMAT_SEPARATOR}c{FORMAT_SEPARATOR}"] + + with pytest.raises(exc.LibTmuxException, match="could not be parsed"): + _split_records(stdout, 2) + + +def test_split_records_handles_no_objects() -> None: + """An empty listing yields no records rather than a bogus one.""" + assert _split_records([], 5) == []