Skip to content

Fix UnknownAttributes encoding to match RFC 5389 - #280

Merged
sirzooro merged 1 commit into
pion:mainfrom
sirzooro:fix_unknown_attrs
Jun 18, 2026
Merged

Fix UnknownAttributes encoding to match RFC 5389#280
sirzooro merged 1 commit into
pion:mainfrom
sirzooro:fix_unknown_attrs

Conversation

@sirzooro

Copy link
Copy Markdown
Contributor

Pion uses 4 bytes per attribute type instead of 2 as specified in RFC 5389. On send side it is not so bad, other STUN peer will see extra zero attributes, so most likely it will be able to deal with this. In opposite direction there is a data loss, half of received attributes is silently discarded when pion decodes received packet.

@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.43%. Comparing base (02391b3) to head (548804d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #280   +/-   ##
=======================================
  Coverage   66.43%   66.43%           
=======================================
  Files          27       27           
  Lines        2124     2124           
=======================================
  Hits         1411     1411           
  Misses        700      700           
  Partials       13       13           
Flag Coverage Δ
go 66.43% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@sirzooro
sirzooro force-pushed the fix_unknown_attrs branch from 221fae6 to 8aee25d Compare June 17, 2026 07:11
@sirzooro
sirzooro requested a review from JoTurk June 17, 2026 14:18
@sirzooro
sirzooro force-pushed the fix_unknown_attrs branch from 8aee25d to 9029f90 Compare June 17, 2026 14:30

@JoTurk JoTurk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The change makes sense, but can you please add a regression test before merging this? thank you.

Pion uses 4 bytes per attribute type instead of 2 as specified in
RFC 5389. On send side it is not so bad, other STUN peer will see extra
zero attributes, so most likely it will be able to deal with this.
In opposite direction there is a data loss, half of received attributes
is silently discarded when pion decodes received packet.
@sirzooro
sirzooro force-pushed the fix_unknown_attrs branch from 9029f90 to 548804d Compare June 18, 2026 06:37
@sirzooro

Copy link
Copy Markdown
Contributor Author

Thanks, added new test.

@sirzooro
sirzooro merged commit e1234ac into pion:main Jun 18, 2026
18 checks passed
@sirzooro
sirzooro deleted the fix_unknown_attrs branch June 18, 2026 06:51
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