vendor: thirteen re.sub sites pass a regex flag as the positional count, capping substitutions at 8

[JarryShaw]

Describe the bug

Thirteen re.sub call sites in pcapkit/vendor/ pass a regex flag where the fourth positional parameter is count. Enumerated by AST rather than grep, on 2d1c8675d:

pcapkit/vendor/default.py:343              pcapkit/vendor/ipv6/router_alert.py:55
pcapkit/vendor/hip/eddsa_curve.py:59       pcapkit/vendor/ipv6/tagger_id.py:55
pcapkit/vendor/http/frame.py:64            pcapkit/vendor/reg/ethertype.py:95
pcapkit/vendor/http/method.py:144          pcapkit/vendor/tcp/flags.py:139
pcapkit/vendor/http/status_code.py:148     pcapkit/vendor/tcp/mp_tcp_option.py:65
pcapkit/vendor/ipv4/router_alert.py:58     pcapkit/vendor/tcp/option.py:78
pcapkit/vendor/ipv6/option.py:89

re.sub(pattern, repl, string, count=0, flags=0) — so re.MULTILINE, which is 8, is read as "replace at most 8 occurrences". Measured:

>>> re.MULTILINE
8
>>> re.sub(r'\r*\n', ' ', 'a\n' * 12, re.MULTILINE)      # flag as positional
'a a a a a a a a a\na\na\na\n'                            # <- stops after 8
>>> re.sub(r'\r*\n', ' ', 'a\n' * 12, flags=re.MULTILINE)
'a a a a a a a a a a a a '

Two defects in one line:

1. The count is a real limit. Any IANA description with more than eight newlines keeps its ninth onward, so the generated const module gets embedded newlines where it should have spaces. That output is committed to the tree, so the damage is visible rather than theoretical.
2. The flag never applies. For r'\r*\n' that happens not to matter — no ^/$ in the pattern — so the intent was already a no-op. Check each site; if any pattern uses an anchor, its behaviour is wrong today too.

Also a deprecation: DeprecationWarning: 'count' is passed as positional argument fires on 3.13+, and it becomes a TypeError in a later release, so these break on a future interpreter regardless of semantics.

Expected behavior

flags=re.MULTILINE, or drop the flag where the pattern has no anchor — cleaner than passing one that does nothing.

Additional context

Correcting this issue's own first version, which said "six". A single-line grep found five; the other eight put the flag on a continuation line (), re.MULTILINE))) and were invisible to it. The AST sweep above is the reliable count.

Found by the cross-review on #789, which had just fixed the same shape in re.split at httpv1.py:310/:346. A re-grep found zero remaining re.split sites, so these are the rest of the family.

Two notes for whoever takes it: eight of the thirteen sit on lines also carrying '%s %s' % and a # pylint: disable=consider-using-f-string, so it overlaps #792's convention work — coordinate or sequence them. And regenerating to show the consequence will fetch live IANA registries and move unrelated rows; #792 records the hand-edit-plus-prove-equivalence route around that.

[JarryShaw]

Labelling blocked so it does not read as unheld.

Checkable blocker: #792 merged. Eight of the thirteen sites sit on lines that also carry '%s %s' % formatting and a # pylint: disable=consider-using-f-string, and #792 is converting %-style to f-strings under the maintainer's convention. Both touch the same lines in pcapkit/vendor/, and two agents editing one line clobber each other silently — no error, the later write just wins.

#792 is wip with a worker on it now, so this unblocks shortly. Its line numbers will shift when it lands; re-derive with the AST sweep in the body rather than trusting the list.

[JarryShaw]

Tightening this blocker, which I worded imprecisely. It said "Checkable blocker: #792 merged" — but #792 is an issue, and an issue does not merge. The thing to check is #797 merged (053c0889d, refactor/const/review: pending), which is the PR that closes #792 and rewrites the lines this issue overlaps.

gh pr view 797 -R JarryShaw/PyPCAPKit --json state,mergedAt is the one-command check.

The overlap itself is narrower than I first implied, and worth recording so nobody re-derives it: #797 left all three %-formatted dunders and extend_enum's '%d' calls alone, converting only the six raises. So the # pylint: disable=consider-using-f-string stays earned, and the eight of this issue's thirteen sites that carry '%s %s' % formatting are not touched by #797 at all — they live in pcapkit/vendor/'s crawlers, not in the AppType template.

Which means the real conflict risk is smaller than "same lines": it is that both changes edit pcapkit/vendor/ files in the same window. Still worth serialising, but this unblocks the moment #797 lands rather than needing a wider stand-down.

[JarryShaw]

Correcting myself: #796 and #798 do overlap, on two files. My earlier comment said they "do not collide with each other, only with #797". That was wrong.

#798 files: 3    #796 files: 13    OVERLAP: 2
  !! pcapkit/vendor/http/method.py
  !! pcapkit/vendor/tcp/flags.py

#798 sweeps the %-style guards in the three bespoke templates — tcp/flags.py, ftp/command.py, http/method.py — and two of those three are also in this issue's thirteen re.sub sites. Two agents on one file clobber each other with no error; the later write simply wins.

Worth saying why I got it wrong, since the shape recurs: I ran the comparison as a shell loop and it printed nothing, and I took the empty output as "no overlap" rather than as "the loop did not work". Re-derived in Python, the overlap is two files. An empty result from a one-off script is not evidence until the script is shown to detect the thing it is looking for.

Blocker updated: #798 merged, not #797 — which has already merged, so that hold is spent and this one replaces it. #798 is the smaller change (3 files, one construct) and its line numbers move less, so it goes first.

gh pr list -R JarryShaw/PyPCAPKit --search "798 in:body" --state all --json number,state

Re-derive the thirteen sites with the AST sweep in the body when this starts — #798 will shift line numbers in two of them, and a grep misses the eight that put the flag on a continuation line.

[JarryShaw]

Restating the blocker as the last comment, so it is checkable without reading up the thread.

Checkable blocker: #798 merged.

gh issue view 798 -R JarryShaw/PyPCAPKit --json state

Reason, re-derived in Python after a shell loop misled me into reporting no overlap: #798 touches 3 files, this issue touches 13, and 2 are shared — pcapkit/vendor/http/method.py and pcapkit/vendor/tcp/flags.py. Both issues rewrite the same raise lines in those two, so whichever lands second rewrites the other's work with no conflict marker to show it happened.

#798 is the smaller change and has a worker on it now, so it goes first. This unblocks the moment it merges. Chain: #798 → this issue → #775.

[JarryShaw]

Unblocked — #798 merged via #803 at 21:44:59Z as 4530424df. main is now 4530424df, and the two files that overlapped (pcapkit/vendor/http/method.py, pcapkit/vendor/tcp/flags.py) are free.

Dispatching a worker. Two things from #803 that bear on this:

• Thirteen sites, not six. A single-line grep catches five; eight put the flag on a continuation line. Use an AST sweep over Call(re.sub/re.split) and self-test it against a known-positive and a known-negative — a one-line grep produced the original wrong count of 6.
• refactor(vendor): finish the %-to-f-string sweep on the four bespoke templates #803 converted %-formatting in these same files, so re-derive every line number against origin/main (git show origin/main:<path>) rather than reusing any number from this thread.

#804 is now blocked on this issue, since it also edits vendor/http/method.py.

[JarryShaw]

Correcting this issue's own claim about the damage, found by #813's cross-review and verified by me.

The body says the generated const module "gets embedded newlines where it should have spaces" and that the damage is visible rather than theoretical. That is wrong. All 13 sites feed Vendor.wrap_comment(), which is textwrap.wrap(text.strip(), 76), and textwrap.wrap defaults to replace_whitespace=True — so any newline surviving the count=8 cap is turned into a space before it reaches the generated file:

pre-fix  re.sub output has newlines: True   |  post-fix: False
after wrap_comment, pre == post ?    True        <- the newline never reaches the file
SELF-TEST replace_whitespace=False would differ -> True   (so the masking is real, not an artefact)

The fix is still correct and still worth making — passing a flag as the positional count is wrong regardless, it is a DeprecationWarning on 3.13+ and a TypeError later, and #813 proves the flag was inert for this pattern at any count. But the justification is "the call means something other than what it says", not "the output is corrupted".

The one genuinely observable residual is \r\n past the 8th match, which leaves doubled spaces rather than newlines:

pre : 'x x x x x x x x x  x  x  x  TAIL'      post: 'x x x x x x x x x x x x TAIL'

#813's reviewer scanned all of pcapkit/const/ for that signature and found 25 hits, every one attributable to another cause — a [**]/[nn] footnote deleted by tcp/option.py's outer re.sub (3), a lone \r surviving name.replace('\n',' ') in reg/ethertype.py (1), and IANA's own intra-bracket whitespace in sctp/parameter.py and sctp/payload_protocol_identifier.py (21). So there is no evidence the cap ever bit in practice.

Recording this because #813's ast.parse-over-139-files check cannot fail on this defect and should not be read as evidence that it did not occur. The real evidence is the exhaustive 3,124-case proof that MULTILINE is a no-op for r'\r*\n'.