diff --git a/mapillary_tools/geotag/utils.py b/mapillary_tools/geotag/utils.py index a1578934..a85d5101 100644 --- a/mapillary_tools/geotag/utils.py +++ b/mapillary_tools/geotag/utils.py @@ -71,8 +71,15 @@ def read_gpx_tracks(gpx_file: Path) -> list[Track]: try: # Parsed from the file opened in text mode, which gpxpy reads # whole itself, so that no other copy of its content is kept - # while it parses - parsed = _parse_gpx_file(io.TextIOWrapper(io.BufferedReader(reader))) + # while it parses. As UTF-8, which GPX files almost always + # are, on every platform rather than in the encoding of the + # locale: a byte that is not UTF-8 only turns a letter of a + # name into U+FFFD, as for NMEA + parsed = _parse_gpx_file( + io.TextIOWrapper( + io.BufferedReader(reader), encoding="utf-8", errors="replace" + ) + ) except (gpxpy.gpx.GPXException, UnicodeError) as ex: parsed = ex # By the content parsed, which a file changed since it was looked diff --git a/tests/unit/test_read_track_files_once.py b/tests/unit/test_read_track_files_once.py index bc48bde3..ca38ae5a 100644 --- a/tests/unit/test_read_track_files_once.py +++ b/tests/unit/test_read_track_files_once.py @@ -10,14 +10,15 @@ from __future__ import annotations +import io import logging import multiprocessing +import types from pathlib import Path import gpxpy import pytest - -from mapillary_tools import sample_video +from mapillary_tools import exceptions, sample_video from mapillary_tools.geotag import ( geotag_images_from_gpx_file, geotag_videos_from_gpx, @@ -154,26 +155,67 @@ def changing_parse(xml_or_file, *args, **kwargs): assert len(utils.read_gpx_tracks(gpx)) == 1 -def test_a_gpx_file_is_decoded_as_a_file_opened_in_text_mode( - tmp_path: Path, parses: list[int] +def _points(tracks: list[utils.Track]) -> list[list[tuple[float, float]]]: + return [[(p.time, p.lat) for p in track] for track in tracks] + + +@pytest.fixture +def cp1252_locale(monkeypatch): + """ + Open files in text mode with cp1252 when no encoding is given, as on + Western Windows + """ + text_io_wrapper = io.TextIOWrapper + + def wrapper(buffer, encoding=None, *args, **kwargs): + return text_io_wrapper(buffer, encoding or "cp1252", *args, **kwargs) + + monkeypatch.setattr( + utils, + "io", + types.SimpleNamespace(TextIOWrapper=wrapper, BufferedReader=io.BufferedReader), + ) + + +@pytest.mark.parametrize("bom", [b"", b"\xef\xbb\xbf"]) +def test_a_gpx_file_is_decoded_as_utf8_whatever_the_locale( + tmp_path: Path, parses: list[int], cp1252_locale, bom: bytes ): """ - As gpxpy parsed a file opened in text mode before, with the encoding of - the platform: a byte order mark and names that are not ASCII + Names that cp1252 cannot decode failed the whole file on Western + Windows, with or without a byte order mark """ + plain = tmp_path / "plain.gpx" + plain.write_bytes(ONE_TRACK) gpx = tmp_path / "track.gpx" - name = "Stra\u00dfe \u00e9t\u00e9".encode("utf-8") - gpx.write_bytes(b"\xef\xbb\xbf" + ONE_TRACK.replace(b"", b"" + name)) - - def outcome(read): - try: - return [[(p.time, p.lat) for p in track] for track in read()] - except Exception as ex: - return type(ex) - - with gpx.open("r") as f: - expected = outcome(lambda: utils._parse_gpx_file(f)) - assert outcome(lambda: utils.parse_gpx(gpx)) == expected + name = "\u00c1rbol \u5317\u4eac".encode("utf-8") + gpx.write_bytes(bom + ONE_TRACK.replace(b"", b"" + name)) + + assert _points(utils.parse_gpx(gpx)) == _points(utils.parse_gpx(plain)) + + +def test_a_byte_that_is_not_utf8_only_spoils_a_name(tmp_path: Path, parses: list[int]): + """An ISO-8859-1 GPX reads with the points of its UTF-8 copy""" + plain = tmp_path / "plain.gpx" + plain.write_bytes(ONE_TRACK) + gpx = tmp_path / "track.gpx" + gpx.write_bytes( + ONE_TRACK.replace( + b'', b'' + ).replace(b"", "Caf\u00e9".encode("latin-1")) + ) + + assert _points(utils.parse_gpx(gpx)) == _points(utils.parse_gpx(plain)) + + +def test_a_utf16_gpx_file_is_still_invalid(tmp_path: Path, parses: list[int]): + gpx = tmp_path / "track.gpx" + gpx.write_bytes(ONE_TRACK.decode("utf-8").encode("utf-16")) + + with pytest.raises(exceptions.MapillaryInvalidGPXError): + geotag_images_from_gpx_file._read_gpx_points(gpx) + with pytest.raises(exceptions.MapillaryInvalidGPXError): + GPXVideoExtractor(tmp_path / "video.mp4", gpx)._read_tracks() def test_only_the_last_gpx_files_are_kept(tmp_path: Path, parses: list[int]):