Skip to content

feat: add support for original_dst for windows #529

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.

Sign up for GitHub

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

Jump to bottom
Merged
Thomasdezeeuw merged 8 commits into from
Nov 27, 2024
Merged

feat: add support for original_dst for windows #529

Thomasdezeeuw merged 8 commits into from
Nov 27, 2024

Conversation

@keithmattix
Copy link
Contributor

@keithmattix keithmattix commented Sep 12, 2024 •
edited
Loading

I noticed there wasn't support for original dst in Windows, so I added it. Also add windows gnu target to allow cross compiling

Thomasdezeeuw
Thomasdezeeuw reviewed Sep 18, 2024
View reviewed changes
Copy link
Collaborator

@Thomasdezeeuw Thomasdezeeuw left a comment

Choose a reason for hiding this comment

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

It seems that at least Linux has SO_ORIGINAL_DST and IP6T_SO_ORIGINAL_DST as well, can we add it for it as well?

@keithmattix
Copy link
Contributor Author

keithmattix commented Sep 18, 2024

@Thomasdezeeuw it's already added in the sys/unix.rs file:

socket2/src/sys/unix.rs

Line 2420 in 3a93893

pub fn original_dst(&self) -> io::Result<SockAddr> {

@Thomasdezeeuw
Copy link
Collaborator

Thomasdezeeuw commented Sep 18, 2024

@Thomasdezeeuw it's already added in the sys/unix.rs file:

socket2/src/sys/unix.rs

Line 2420 in 3a93893

pub fn original_dst(&self) -> io::Result<SockAddr> {

Ah, in that case it should be moved to the src/socket.rs file to ensure the API is the same across OS.

@keithmattix
Copy link
Contributor Author

keithmattix commented Sep 18, 2024

Ok yeah I had that thought but wanted to confirm. Will make the change!

@keithmattix
Copy link
Contributor Author

keithmattix commented Sep 30, 2024

@Thomasdezeeuw any clue what's going on with the FreeBSD CI? From what I can tell, it looks like it's failing due to the test runners not support ipv6

Thomasdezeeuw
Thomasdezeeuw requested changes Oct 6, 2024
View reviewed changes
@keithmattix keithmattix force-pushed the windows-original-dst branch from 7a46f91 to d75c86d Compare October 7, 2024 20:25
@keithmattix
Copy link
Contributor Author

keithmattix commented Oct 12, 2024

I opened #534 to address the FreeBSD failures

@keithmattix
Copy link
Contributor Author

keithmattix commented Nov 25, 2024

@Thomasdezeeuw gentle ping for review

Thomasdezeeuw
Thomasdezeeuw reviewed Nov 25, 2024
View reviewed changes
@keithmattix
Implement original_dst for windows
f026ca7 
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix
Generalize original_dst funcs
c278b9c 
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
@keithmattix
Remove windows-gnu as target due to failure
128d5dc 
Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>
Thomasdezeeuw
Thomasdezeeuw reviewed Nov 27, 2024
View reviewed changes
@Thomasdezeeuw Thomasdezeeuw mentioned this pull request Nov 27, 2024
Merged
@Thomasdezeeuw Thomasdezeeuw enabled auto-merge (squash) November 27, 2024 16:11
@Thomasdezeeuw Thomasdezeeuw merged commit 33a98be into rust-lang:master Nov 27, 2024
39 of 42 checks passed
@Thomasdezeeuw
Copy link
Collaborator

Thomasdezeeuw commented Nov 27, 2024

Thanks @keithmattix

Hasan6979 pushed a commit to NordSecurity/socket2 that referenced this pull request May 15, 2025
@keithmattix @Hasan6979
Add support for original_dst for windows (rust-lang#529)
f0e1a76
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Thomasdezeeuw Thomasdezeeuw Thomasdezeeuw approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@keithmattix @Thomasdezeeuw