fix: strip a systemd journal entry's own block padding, not its data

pcapkit/protocols/schema/misc/pcapng.py

@@ -1833,6 +1833,45 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Self':
             own NUL padding or plain end of data, still starts the next entry --
             even an empty one -- matching what splitting on it always did.
 
+            The guard above ends the entry on a line of *nothing but* NUL
+            padding, but only catches it when the real content ahead of the
+            padding ended with its own newline -- which put the padding on a
+            line by itself for :meth:`~io.BytesIO.readline` to return alone.
+            An entry whose last field is missing that trailing newline, as
+            the format requires, has no such separation: :meth:`readline`
+            runs straight through the field's own bytes and into the
+            padding behind them, returning both as one line, and
+            :meth:`bytes.strip` still will not take the NUL octets off since
+            they are not ASCII whitespace. The padding then went out as part
+            of the field's value with nothing to flag it, silently, however
+            small -- see `#794
+            <https://github.com/JarryShaw/PyPCAPKit/issues/794>`__.
+            :meth:`~io.BytesIO.readline` returns a line without its own
+            trailing newline only at end of stream, so a terminator-less
+            line is necessarily the buffer's last one; a binary field's
+            *name* landing on such a line fails its own 8-octet
+            length-prefix read for the same reason, nothing being left
+            behind it to hold one, so the guard below firing on that shape
+            of line too is harmless. A binary field's *value*, once its
+            name and length prefix are known, is read by that length prefix
+            directly, never by scanning for a line ending, so real block
+            padding immediately behind one cannot land inside it this way:
+            the one-octet terminator check already in place is always
+            reached in that case, and either reads the real separator
+            newline and passes silently, or reads padding's first octet
+            instead, fails, and reports it. Block padding is always 0-3
+            octets, whatever the last field turns out to be, so at most the
+            trailing three octets of a terminator-less line are stripped as
+            padding and warned about, and only when the line's own raw,
+            unstripped tail is itself NUL -- anything past three such
+            octets, or a line whose actual last octet is not NUL at all,
+            cannot be padding and is left as data. This cannot tell a
+            padding octet from a text value that itself legitimately ends
+            in one -- NUL is valid UTF-8 -- but that entry is already
+            malformed for lacking the newline the format mandates, and
+            returning the block's own padding as field data is the one
+            outcome that must not survive it.
+
         """
         self = cast('Self', super().post_process(packet))
 
@@ -1857,6 +1896,47 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Self':
                 if not line.strip(b'\x00'):
                     break
 
+                if not raw_line.endswith(b'\n') and raw_line.endswith(b'\x00'):
+                    # the format requires this line's own trailing newline,
+                    # and it is missing -- so nothing separates its data from
+                    # the block's 32-bit alignment padding that immediately
+                    # follows it, unlike the padding-only line above, which
+                    # only exists as its own line *because* a real newline
+                    # put it there. Padding is at most three octets (0-3, to
+                    # round the entry up to the next multiple of four), so at
+                    # most the last three octets of `line` can be it; strip
+                    # only that many. Anything past that cannot be padding
+                    # and is left as the data the sender actually sent.
+                    #
+                    # gated on the *raw*, unstripped line ending in NUL: pad
+                    # octets are the only NULs to strip, and it is `raw_line`,
+                    # not `line`, that names the entry's actual last octet.
+                    # `line` is `raw_line.strip()`, which has already dropped
+                    # ASCII whitespace from both ends -- so if the entry's
+                    # real last octet were itself whitespace, `line`'s tail
+                    # would no longer match `raw_line`'s, and NUL octets
+                    # further in would be mistaken for the tail. Padding is
+                    # always NUL, never whitespace, so a `raw_line` ending in
+                    # anything else -- ordinary trailing whitespace included
+                    # -- proves the true padding is zero, and nothing here
+                    # may be stripped. See #794.
+                    # the outer gate above already requires `raw_line` --
+                    # and therefore `line`, since `bytes.strip()` cannot
+                    # remove a trailing NUL -- to end in one, so this loop
+                    # always runs at least once and `pad_octets` is never 0
+                    # here; there is no zero-padding case left to guard
+                    # against once the gate has passed.
+                    stripped = line
+                    pad_octets = 0
+                    while pad_octets < 3 and stripped.endswith(b'\x00'):
+                        stripped = stripped[:-1]
+                        pad_octets += 1
+                    warn(f'PCAP-NG: [systemd Journal Export] entry field {line!r} '
+                         f'has no terminating newline; treating its last '
+                         f'{pad_octets} NUL octet(s) as the block\'s own alignment '
+                         'padding, not data', SchemaWarning, stacklevel=stacklevel())
+                    line = stripped
+
                 line_split = line.split(b'=', maxsplit=1)
                 if len(line_split) == 2:
                     key, value = line_split

tests/protocols/misc/test_pcapng_unit.py

@@ -4509,6 +4509,120 @@ def test_a_journal_field_that_is_not_utf8_is_replaced_and_reported(self) -> None
                 self.assertIn('is not UTF-8', str(schema_warnings[0].message))
                 self.assertEqual(entries[0][key], value)
 
+    def test_a_journal_entry_missing_its_final_newline_does_not_return_padding(self) -> None:
+        """#794: no trailing newline lets the block's own padding read as data.
+
+        The block pads its content to a 32-bit boundary with NULs, and the
+        existing padding-only-line guard only fires when the real content
+        ahead of it ended with its own newline, putting the padding on a
+        line of its own. Without that newline, ``readline()`` runs straight
+        through the field's value and into the padding behind it, returning
+        both together -- and ``bytes.strip()`` still will not touch the NUL
+        octets, since they are not ASCII whitespace. Swept over every
+        padding count the alignment rule can produce (0-3 octets), keyed by
+        how many octets ``MESSAGE=...`` needs trimmed from ``entry`` to land
+        on each remainder mod four.
+
+        """
+        from pcapkit.utilities.warnings import SchemaWarning
+
+        cases = {
+            0: b'MESSAGE=abcd',    # 12 octets, already a multiple of four
+            1: b'MESSAGE=abc',     # 11 octets, one NUL pads it to 12
+            2: b'MESSAGE=hi',      # 10 octets, two NULs pad it to 12
+            3: b'MESSAGE=hello',   # 13 octets, three NULs pad it to 16
+        }
+
+        for pad_octets, entry in cases.items():
+            with self.subTest(pad_octets=pad_octets):
+                entries, caught = self._extract_journal(entry)
+
+                self.assertEqual(len(entries), 1)
+                self.assertEqual(entries[0]['MESSAGE'], entry.split(b'=', 1)[1].decode())
+
+                schema_warnings = [item for item in caught
+                                   if item.category is SchemaWarning]
+                if pad_octets == 0:
+                    # nothing was stripped, so there is nothing to warn about
+                    self.assertEqual(schema_warnings, [])
+                else:
+                    self.assertEqual(len(schema_warnings), 1)
+                    self.assertIn('has no terminating newline', str(schema_warnings[0].message))
+                    self.assertIn(f'last {pad_octets} NUL octet(s)', str(schema_warnings[0].message))
+
+    def test_a_terminated_journal_value_ending_in_a_nul_octet_is_kept_whole(self) -> None:
+        """A legitimate value ending in NUL is not the same shape as padding.
+
+        NUL is valid UTF-8, so a text field's value may end in one. Given its
+        own trailing newline, this line is one ``readline()`` call away from
+        the block's own padding -- exactly as it always was -- so #794's fix,
+        which only triggers when that newline is missing, must leave it
+        untouched.
+
+        """
+        entries, caught = self._extract_journal(b'MESSAGE=hi\x00\n')
+
+        self.assertEqual(len(entries), 1)
+        self.assertEqual(entries[0]['MESSAGE'], 'hi\x00')
+        self.assertEqual([item for item in caught
+                          if item.category.__name__ == 'SchemaWarning'], [])
+
+    def test_a_terminator_less_line_whose_raw_tail_is_whitespace_has_zero_padding(self) -> None:
+        """Cross-review on #795: gating on ``raw_line``, not ``line``, matters.
+
+        ``line`` is ``raw_line.strip()`` -- ASCII whitespace already gone from
+        both ends. The first cut of #794's fix checked ``raw_line`` for the
+        missing newline but then counted padding off ``line``, so whenever
+        the entry's real last octet was itself ASCII whitespace (not the
+        newline the format wants, but whitespace all the same), ``strip()``
+        had already eaten it, exposing whatever NUL octets came *before* it
+        as if they were now the tail -- and those get read as block padding
+        that never existed. Padding is NUL, never whitespace, and
+        ``bytes.strip()`` does not touch NUL, so a ``raw_line`` ending in
+        whitespace is proof on its own that the true padding is zero: the
+        block's own last octet, unpadded, IS that whitespace. Swept over
+        every octet :meth:`bytes.strip` treats as ASCII whitespace -- space,
+        CR, tab, vtab and formfeed -- against a value that ends in NUL for a
+        genuine reason of its own -- a NUL is valid UTF-8 -- rather than by
+        accident.
+
+        """
+        from pcapkit.utilities.warnings import SchemaWarning
+
+        cases = {
+            'space': (b'MESSAGE=hi\x00 ', 'hi\x00'),
+            'CR': (b'MESSAGE=hi\x00\r', 'hi\x00'),
+            'tab': (b'MESSAGE=\x00\x00\x00\t', '\x00\x00\x00'),
+            'vtab': (b'MESSAGE=hi\x00\x0b', 'hi\x00'),
+            'formfeed': (b'MESSAGE=\x00\x00\x00\x0c', '\x00\x00\x00'),
+        }
+
+        for tail, (entry, expected) in cases.items():
+            with self.subTest(tail=tail):
+                entries, caught = self._extract_journal(entry)
+
+                self.assertEqual(len(entries), 1)
+                self.assertEqual(entries[0]['MESSAGE'], expected)
+                self.assertEqual([item for item in caught
+                                  if item.category is SchemaWarning], [])
+
+    def test_a_journal_binary_value_ending_in_nul_octets_is_unreachable_by_794(self) -> None:
+        """A binary field's value is read by its own length, never by line.
+
+        Nothing here needs a trailing newline to bound it, so there is no
+        terminator-less line for #794's fix to act on -- confirmed by giving
+        the value itself the shape of alignment padding, then padding the
+        block on top of that, and getting both back distinctly.
+
+        """
+        entries, caught = self._extract_journal(
+            b'BIN\n' + struct.pack('<Q', 2) + b'\x00\x00' + b'\n')
+
+        self.assertEqual(len(entries), 1)
+        self.assertEqual(entries[0]['BIN'], b'\x00\x00')
+        self.assertEqual([item for item in caught
+                          if item.category.__name__ == 'SchemaWarning'], [])
+
     def test_the_captured_len_vector_parses_under_a_memory_cap(self) -> None:
         """#594's amplification band, on the ``captured_len`` vector.