fix(gp2rs): write arrangement XML as UTF-8 so GP import survives non-ASCII metadata (#984)

The GP→arrangement-XML writers persisted their output with
`Path.write_text(xml_str)` and no explicit encoding. On Windows that uses
the cp1252 default, so a non-ASCII metadata character — e.g. the © in an
album name like "Chrysalis©1982" — was written as the lone byte 0xA9.
The XML is read back as UTF-8 (expat's default), where 0xA9 is an invalid
start byte, so `parse_arrangement` died with:

    xml.etree.ElementTree.ParseError: not well-formed (invalid token): line 10, column 22

and the whole Guitar Pro import failed (HTTP 500). All three arrangement
XML writes (gp2rs.py, gp2rs_gpx.py ×2) now pin encoding="utf-8".

CI runs on Linux (UTF-8 default) so the bug was invisible there; the new
test pins the locale-independent contract at the source level plus a
round-trip of a © album name.


Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q

Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
ChrisBeWithYou
2026-07-19 00:56:32 -05:00
committed by GitHub
co-authored by Claude Fable 5
parent 1712803dc7
commit 32ed564006
4 changed files with 63 additions and 3 deletions
+7
View File
@@ -229,6 +229,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
engine (`app.js`, `highway.js`, `playSong`, `showScreen`, the capability registry). engine (`app.js`, `highway.js`, `playSong`, `showScreen`, the capability registry).
### Fixed ### Fixed
- **Guitar Pro import no longer fails on non-ASCII song metadata (Windows).**
The GP→arrangement-XML writers wrote their output with `Path.write_text()`
and no explicit encoding, so on Windows (cp1252 default) a metadata
character like the © in an album name ("Chrysalis©1982") was written as a
lone `0xA9` byte — invalid UTF-8 — and import died with
`not well-formed (invalid token): line N, column 22`. All three arrangement
XML writes now pin `encoding="utf-8"`.
- **3D Highway: the lane stops at the hit line** (#991) — the highway lane, its - **3D Highway: the lane stops at the hit line** (#991) — the highway lane, its
dividers, and the fret boundary extension lines ran `BEHIND` seconds *past* the dividers, and the fret boundary extension lines ran `BEHIND` seconds *past* the
hit line toward the player. Nothing is ever drawn in that strip (notes and chord hit line toward the player. Nothing is ever drawn in that strip (notes and chord
+1 -1
View File
@@ -2080,7 +2080,7 @@ def convert_file(
safe_name = track.name.strip().replace(" ", "_").replace("/", "_") safe_name = track.name.strip().replace(" ", "_").replace("/", "_")
filename = f"{safe_name}_{arr_name or 'arr'}.xml" filename = f"{safe_name}_{arr_name or 'arr'}.xml"
filepath = out / filename filepath = out / filename
filepath.write_text(xml_str) filepath.write_text(xml_str, encoding="utf-8")
output_files.append(str(filepath)) output_files.append(str(filepath))
return output_files return output_files
+2 -2
View File
@@ -1680,7 +1680,7 @@ def convert_file(
filepath = safe_join(out, filename) filepath = safe_join(out, filename)
if filepath is None: if filepath is None:
raise ValueError(f"unsafe output filename from track name: {track['name']!r}") raise ValueError(f"unsafe output filename from track name: {track['name']!r}")
filepath.write_text(xml_str) filepath.write_text(xml_str, encoding="utf-8")
output_files.append(str(filepath)) output_files.append(str(filepath))
continue continue
@@ -2108,7 +2108,7 @@ def convert_file(
filepath = safe_join(out, filename) filepath = safe_join(out, filename)
if filepath is None: if filepath is None:
raise ValueError(f"unsafe output filename from track name: {track['name']!r}") raise ValueError(f"unsafe output filename from track name: {track['name']!r}")
filepath.write_text(xml_str) filepath.write_text(xml_str, encoding="utf-8")
output_files.append(str(filepath)) output_files.append(str(filepath))
# Keys/piano tracks additionally get a standard-notation sidecar # Keys/piano tracks additionally get a standard-notation sidecar
+53
View File
@@ -0,0 +1,53 @@
"""Regression: the GP→arrangement-XML writers must pin UTF-8.
A bare ``Path.write_text(xml_str)`` uses the platform's *default* text
encoding. On Windows that is cp1252, which encodes a non-ASCII metadata
character e.g. the © in an album name like "Chrysalis©1982" as the lone
byte 0xA9. The XML is then read back as UTF-8 (expat's default), where 0xA9
is an invalid start byte, so parsing dies with
not well-formed (invalid token): line N, column 22
CI runs on Linux (UTF-8 default), so the bug is invisible there and a plain
functional test would pass on the old code too. These assertions instead pin
the locale-independent contract directly.
"""
import inspect
import re
import xml.etree.ElementTree as ET
import gp2rs
import gp2rs_gpx
def test_arrangement_xml_writes_specify_utf8():
# Every write of the arrangement XML string must pass encoding="utf-8"
# so non-ASCII metadata survives regardless of the host locale.
for mod in (gp2rs, gp2rs_gpx):
src = inspect.getsource(mod)
bare = re.findall(r"\.write_text\(\s*xml_str\s*\)", src)
assert not bare, (
f"{mod.__name__}: XML write must pass encoding=\"utf-8\" — a bare "
f"write_text() uses the platform default (cp1252 on Windows) and "
f"mangles non-ASCII metadata into invalid UTF-8"
)
assert 'write_text(xml_str, encoding="utf-8")' in src, (
f"{mod.__name__}: expected a UTF-8-pinned arrangement XML write"
)
def test_utf8_write_round_trips_non_ascii_album():
# The behavioural end of the contract: a © album name written as UTF-8
# parses cleanly and reads back intact (the cp1252 write does not).
from pathlib import Path
import tempfile
xml_str = (
'<?xml version="1.0"?>\n<song>\n'
" <albumName>Chrysalis©1982</albumName>\n</song>\n"
)
path = Path(tempfile.mkdtemp()) / "arr.xml"
path.write_text(xml_str, encoding="utf-8")
root = ET.parse(path).getroot()
assert root.findtext("albumName") == "Chrysalis©1982"