Skip to content

Error on unknown comprehension-required attributes w/strict#276

Open
fippo wants to merge 1 commit intopion:mainfrom
fippo:strict-attributes
Open

Error on unknown comprehension-required attributes w/strict#276
fippo wants to merge 1 commit intopion:mainfrom
fippo:strict-attributes

Conversation

@fippo
Copy link
Copy Markdown
Contributor

@fippo fippo commented May 5, 2026

Throws an error when encountering a unknown comprehension-required attribute in strict mode and log a warning in non-strict mode.

(not sure if this is worth it even...)

@fippo fippo force-pushed the strict-attributes branch from 5d7df83 to 758c452 Compare May 5, 2026 08:13
Comment thread attributes.go
Comment on lines +142 to 176
var attrNames = map[AttrType]string{
AttrMappedAddress: "MAPPED-ADDRESS",
AttrUsername: "USERNAME",
AttrErrorCode: "ERROR-CODE",
AttrMessageIntegrity: "MESSAGE-INTEGRITY",
AttrUnknownAttributes: "UNKNOWN-ATTRIBUTES",
AttrRealm: "REALM",
AttrNonce: "NONCE",
AttrXORMappedAddress: "XOR-MAPPED-ADDRESS",
AttrSoftware: "SOFTWARE",
AttrAlternateServer: "ALTERNATE-SERVER",
AttrFingerprint: "FINGERPRINT",
AttrPriority: "PRIORITY",
AttrUseCandidate: "USE-CANDIDATE",
AttrICEControlled: "ICE-CONTROLLED",
AttrICEControlling: "ICE-CONTROLLING",
AttrChannelNumber: "CHANNEL-NUMBER",
AttrLifetime: "LIFETIME",
AttrXORPeerAddress: "XOR-PEER-ADDRESS",
AttrData: "DATA",
AttrXORRelayedAddress: "XOR-RELAYED-ADDRESS",
AttrEvenPort: "EVEN-PORT",
AttrRequestedTransport: "REQUESTED-TRANSPORT",
AttrDontFragment: "DONT-FRAGMENT",
AttrReservationToken: "RESERVATION-TOKEN",
AttrConnectionID: "CONNECTION-ID",
AttrRequestedAddressFamily: "REQUESTED-ADDRESS-FAMILY",
AttrMessageIntegritySHA256: "MESSAGE-INTEGRITY-SHA256",
AttrPasswordAlgorithm: "PASSWORD-ALGORITHM",
AttrUserhash: "USERHASH",
AttrPasswordAlgorithms: "PASSWORD-ALGORITHMS",
AttrAlternateDomain: "ALTERNATE-DOMAIN",
AttrDtlsInStun: "DTLS-IN-STUN",
AttrDtlsInStunAck: "DTLS-IN-STUN-ACKNOWLEDGEMENT",
}
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.

not related, but I wonder if we should refactor this, and other Pion libraries so that IANA fields are automated in the CI (cron-job and check?), I did this for a rust SDP library I'm developing with @Juliapixel
https://github.com/meowdia/sphynx/blob/main/src/iana/generated.rs
meowdia/sphynx#6

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah... what I have done in the past is to let depending packages (i.e. a TURN package) register methods and attributes

@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.98%. Comparing base (01aa5b8) to head (c6890a4).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #276      +/-   ##
==========================================
- Coverage   66.43%   65.98%   -0.45%     
==========================================
  Files          27       27              
  Lines        2124     2096      -28     
==========================================
- Hits         1411     1383      -28     
  Misses        700      700              
  Partials       13       13              
Flag Coverage Δ
go 65.98% <100.00%> (-0.45%) ⬇️

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

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

Throws an error when encountering a unknown comprehension-required
attribute in strict mode and log a warning in non-strict mode.
@fippo fippo force-pushed the strict-attributes branch from 758c452 to c6890a4 Compare May 5, 2026 08:40
@fippo fippo marked this pull request as ready for review May 5, 2026 10:12
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