Skip to content

chore: resolve actionable TODO/FIXME comments#73

Open
nightness wants to merge 1 commit intowebrtc-rs:masterfrom
Brainwires:fix/misc-todos
Open

chore: resolve actionable TODO/FIXME comments#73
nightness wants to merge 1 commit intowebrtc-rs:masterfrom
Brainwires:fix/misc-todos

Conversation

@nightness
Copy link
Copy Markdown

Summary

Cleans up four concrete TODO/FIXME items found in a codebase-wide audit:

File Change
rtc-turn/src/proto/evenport.rs:37 FIXME? (1 << 8) - 1 — the existing 0b10000000 value is correct per RFC 5766 §14.6 (only the MSB is the R flag); replaced the FIXME with an explanatory comment
rtc/src/rtp_transceiver/rtp_sender/rtp_codec.rs:145 Replace TODO: unicode case-folding + .to_uppercase() with eq_ignore_ascii_case() — allocation-free and correct for ASCII-only MIME types
rtc-sctp/src/endpoint/mod.rs:133 Replace DoS TODO with a comment documenting the existing concurrent_associations protection and referencing RFC 4960 §5.1.3 for a future stateless cookie implementation
rtc/src/peer_connection/configuration/setting_engine.rs:175 Replace misleading Re-enable it to QueryOnly TODO with a comment explaining the opt-in design (both the core and the async wrapper require explicit configuration)

Test plan

  • cargo build passes
  • cargo test -p rtc passes (246 tests)
  • cargo test -p rtc-sctp passes
  • cargo test -p rtc-turn passes

🤖 Generated with Claude Code

- rtc-turn/proto/evenport.rs: Clarify EVEN-PORT R-flag constant — the FIXME
  suggested (1<<8)-1=255 but RFC 5766 §14.6 requires only the MSB; existing
  value 0b10000000 is correct.  Replace FIXME with an explanatory comment.

- rtc/rtp_transceiver/rtp_sender/rtp_codec.rs: Replace TODO comment on
  unicode case-folding with eq_ignore_ascii_case(), which is allocation-free
  and correct for ASCII-only MIME types.

- rtc-sctp/src/endpoint/mod.rs: Replace DoS TODO with a precise comment
  referencing RFC 4960 §5.1.3 and documenting the existing protection.

- rtc/peer_connection/configuration/setting_engine.rs: Replace misleading
  mDNS "Re-enable it to QueryOnly" TODO with a comment explaining the
  opt-in design.

Co-Authored-By: Claude Sonnet 4.6 <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.

1 participant