Skip to content

fix: keep key= distinct from a valueless key in TXT records - #1813

Merged
bdraco merged 3 commits into
python-zeroconf:masterfrom
bboe:fix/txt-empty-value
Aug 28, 2026
Merged

fix: keep key= distinct from a valueless key in TXT records#1813
bdraco merged 3 commits into
python-zeroconf:masterfrom
bboe:fix/txt-empty-value

Conversation

@bboe

@bboe bboe commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

RFC 6763 6.4: a key with no '=' is a boolean attribute; key= is present with an empty value. Collapsing both to None makes them indistinguishable, so a received key= re-encodes as a bare key.

Regression in #1225 (first released in 0.80.0), which replaced explicit branching on the separator with value or None. Restores the behaviour #226 introduced.

@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.81%. Comparing base (df947b2) to head (28edc80).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1813      +/-   ##
==========================================
- Coverage   99.81%   99.81%   -0.01%     
==========================================
  Files          33       33              
  Lines        3760     3759       -1     
  Branches      533      533              
==========================================
- Hits         3753     3752       -1     
  Misses          5        5              
  Partials        2        2              

☔ View full report in Codecov by Harness.
📢 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.

@bboe
bboe force-pushed the fix/txt-empty-value branch from 8edd983 to 3f555e7 Compare August 12, 2026 20:23
@codspeed-hq

codspeed-hq Bot commented Aug 12, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 21 untouched benchmarks


Comparing bboe:fix/txt-empty-value (28edc80) with master (df947b2)

Open in CodSpeed

@bdraco

bdraco commented Aug 26, 2026

Copy link
Copy Markdown
Member

@bluetoothbot review

@bluetoothbot

bluetoothbot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Previous review — superseded by a newer review below.

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

No blocking issues found — ready to merge.

@bluetoothbot

Copy link
Copy Markdown
Contributor

@bdraco: @bluetoothbot review

Re-reviewed. Head unchanged at 3f555e7. Fix still correct: sep branch inverts _set_properties exactly. Four nits carry over unresolved — stale key_sep_value=tuple at info.pxd:89, regression history in the info.py:468 comment, test docstring body, and the "decoded properties" comment at test_info.py:681 that passes info.properties. All non-blocking. Approving.

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

No blocking issues found — ready to merge.

@bdraco

bdraco commented Aug 28, 2026

Copy link
Copy Markdown
Member

note that CI is red until #1820 but thats waiting for PSR

@bboe

bboe commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

note that CI is red until #1820 but thats waiting for PSR

Thanks! I was briefly wondering what I might have broke.

@bboe
bboe force-pushed the fix/txt-empty-value branch from 5e86eb1 to cd1f780 Compare August 28, 2026 04:17
bboe added 2 commits August 27, 2026 21:20
RFC 6763 6.4: a key with no '=' is a boolean attribute; `key=` is present
with an empty value. Collapsing both to None makes them indistinguishable,
so a received `key=` re-encodes as a bare `key`.

Regression in #1225 (first released in 0.80.0), which replaced explicit
branching on the separator with `value or None`. Restores the behaviour
#226 introduced.
@bboe
bboe force-pushed the fix/txt-empty-value branch from cd1f780 to 9f2b4ac Compare August 28, 2026 04:20
@bluetoothbot

Copy link
Copy Markdown
Contributor

@bdraco: note that CI is red until #1820 but thats wai...

Branch now carries that fix. Merge commit 28edc80 pulls master, including df947b2 (#1820). CI should clear on rerun.

@bluetoothbot

Copy link
Copy Markdown
Contributor

PR Review — fix: keep key= distinct from a valueless key in TXT records

Merge-ready — all four prior nits are now resolved and the fix is verified correct end to end.

What's solid:

  • info.py:467-471 branches on the partition separator, which is the exact inverse of _set_properties (info.py:416-420): a b"" value encodes as key=, None encodes as a bare key. This is the only partition(b"=") site in the tree (git grep), so the blast radius is contained.
  • info.pxd:89-96 now declares key/sep/value as bytes and drops the stale key_sep_value=tuple — the .pxd matches the .py in the same commit, per CLAUDE.md's build convention.
  • The comment is trimmed to the RFC 6763 §6.4 invariant, with the regression history left to the PR body.
  • The test docstring is a single line naming what it pins, and the round trip now loops over both info.properties and info.decoded_properties — the str form exercises the properties_contain_str encode path, where "" could plausibly have been re-collapsed to a bare key.

Notes, nothing blocking:

  • Downstream-visible decode change: key= now yields b"" / "" instead of None. That is the stated intent and restores pre-0.80.0 behaviour, but consumers doing props.get(b"x") is None will see it — worth a line in the release notes.
  • Pre-existing and untouched: an empty key (b"=") is still stored rather than ignored per RFC 6763 §6.4. The diff only changes what it stores (None -> b""), and the round trip still reproduces b"=", so no regression.
  • 28edc80 merges master, which carries the cython 3.3.0 build fix (fix: build with cython 3.3.0 #1820), so the earlier red CI should clear on the next run.

✅ Resolved since last review (2)

Previously-flagged issues verified fixed
  • src/zeroconf/_services/info.py:468 Comment carries the regression history, which CLAUDE.md routes to the PR body
  • tests/services/test_info.py:681 Comment says "decoded properties" but the assertion passes info.properties


Checklist

  • Decode is the inverse of encode (round trip verified against _set_properties)
  • No regression for existing None-valued properties
  • .pxd locals match the changed function signature
  • Comment/docstring conventions per CLAUDE.md
  • New behaviour covered by tests (bytes and str APIs, both encode paths)
  • Test isolation and teardown
  • Diff matches PR description, no scope creep
  • No security-relevant surface (no injection, secrets, deserialization, or path handling)

Automated review by Kōan (Claude) HEAD=28edc80 49s

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

No blocking issues found — ready to merge.

@bdraco
bdraco merged commit 899eaaa into python-zeroconf:master Aug 28, 2026
38 checks passed
@bboe
bboe deleted the fix/txt-empty-value branch August 28, 2026 15:31
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 28, 2026
## v0.150.2 (2026-08-28)

### Bug Fixes

- Keep `key=` distinct from a valueless `key` in TXT records
  ([#1813](python-zeroconf/python-zeroconf#1813),
  [`899eaaa`](python-zeroconf/python-zeroconf@899eaaa))


## v0.150.1 (2026-08-28)

### Bug Fixes

- Build with cython 3.3.0 ([#1820](python-zeroconf/python-zeroconf#1820),
  [`df947b2`](python-zeroconf/python-zeroconf@df947b2))
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.

3 participants