Skip to content

Allow filtering local IPs by candidate type - #1017

Merged
JoTurk merged 1 commit into
mainfrom
jo/allow-filtering-by-candidate-type
Sep 30, 2026
Merged

JoTurk merged 1 commit into
mainfrom
jo/allow-filtering-by-candidate-type

Conversation

@JoTurk

@JoTurk JoTurk commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

Description

#899 made it so IP filter works for stun and turn, which is the correct thing to do for privacy reasons and control, this change caused an issue for snowflake: #899 (comment) for v5, I propose we change the IP filter parameter from a string IP to a struct so user can filter by the candidate type or any other info we add in the future (we can add interfaces!).

Change WithIPFilter to accept IPFilterInfo containing local IP and candidate type.

@codecov

codecov Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.07%. Comparing base (1925aac) to head (bb1e43c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
gather.go 88.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1017      +/-   ##
==========================================
+ Coverage   91.90%   92.07%   +0.16%     
==========================================
  Files          46       46              
  Lines        5510     5536      +26     
==========================================
+ Hits         5064     5097      +33     
+ Misses        446      439       -7     
Flag Coverage Δ
go 92.07% <91.66%> (+0.16%) ⬆️

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.

@JoTurk
JoTurk requested review from Sean-Der and sirzooro September 29, 2026 05:01

@sirzooro sirzooro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes looks good, just update that comment.

Side note from AI review: raddr/rport in candidates still reveal internal address/port, what is not desirable for applications which hide host candidates for privacy reason. This would deserve separate PR.

Comment thread agent_options.go
@JoTurk

JoTurk commented Sep 30, 2026 •

Copy link
Copy Markdown
Member Author

@sirzooro thank you for the review, I'll add an example comment before I merge.

raddr/rport in candidates still reveal internal address/port,

users can filter ICE extensions, we can make it more clear.

@JoTurk

JoTurk commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

Also this comment reminded me that we need to fix how we serialize extension from and to webrtc. it's currently kinda bad because I couldn't break the webrtc or ICE API when i added the parser.

Change WithIPFilter to accept IPFilterInfo containing
local IP and candidate type.

context for this change:

#899 (comment)
@JoTurk
JoTurk force-pushed the jo/allow-filtering-by-candidate-type branch from 75d2040 to bb1e43c Compare September 30, 2026 20:49
@JoTurk
JoTurk merged commit bb1e43c into main Sep 30, 2026
18 of 19 checks passed
@JoTurk
JoTurk deleted the jo/allow-filtering-by-candidate-type branch September 30, 2026 20:58
JoTurk added a commit to pion/webrtc that referenced this pull request Oct 1, 2026
JoTurk added a commit to pion/webrtc that referenced this pull request Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants