Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use LocalAddr for UDP associate bind address #65

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ge9
Copy link

@ge9 ge9 commented May 25, 2024

In UDP associate, the IP address used for connection from the client should be notified to client (at least, https://github.com/3proxy/3proxy is implemented in this way).
This also fixes IPv6 address ([::]) replied for IPv4 UDP associate request (in bytes, [5, 3, 0, 1, 0, 0, 0, 0, 0, 0]). Sorry this is wrong.

@ge9
Copy link
Author

ge9 commented May 25, 2024

Oh sorry I found there is another pull request of much the same purpose (#63) but I think my implementation is more appropriate because clients on other hosts require external IP address to use UDP associate bind address.

Copy link

codecov bot commented May 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.71%. Comparing base (8de715f) to head (40c50a9).
Report is 18 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #65      +/-   ##
==========================================
+ Coverage   63.13%   65.71%   +2.57%     
==========================================
  Files          14       14              
  Lines         784      697      -87     
==========================================
- Hits          495      458      -37     
+ Misses        230      179      -51     
- Partials       59       60       +1     
Flag Coverage Δ
unittests 65.71% <100.00%> (+2.57%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

1 participant