Component update #9
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Remove deprecated RFC 3489 tests (ChangeRequestAttributeTest, XorOnlyTest, StunAddressDiscovererTest variants) as these rely on NAT type detection which was deprecated in RFC 5389 - Remove deprecated attribute tests from AddressAttributeTest and AttributeDecoderTest (SOURCE-ADDRESS, CHANGED-ADDRESS, RESPONSE-ADDRESS, REFLECTED-FROM, CHANGE-REQUEST) - Simplify TransactionSupportTests to test transaction ID generation directly without requiring deprecated standalone StunStack architecture - Remove ShallowStackTest and MessageEventDispatchingTest from test suite (relied on deprecated architecture) - Add new RFC 8445 attribute tests: PriorityAttributeTest, UseCandidateAttributeTest, IceControlAttributeTest (ICE-CONTROLLING/ICE-CONTROLLED) - Add FingerprintAttributeTest for RFC 5389 FINGERPRINT attribute - Fix IceSocketWrapper to handle null Agent for testing scenarios All 128 tests pass.
Updated calculateTa() method to: - Reference RFC 8445 Section 14.2 instead of obsolete RFC 5245 - Use 50ms as the fallback default per RFC 8445 recommendation - Clean up outdated formula comments The 50ms default provides better compatibility with hardware encoders and follows the current ICE specification.
The previous code forced useCandidate=true whenever we were in the controlled role, regardless of whether the controlling peer actually sent the USE-CANDIDATE attribute. This violated RFC 8445. RFC 8445 Section 7.3.1.5 specifies that USE-CANDIDATE should only be considered when the attribute is actually present in the request: - The controlling agent includes USE-CANDIDATE to nominate a pair - The controlled agent MUST NOT include USE-CANDIDATE in its requests This fix ensures proper nomination behavior with RFC-compliant peers and hardware encoders like Videon.
ConnectivityCheckClient.java: - Changed retransmission params from (50ms, 500ms, 2) to (500ms, 1600ms, 6) - RFC 5389 Section 7.2.1 specifies RTO >= 500ms and Rc = 7 (6 retransmissions) - Previous aggressive values caused premature transaction failure with only ~200ms total timeout, now properly allows ~8 seconds for retransmissions - Updated comments to clarify checklistTimeout vs transaction timeout Agent.java: - Fixed calculateStunConnCheckRTO() to use 500ms minimum per RFC 8445/5389 - Updated Javadoc to reference correct RFC formula These changes significantly improve connectivity with hardware encoders like Videon that may have higher latency or packet loss.
When the RFC 8445 symmetric address check fails, now logs: - The candidate pair that failed - Expected vs actual local address (response destination) - Expected vs actual remote address (response source) - Guidance on likely causes (symmetric NAT, NAT rebinding, asymmetric routing) This helps diagnose connectivity issues with hardware encoders behind complex NAT configurations without violating RFC compliance.
Creates integration test infrastructure for verifying STUN and TURN candidate harvesting against a real server: - CoturnContainer: Docker container management for coturn server - Automatic container lifecycle (start/stop/cleanup) - Health checking via STUN binding requests - Configurable credentials and ports - StunHarvesterIntegrationTest: STUN harvesting tests - Server-reflexive candidate discovery - Multiple binding requests - TCP transport support - Unreachable server handling - TurnHarvesterIntegrationTest: TURN harvesting tests - Relayed candidate allocation - Long-term credential authentication - Invalid credential rejection - Combined STUN+TURN harvesting - Allocation lifetime maintenance - TCP transport support Tests use assumeTrue() to skip when Docker is unavailable.
Pull the coturn image explicitly before running the container to avoid mixing pull progress output with the container ID. Also improved error handling and logging for debugging container startup issues.
- Use alternate host ports (13478/15349) to avoid conflicts with local STUN/TURN servers - Map host ports to container's standard ports (3478/5349) correctly - Fix container ID parsing to use first line (64 hex chars) - Clean up partial containers on startup failure
When both sides start as non-controlling (role conflict), the server wins the tie-breaker and becomes controlling. Two issues prevented nomination from completing: 1. Agent.setControlling(): When role changes to controlling during active ICE, now checks for valid pairs that need nomination. Handles the race condition where pairs validate before role switch. 2. DefaultNominator.strategyNominateHighestPrio(): No longer waits for allChecksCompleted() before nominating. When a valid pair is received and isn't nominated, nominates immediately. This fixes the case where only some pairs succeed (others stay FROZEN/WAITING) and the checklist never fully completes. Fixes WHEP subscription failures with Chrome where ICE would timeout waiting for nominations that never happened.
… priority The ConcurrentSkipListSet comparator only compared componentId, priority, and defaultPreference. When multiple host candidates on different IPs had identical values, the second candidate was rejected as a duplicate. Added transport address comparison as tiebreaker to ensure candidates with different addresses are correctly distinguished. Also added diagnostic logging for candidate creation/lookup debugging. Bump version to 1.2.9
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.