Skip to content

TurnServer: remove workarounds and add tests for uncovered error paths #1529

@CraziestPower

Description

@CraziestPower

Summary

TurnServer.cs contained two workarounds for bugs that have since been fixed upstream:

These should be replaced with the fixed library methods:

  • new STUNErrorCodeAttribute(code, reason) (8 call sites)
  • request.CheckIntegrity(key) (1 call site)

Additionally, 5 of the 8 STUNErrorCodeAttribute error paths have no test coverage:

  1. 437 — Duplicate allocation (allocate when allocation already exists)
  2. 437 — Refresh without allocation
  3. 437 — CreatePermission without allocation
  4. 437 — ChannelBind without allocation
  5. 400 — ChannelBind missing channel number or peer address

Acceptance criteria

  • Remove BuildErrorCodeAttribute() helper and replace all 8 call sites with new STUNErrorCodeAttribute(code, reason)
  • Remove VerifyMessageIntegrity() helper and replace with request.CheckIntegrity(key)
  • Remove the rawBytes parameter threaded through ProcessMessage and HandleAllocate
  • Remove the outdated security doc comment about the workarounds
  • Add 5 unit tests covering the uncovered error paths
  • All 17 TurnServer tests pass

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions