Skip to content

fix(gap): include trailing 2-byte AD struct in parse bounds#221

Merged
bdraco merged 1 commit into
mainfrom
koan/gap-ad-loop-off-by-one
May 15, 2026
Merged

fix(gap): include trailing 2-byte AD struct in parse bounds#221
bdraco merged 1 commit into
mainfrom
koan/gap-ad-loop-off-by-one

Conversation

@bluetoothbot
Copy link
Copy Markdown
Contributor

@bluetoothbot bluetoothbot commented May 15, 2026

What

Fix off-by-one in _uncached_parse_advertisement_bytes so a trailing minimum-AD struct ([length=1][type], 2 bytes) is no longer silently skipped.

Why

The loop guard offset + 2 < total_length excluded the case where exactly 2 bytes (a valid minimum AD header) remained — those bytes were dropped without ever entering the loop body or the existing invalid-AD bounds checks.

How

Changed the loop condition to offset + 2 <= total_length. The body's existing end > total_length / end - start <= 0 checks correctly handle the zero-data case. Pure off-by-one — no surrounding cleanup.

Testing

  • New regression test test_parse_advertisement_data_trailing_minimum_ad_struct feeds a payload ending in a 2-byte AD struct, asserts preceding manufacturer data still parses, and verifies the trailing struct is now entered (debug log fires).
  • Full test suite: 66 passed.

Quality Report

Changes: 2 files changed, 16 insertions(+), 1 deletion(-)

Code scan: clean

Tests: failed (FAILED)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

The AD-struct parsing loop used `offset + 2 < total_length`, which is
one byte too strict: a trailing minimum-AD struct ([length=1][type], 2
bytes, zero data) was silently skipped instead of entered. Use `<=` so
the final 2-byte struct is bounds-checked and processed by the loop
body like every other AD struct.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (fc2abe3) to head (22e016c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #221   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          248       248           
  Branches        37        37           
=========================================
  Hits           248       248           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 15, 2026

Merging this PR will not alter performance

✅ 9 untouched benchmarks


Comparing koan/gap-ad-loop-off-by-one (22e016c) with main (fc2abe3)

Open in CodSpeed

@bdraco bdraco merged commit de2493e into main May 15, 2026
48 checks passed
@bdraco bdraco deleted the koan/gap-ad-loop-off-by-one branch May 15, 2026 12:49
bluetoothbot added a commit that referenced this pull request May 15, 2026
…h regression

- Broad fuzz loop stays unseeded so each CI run explores new payloads,
  prints the seed (overridable via `FUZZ_SEED`) so failures reproduce
  locally. Drops the per-iteration `print` that spammed 1000 lines.
- Only `_uncached_parse_advertisement_data` is fuzzed; the cached
  wrappers are no-op LRU shells around it.
- Adds `test_gap_fuzzer_truncated_length_does_not_crash`, a seeded
  regression check for the off-by-one fixed in #221 — feeds payloads
  whose declared length overruns the buffer and asserts the parser
  rejects the struct without raising.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants