Skip to content

Fix ICE username/password validation#103

Merged
fippo merged 3 commits intoaiortc:mainfrom
mikhail-barg:main
Nov 28, 2025
Merged

Fix ICE username/password validation#103
fippo merged 3 commits intoaiortc:mainfrom
mikhail-barg:main

Conversation

@mikhail-barg
Copy link
Contributor

@mikhail-barg mikhail-barg commented Nov 26, 2025

This change fixes username and password validation. Old regexes are only checking for lowercase letters, so even the default values generated by random_string() are not being validated. This became apparent while attempting to fix this issue in aiortc.

@lgrahl
Copy link

lgrahl commented Nov 26, 2025

Good catch. Any objections @fippo?

@lgrahl
Copy link

lgrahl commented Nov 26, 2025

Supplement: Can you add a test for this, @mikhail-barg?

@fippo
Copy link
Contributor

fippo commented Nov 26, 2025

Adjusting the set_local_username_and_password to have an uppercase letter should be sufficient as test.

Now how do we get MacOS to work with mdns ...

@codecov
Copy link

codecov bot commented Nov 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (e5b41cb) to head (1bfd51b).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #103   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines         1236      1236           
=========================================
  Hits          1236      1236           

☔ View full report in Codecov by Sentry.
📢 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.

@mikhail-barg
Copy link
Contributor Author

mikhail-barg commented Nov 26, 2025

@lgrahl @fippo I've updated the set_local_username_and_password test, but I'm not sure what to do regarding mDNS test

@fippo
Copy link
Contributor

fippo commented Nov 26, 2025

I think this is hitting a totally unrelated issue (actions/runner-images#10924). Let me look into how to disable MDNS tests on github actions ...

@fippo
Copy link
Contributor

fippo commented Nov 26, 2025

#104 should make this pass hopefully with no more action required 🤞

@fippo
Copy link
Contributor

fippo commented Nov 27, 2025

Looks like the CI doesn't rebase automatically, could you do it locally please?

@mikhail-barg
Copy link
Contributor Author

I've synced my repo (PR source) via github interface, so the PR is updated

@fippo
Copy link
Contributor

fippo commented Nov 27, 2025

(ready to merge but we are still pondering whether to pin the Mac CI to an older version)

@fippo fippo merged commit c520fdd into aiortc:main Nov 28, 2025
34 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

createOffer() is creating two different a=ice-ufrag lines, confusing media servers

3 participants