The four buffered read paths cap size with min(size, self._buffer_cur - 1): a count-less-one measured from the buffer's start instead of the read position

[JarryShaw]

Every buffered read path in SeekableReader caps the requested size with min(size, self._buffer_cur - 1). That expression is wrong twice over, and the two errors have different consequences:

• _buffer_cur is measured from the start of the buffer, not from the position being read from. The amount genuinely available from the current position is _buffer_cur - (self._tell - self._buffer_set). Capping with _buffer_cur asks the buffer for octets that are not valid from where the read begins.
• The - 1 makes it a count-less-one, so even when the origin happens to be right — _tell == _buffer_set — the read returns one octet fewer than is available.

The two combine into silent data corruption rather than a short read, because the shortfall is made up from the raw stream, whose position has already moved on.

The four sites

grep -n '_buffer_cur - 1' pcapkit/corekit/io.py finds exactly four, with no other spacing variants in the file:

pcapkit/corekit/io.py:212      buf = self._buffer.readline(min(size, self._buffer_cur - 1))     # readline
pcapkit/corekit/io.py:375      buf = self._buffer.read(min(size, self._buffer_cur - 1))         # read
pcapkit/corekit/io.py:405      buf = self._buffer.read1(min(size, self._buffer_cur - 1))        # read1
pcapkit/corekit/io.py:492      buf = self._buffer.read(min(size, self._buffer_cur - 1))         # peek

Lines 375 (read) and 492 (peek) are byte-identical; peek shares read's expression rather than carrying its own variant. Lines 212 and 405 differ from them only in the buffer method called — .readline( and .read1( — so all four are the same expression, and a fix has to address all four rather than the one that happens to be reported.

Worth stating because the finding was first reported twice with different subsets — once as readline/read1/peek, once as the four line numbers above. The four line numbers are correct and the fourth site is peek; the union is all four methods.

Measured

On origin/main (375e9d411), CPython 3.14.7, tree asserted as the repository's own rather than trusting the editable install — pcapkit.__file__ printed and checked on every run, because the __editable__ finder on this machine resolves to a checkout behind origin/main. Every reproduction is io.BytesIO (or a small non-seekable wrapper over one) with no fixtures.

The - 1 alone: an octet vanishes from the middle of a read

r = SeekableReader(io.BytesIO(b'abcde'))
r.read(4)          # b'abcd'
r.seek(0)
r.read(5)          # b'abce'    <- 4 octets; correct is b'abcde'

The arithmetic at line 375, printed from the live object:

_tell=0  _buffer_set=0  _buffer_cur=4
min(size, _buffer_cur - 1) = min(5, 3) = 3      <- what the code asks the buffer for
_buffer_cur - (_tell - _buffer_set) = 4         <- what is actually available

So the buffered part returns b'abc', three of the four available octets. size_rem = 5 - 3 = 2 is positive, so read tops up from self._stream.read(2) — but the raw BytesIO is already at position 4 from the first read(4), so it yields only b'e'. Result b'abce': d is dropped from the middle of the returned octets and nothing indicates it.

The wrong origin: real octets replaced by NUL, and the top-up never fires

Seeking to a position inside the buffered region isolates the origin half, and it is worse — the returned length is correct, so the top-up branch is skipped entirely and the missing data is never fetched:

r = SeekableReader(io.BytesIO(b'0123456789' + b'X' * 20))
r.read(10)         # b'0123456789';  _tell=10, _buffer_set=0, _buffer_cur=10
r.seek(5)          # _tell=5, so the offset into the buffer is 5
r.read(8)          # b'56789\x00\x00\x00'    <- correct is b'56789XXX'

_tell=5  _buffer_set=0  _buffer_cur=10   offset into buffer = 5
min(size, _buffer_cur - 1) = min(8, 9) = 8      <- asks for 8
_buffer_cur - 5 = 5                             <- only 5 are valid from here

self._buffer is constructed as io.BytesIO(bytearray(buffer_size)) at pcapkit/corekit/io.py:105, i.e. pre-filled with NUL. Asking it for 8 octets when 5 are valid does not error — it returns the 5 real octets plus 3 octets of that pre-allocated zero padding. And because len(buf) == size, size_rem is 0 rather than positive, so the raw-stream top-up at lines 378-381 never runs and the real b'XXX' is silently discarded.

Three NUL octets substituted for real capture data, with no exception, no warning, and a correct-looking length. This is the most severe observation in this issue.

Reachable from Extractor, on the very first read of a non-seekable input

This is not a latent API corner. Extractor wraps any non-seekable input stream and then peeks it:

pcapkit/foundation/extraction.py:966-968   if not self._ifile.seekable():  ->  wrap in SeekableReader
pcapkit/foundation/extraction.py:991       self._magic = self._ifile.peek(4)[:4]
pcapkit/foundation/engines/pcapng.py:336   buffer = ext._ifile.peek(4)[:4]

Reproducing that exact sequence against a non-seekable stream with no peek of its own — which is what a pipe, a socket or sys.stdin.buffer looks like:

DATA = b'\xd4\xc3\xb2\xa1' + b'RESTOFHEADER' + b'FIRSTRECORD'   # PCAP magic, then content
r = SeekableReader(Pipe(DATA), stream_closing=False)            # as extraction.py:968 does
r.peek(4)[:4]      # b'\xd4\xc3\xb2\xa1'           correct magic, as read at :991
r.read(16)         # b'\xd4\xc3\xb2RESTOFHEADERF'
                   # correct is b'\xd4\xc3\xb2\xa1RESTOFHEADER'

The \xa1 — the fourth octet of the PCAP magic number — is dropped, and a stray F from the following record is spliced on to make the length up. The control run, identical but with the peek omitted, returns b'\xd4\xc3\xb2\xa1RESTOFHEADER' correctly.

A peek-less non-seekable stream also mis-reads a record boundary the same way:

r = SeekableReader(Pipe(b'RECORD1_RECORD2_'), stream_closing=False)
r.peek(8)          # b'RECORD1_'      correct preview
r.read(8)          # b'RECORD1R'      <- the delimiter is lost and the leading octet
                   #                     of record 2 is spliced into record 1

That reproduction is a joint consequence of this defect and the peek bookkeeping drift filed as #643: the peek populates the buffer without advancing _tell, and the - 1 here is what then drops the octet. Either fix alone would change the symptom, so it is recorded in both issues rather than in only one.

A negative _tell turns the cap into read-to-EOF

min(size, self._buffer_cur - 1) evaluates to -1 when _buffer_cur is 0, and BytesIO.read(-1) means read-to-EOF. Combined with the unguarded SEEK_CUR/SEEK_END arithmetic filed as #643, a one-octet request returns the whole buffer:

r = SeekableReader(io.BytesIO(bytes(range(50))), buffer_size=16)
r.seek(-5, io.SEEK_CUR)    # raises SeekError, but leaves _tell = -5 and _buffer_cur = 0
r.read(1)                  # b'\x00' * 16   <- 16 octets for a 1-octet request

Recorded here because the - 1 is the mechanism that converts a bad _tell into a contract violation on the returned length; the bad _tell itself is #643's.

Why nothing catches it

The suite exercises all four sites, and every assertion that touches them happens to land on a case where the wrong expression gives the right answer.

• tests/corekit/test_io.py:24-32 — test_read_and_seek_round_trip does read(3), seek(1), read(2) with buffer_size=4. The cap is min(2, 3-1) = 2, which equals what was asked for, so the defect is invisible. It passes by coincidence.
• tests/corekit/test_io.py:190-194 — test_buffered_readline_read_read1_and_peek_paths does read(3), seek(1), readline(5) with buffer_size=6 and asserts b'bc\n'. Here the cap is short — min(5, 3-1) = 2 returns only b'bc' — but the line has not ended, so the top-up at lines 214-217 fetches b'\n' from the raw stream, which happens to be exactly the octet the cap dropped. The assertion passes because the top-up masked the shortfall, not because the cap was right.
• tests/corekit/test_io.py:31, :117, :185, :208, :240 — five peek assertions, all with small sizes that avoid the cap, and none checking what the following read returns.

No test asserts a read whose requested size exceeds _buffer_cur - 1, and none seeks to a position strictly inside the buffered region before reading — the two conditions that expose the two halves of the expression.

Notes

• Found as side-findings by the workers on SeekableReader.truncate pads on the wrong side and returns fewer octets than requested #622 (PR fix(corekit): keep SeekableReader's buffer content when truncating it (#622) #633) and Extractor._cleanup closes the caller-supplied stream and leaks the file it opened itself #610 / extract(no_eof=True) never returns: EOF is detected but nothing stops the loop #620 (PRs fix(foundation): close the input stream pcapkit opened, not the caller's (#610) #636 / fix(foundation): give no_eof a way to stop, so extract() returns (#620) #639), several by fuzzing; re-verified here from scratch on 375e9d411 before filing.
• fix(corekit): keep SeekableReader's buffer content when truncating it (#622) #633 is open and unmerged at the time of writing, so pcapkit/corekit/io.py on main does not carry its truncate fix. Everything above was measured against main as it stands, and no reproduction here calls truncate.
• No fix is proposed. The corrected available-from-position count is _buffer_cur - (_tell - _buffer_set), which is a fact about the bookkeeping rather than a patch — the buffer is a sliding window in which _buffer_set + _buffer_cur must stay invariant, and fix(corekit): keep SeekableReader's buffer content when truncating it (#622) #633's own first revision turned a loud ValueError into silent data corruption by changing this file without honouring that. The design is left to whoever picks this up.
• Filed separately from SeekableReader's position bookkeeping is not maintained across operations: a refused seek leaves _tell moved, SEEK_CUR/SEEK_END can drive it negative, and peek desyncs the buffer cursor #643 deliberately. This is four wrong expressions and is fixable on its own; SeekableReader's position bookkeeping is not maintained across operations: a refused seek leaves _tell moved, SEEK_CUR/SEEK_END can drive it negative, and peek desyncs the buffer cursor #643 is a state-machine problem in seek and peek. They interact in two of the reproductions above, and each is cross-referenced from the other, but neither fix requires the other.
• Related: SeekableReader.truncate pads on the wrong side and returns fewer octets than requested #622 (truncate padding, same file), OptionField.unpack never returns for a well-formed HOPOPT header with an SMF_DPD option #431 (the short-read accommodation).