Add port mapping to address rewrite rules by xinze-zheng · Pull Request #831 · pion/ice

xinze-zheng · 2025-11-08T22:31:53Z

Summary

Extend AddressRewriteRule with OriginalPort and NewPort so address and port translation use one configuration API.
Select port mappings with the rule's existing candidate type, local address, interface, CIDR, network, and precedence filters.
Match one gathered port when OriginalPort is non-zero, or every gathered port when it is zero. NewPort must be non-zero unless both fields are zero, which disables port rewriting.
Apply port rewriting to gathered host, server-reflexive, and relay candidates across direct, UDP mux, universal UDP mux, STUN, TURN, and address-rewrite paths.
Change only the advertised candidate port; the underlying packet connection remains bound to its original port.

This replaces the earlier callback API, so the PR adds no handler or agent state.

Tests

Full Go test suite
golangci-lint
Pion shared commit-message linter
Public AddressRewriteRules validation plus wildcard and exact port gathering coverage

Reference

pion/webrtc#3155

codecov · 2025-11-08T22:34:45Z

Codecov Report

❌ Patch coverage is 83.33333% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.60%. Comparing base (27a9ac1) to head (7020974).

Files with missing lines	Patch %	Lines
external_ip_mapper.go	78.94%	2 Missing and 2 partials ⚠️
gather.go	84.00%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #831      +/-   ##
==========================================
- Coverage   88.66%   88.60%   -0.06%     
==========================================
  Files          46       46              
  Lines        6475     6513      +38     
==========================================
+ Hits         5741     5771      +30     
- Misses        500      502       +2     
- Partials      234      240       +6

Flag	Coverage Δ
go	`88.60% <83.33%> (-0.06%)`	⬇️

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

Thank you so much, I like this direction let me know when it's ready for test and review.

JoTurk · 2025-11-08T22:37:24Z

 	EnableUseCandidateCheckPriority bool
+
+	// MapPortHanlder is the handler used to compute mapped port for host candidate.
+	MapPortHanlder func(candidate Candidate) int


We've stopped adding new configurations to the AgentConfig struct since it's planned for deprecation in favor of a more Pion-aligned API. this feature should instead be exposed as an option, WithMapPortHandler, and used with the new NewAgentWithConfig API. Users who want to use this should migrate to the new API.

we did the same thing recently with the renomination API: #822 (comment)

JoTurk · 2025-11-09T21:32:24Z

 	Port() int

+	// Port mapping support for containers
+	MappedPort() int


question, do we have to make this public?

Should we? Will webrtc care about this? I was thinking for design of separted ICE (maybe the future ion) this will be somehow used. What do you think?

I think we should keep it private for now, unless we need it, or someone requests that we expose it later.

JoTurk · 2025-11-09T21:33:12Z

@xinze-zheng I'm sorry about the internal API change, we had to fix the options so we can upgrade webrtc to use them.

JoTurk · 2025-11-09T21:36:23Z

I think it would be better if we can change the test to use the new Public api instead, NewAgentWithOptions, so it doesn't break if we change the internal API in the future, and so it matches the user contract.
But this is up to you.

JoTurk · 2025-12-20T01:54:01Z

@xinze-zheng hello, we merged the options API and added the new Address rewrite API if you want to continue working on this.

JoTurk · 2026-01-06T14:15:42Z

@xinze-zheng I'll try to test and review this today or by tomorrow, thank you a lot :)

xinze-zheng · 2026-01-06T16:03:29Z

@JoTurk Hi I've been looking into integrating this feature with webrtc. Seems we need to replace webrtc.ICECandidate.Port with mappedPort in webrtc.newICECandidateFromICE (or the information of mappedPort is lost). I think we can implement via

	port := candidate.Port()
	if candidate.GetMappedPort() != 0 {
		port = candidate.GetMappedPort()
	}

we just need to make GetMappedPort public in ice.candidate.

JoTurk · 2026-01-08T16:03:27Z

@xinze-zheng I think the only clean path if we go this route is to make a custom extension for mapped ports but i think it's maybe too much. I changed webrtc's candidates so it keeps ICE extensions when converting. Not sure about exposing mapped ports as a helper (but we can do this if no other solution).

I'm maybe missing something, but why we can't overwrite candidates the same we do with addresses?

xinze-zheng · 2026-01-11T10:04:53Z

@JoTurk Hello, I've changed the behavior of mapPort to overwrite the port field of a potential ice candidate.

JoTurk · 2026-01-22T13:04:24Z

+
+func WithMapPortHandler(handler func(cand Candidate) int, candTyp CandidateType) AgentOption {
+	return func(a *Agent) error {
+		a.mapPort = func(candidate Candidate) int {
+			if candidate.Type() == candTyp {


I think we'll either need to remove the candidate type filter or make it possible to add multiple filters for different candidates types.
I think removing is better because the user filter have access to the candidate type.

Agree! Updated.

JoTurk · 2026-02-15T11:00:10Z

@xinze-zheng Sorry for losing track of this, I think we just need to keep track of the original port internally, but this lgtm

@Sean-Der just a quick review is this API what you meant in the original issue?

Containers may bind ICE sockets on ports that differ from the ports published outside the container. Extend AddressRewriteRule with OriginalPort and NewPort so the same rules that publish external addresses can also publish the corresponding external port. Use the existing candidate type, local address, interface, CIDR, network, and rule-precedence filters when selecting a port mapping. OriginalPort selects one gathered port, or matches every port when zero. NewPort must be a valid non-zero port unless both fields are zero, which leaves ports unchanged. Apply the mapping to gathered host, server-reflexive, and relay candidates across direct, UDP mux, universal UDP mux, STUN, TURN, and address-rewrite gathering paths. Only the advertised candidate port changes; the underlying packet connection retains its bound port. Cover validation, wildcard mapping, and exact mapping through the public AddressRewriteRules option.

JoTurk reviewed Nov 8, 2025

View reviewed changes

xinze-zheng marked this pull request as ready for review November 9, 2025 01:19

xinze-zheng requested review from JoTurk and Sean-Der November 9, 2025 01:19

JoTurk reviewed Nov 9, 2025

View reviewed changes

JoTurk mentioned this pull request Nov 11, 2025

[Feature] Docker port mapping #790

Closed

xinze-zheng requested a review from JoTurk December 24, 2025 14:27

JoTurk reviewed Jan 22, 2026

View reviewed changes

Sean-Der force-pushed the feat/port-map branch from 3ded026 to 29b339f Compare September 14, 2026 02:53

Sean-Der changed the title ~~MapPortHandler Callback for container support~~ Add candidate port mapping callback Sep 14, 2026

Sean-Der force-pushed the feat/port-map branch from 29b339f to 6d4232f Compare September 14, 2026 11:56

Sean-Der changed the title ~~Add candidate port mapping callback~~ Add port mapping to address rewrite rules Sep 14, 2026

Sean-Der force-pushed the feat/port-map branch from 6d4232f to 2e24687 Compare September 14, 2026 12:02

Sean-Der force-pushed the feat/port-map branch from 2e24687 to 7020974 Compare September 14, 2026 12:32

Sean-Der approved these changes Sep 14, 2026

View reviewed changes

Sean-Der merged commit cd9e29d into main Sep 14, 2026
19 checks passed

Sean-Der deleted the feat/port-map branch September 14, 2026 13:27

Conversation

xinze-zheng commented Nov 8, 2025 • edited by Sean-Der Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Tests

Reference

Uh oh!

codecov Bot commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JoTurk left a comment

Choose a reason for hiding this comment

Uh oh!

JoTurk Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

JoTurk Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

xinze-zheng Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

JoTurk Nov 9, 2025

Choose a reason for hiding this comment

Uh oh!

JoTurk commented Nov 9, 2025

Uh oh!

JoTurk commented Nov 9, 2025

Uh oh!

JoTurk commented Dec 20, 2025

Uh oh!

JoTurk commented Jan 6, 2026

Uh oh!

xinze-zheng commented Jan 6, 2026

Uh oh!

JoTurk commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xinze-zheng commented Jan 11, 2026

Uh oh!

JoTurk Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

xinze-zheng Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

JoTurk commented Feb 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

xinze-zheng commented Nov 8, 2025 •

edited by Sean-Der

Loading

codecov Bot commented Nov 8, 2025 •

edited

Loading

JoTurk commented Jan 8, 2026 •

edited

Loading