Every systemd journal field after the first binary field is discarded: entry_data.read() reads to EOF where it means to skip one newline

[JarryShaw]

Found while fixing #678 (PR #699), which fixes the exception leak two lines above this one and leaves
this alone deliberately: it is silent data loss on valid input rather than a length reaching a read, so
it changes parse output for well-formed captures and wants its own review.

Every journal field after the first binary field is discarded

pcapkit/protocols/schema/misc/pcapng.py, in SystemdJournalExportBlock.post_process:

                    length = struct.unpack('<Q', prefix)[0]  # type: int
                    entry.add(line.decode('utf-8'), entry_data.read(length))
                    entry_data.read()  # Skip trailing newline.

The comment says "skip trailing newline"; io.BytesIO.read() with no argument reads to EOF. So the
while True loop's next readline() returns b'', breaks, and everything after the binary field is
gone. Measured on 08f5b8df7 (PR #699's head; identical on f0999858e), against
pcapkit.__file__ = .../pcapkit/__init__.py printed on the run:

entry data	parsed
b'BINARY\n' + <8-octet length 3> + b'abc\nAFTER=one\nMORE=two\n'	[('BINARY', b'abc')] — AFTER and MORE lost
b'BEFORE=zero\nBINARY\n' + <8-octet length 3> + b'abc\n'	[('BEFORE', 'zero'), ('BINARY', b'abc')] — correct
b'BEFORE=zero\nAFTER=one\nMORE=two\n'	all three — correct

No warning, no error. A field before the binary one survives; every field after it does not.

Why it matters

The systemd Journal Export Format puts no ordering constraint on binary fields — a binary field is a
name line, a 64-bit little-endian length, that many octets, and a newline, and it may appear anywhere in
an entry. MESSAGE is routinely binary (it is whenever the message contains a newline or a non-UTF-8
octet), and it is conventionally written early, so the common case is the broken one: every field
after MESSAGE disappears.

The fix, and the one thing it has to decide

entry_data.read() should consume exactly the one octet the format puts there:

entry_data.read(1)  # the newline that terminates a binary field

What wants deciding is what to do when that octet is not a newline, or is absent because the entry
was cut short. The surrounding code now treats "the entry ran out" as end-of-entry with a
SchemaWarning (PR #699, for the 64-bit length prefix immediately above), and the same shape is
probably right here: a binary field whose terminator is missing or wrong is a malformed entry, and
reporting it beats both raising and silently resynchronising at the wrong offset.

Note this is not the same defect as the struct.error PR #699 fixes, and fixing that one does not
help here: it guards the length prefix being short, whereas this is the read after a
successfully-parsed value.

Scope

Not fixed in #699 for the same reason it is filed separately: recovering the discarded fields changes
the parse output of every journal export block that has a binary field followed by anything, so it is a
behaviour change on valid captures and needs a breaking argument of its own, plus a fixture that
actually exercises a multi-field journal entry — examples/captures/ has none, so #699's coverage of
this block comes entirely from hand-built buffers.

Related

• Every EOF-truncated PCAP-NG file raises an uncaught ValueError: pcapng_block_selector passes a negative __length__ to SchemaField #678 / fix(pcapng): stop a negative length reaching a read on a truncated capture (#678) #699 — the bare struct.error two lines above, from a short 64-bit length prefix, and the
  32-bit NUL padding being read as a field name. Both fixed there; this one is not.
• every PCAP-NG packet block loses its payload octets: packet is computed then overwritten with b'', so dumping through PCAPIO writes a record header claiming 314 octets followed by none #646 / fix(pcapng): keep the captured octets every packet block declares (#646) #683 — also PCAP-NG and also about octets going missing, but at the packet-block payload
  rather than inside a journal entry.