VTT Reader: handle WebVTT files with bad signatures

[palemieux]

No description provided.

[eeshsaxena]

This is a nicer fix than my guard, the spec-based signature check is the right call and it covers the empty-file case cleanly.

One thing worth handling: to_model now returns None when the first line is not a valid signature, but the CLI caller assumes a document. In tt.py around line 356 it does model = vtt_reader.to_model(f, None, progress_callback_read), and from there model is used directly (model.set_lang(...) and the filter/writer path). So converting a .vtt whose first line is not WEBVTT would now raise AttributeError: 'NoneType' object has no attribute ... at the call site instead of failing in the reader. Might be worth erroring out cleanly there when to_model returns None, similar to the unsupported-input branch just below.

[palemieux]

@eeshsaxena addressed at 944c601

[eeshsaxena]

Thanks for taking this on, @palemieux. The signature handling and the extra coverage (test_bad_signature_1..5, the empty-file cases, and excluding invalid/ from test_valid_samples) look solid, and returning None on a bad signature is a clean contract.

Two things I noticed in the file-parsing preprocessing in reader.py:

1. The NUL replacement doesn't actually target NUL.

data_file.read().replace("\^@", "\uFFFD")

"\^@" isn't a NUL byte. \^ is an invalid escape sequence, so the literal is the three characters \, ^, @ (and on Python 3.12+ it raises a SyntaxWarning that is slated to become an error). A real U+0000 in the input is therefore left in place:

>>> ("WEBVTT\x00x").replace("\^@", "\uFFFD")
'WEBVTT\x00x'    # NUL still present
>>> ("WEBVTT\x00x").replace("\x00", "\uFFFD")
'WEBVTT\ufffdx'  # replaced

The spec step is "Replace each U+0000 NULL character ... with a U+FFFD", so this looks like it wants .replace("\x00", "\uFFFD") (or "\u0000").

2. Leftover chained comparison.

if len(subtitle_lines) > 0 is not None:

The is not None is left over from the old subtitle_text is not None check. It parses as (len(subtitle_lines) > 0) and (0 is not None), so it still happens to mean len(subtitle_lines) > 0, but it is easy to misread. if subtitle_lines: would be clearer.

Overall this is the better approach and handles more cases than #534 did, so I am happy to close mine in favor of it. Thanks!

[palemieux]

1. The NUL replacement doesn't actually target NUL.
   data_file.read().replace("^@", "\uFFFD")

Where do you see this in the source code? I see:

lines = data_file.read().replace("\u0000", "\uFFFD") \

if len(subtitle_lines) > 0 is not None:

Fixed at d781f31

[palemieux]

@eeshsaxena I plan on merging this soon