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
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions