Skip to content

_make_http_data never reads frame.flags, so END_STREAM is silently dropped on a parse-reconstruct round trip #652

Description

@JarryShaw

_make_http_data is the only one of the six _make_http_* methods that does not restore its flag from the frame object it is handed. It reads back pad_len and data and silently drops END_STREAM, so a DATA frame parsed with that bit set rebuilds with the bit clear. A parse → reconstruct round trip loses it.

Measured on 375e9d411, CPython 3.14.7, tree asserted against the worktree rather than the editable install. pcapkit/protocols/application/httpv2.py is byte-identical on current origin/main (da381f259).

The site

pcapkit/protocols/application/httpv2.py:945-947:

        if frame is not None:
            pad_len = frame.pad_len
            data = frame.data

There is no end_stream = frame.flags.END_STREAM. Two lines later the accumulator is seeded and the bit is tested against the caller's parameter, which is still at its default because nothing restored it:

        flags = Schema_DataFrame.Flags(0)          # 949
        if end_stream:                             # 950
            flags |= Schema_DataFrame.Flags.END_STREAM

Every sibling does restore it — _make_http_headers:986-988 (priority, end_headers, end_stream), _make_http_settings:1094 (ack = frame.flags.ACK), _make_http_push_promise:1150, _make_http_ping:1186, _make_http_continuation:1278.

Measured

Parsing a real DATA frame and feeding the resulting data object straight back to _make_http_data:

parsed flags=0x00 (END_STREAM clear)  info.flags.END_STREAM=False -> rebuilt flags=<Flags: 0>
parsed flags=0x01 (END_STREAM SET)    info.flags.END_STREAM=True  -> rebuilt flags=<Flags: 0>

The bit is present on the parse and gone on the rebuild. Control, with the same shape of frame object handed to the siblings:

_make_http_settings(frame ACK=True)        -> flags=<Flags.ACK: 1>          ACK kept? True
_make_http_ping(frame ACK=True)            -> flags=<Flags.ACK: 1>          ACK kept? True
_make_http_continuation(END_HEADERS=True)  -> flags=<Flags.END_HEADERS: 4>  kept? True
_make_http_data(frame END_STREAM=True)     -> flags=<Flags: 0>              kept? False

PADDED survives only because it is re-derived from pad_len rather than read from frame.flags.

Consequence

Silent, lossy round trip on a flag that carries real protocol meaning: END_STREAM is how HTTP/2 half-closes a stream (RFC 9113 §6.1). Rebuilding a captured DATA frame through _make_http_data produces bytes that say the stream continues when the original said it ended. Nothing raises and nothing warns — the output is a well-formed frame with one bit wrong.

Why no test caught it

tests/protocols/application/test_http_unit.py:783 exercises this exact path with a stub that has no flags attribute at all:

        frame_schema, frame_flags = proto._make_http_data(SimpleNamespace(pad_len=3, data=b'from-frame'))
        self.assertEqual(frame_schema.data, b'from-frame')
        self.assertTrue(frame_flags & DataFrame.Flags.PADDED)

It asserts PADDED (which is re-derived and survives) and never mentions END_STREAM. The headers test immediately below, at :798-800, does pass a flags stub:

        headers_frame_schema, headers_frame_flags = proto._make_http_headers(SimpleNamespace(
            flags=SimpleNamespace(PRIORITY=True, END_HEADERS=True, END_STREAM=False),

so the test file itself encodes the asymmetry. Adding the missing line to _make_http_data would make the :783 stub raise AttributeError, so a fix has to extend that stub with flags=SimpleNamespace(END_STREAM=..., PADDED=...) in the shape :798-800 already uses — and should assert the restored bit, which nothing currently does.

Notes

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions