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

feat(localip): use reserved remote address #4648

Merged
merged 1 commit into from Dec 1, 2022

Conversation

cgzones
Copy link
Contributor

@cgzones cgzones commented Nov 25, 2022

Description

Instead of the remote address of 8.8.8.8 (Google DNS) in the crate local_ipaddress use a reserved IPv4 address, that should never be assigned.
Also forward the underlying error on failure.

Motivation and Context

The current used crate local_ipaddress connects an UDP socket to 8.8.8.8 (Google DNS), which might trigger some privacy concerns (although no actual packet should be send).

Supersedes: #4614

How Has This Been Tested?

  • I have tested using MacOS
  • I have tested using Linux
  • I have tested using Windows

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

Copy link
Contributor

@chipbuster chipbuster left a comment

Choose a reason for hiding this comment

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

I'll want to check this on my Windows and MacOS machines to make sure nothing breaks, but I think this is in pretty good form at the minute. A few small tweaks.

src/modules/localip.rs Outdated Show resolved Hide resolved
src/modules/localip.rs Show resolved Hide resolved
Instead of the remote address of 8.8.8.8 (Google DNS) in the crate
local_ipaddress use a reserved IPv4 address, that should never be
assigned.
Also forward the underlying error on failure.

Supersedes: starship#4614
Copy link
Contributor

@chipbuster chipbuster left a comment

Choose a reason for hiding this comment

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

👍

@andytom andytom merged commit ddd54e9 into starship:master Dec 1, 2022
@andytom
Copy link
Member

andytom commented Dec 1, 2022

Thank you for your contribution @cgzones and thank you for reviewing @chipbuster.

@cgzones cgzones deleted the localip2 branch December 2, 2022 13:39
Indyandie pushed a commit to Indyandie/starship that referenced this pull request Jul 26, 2023
Instead of the remote address of 8.8.8.8 (Google DNS) in the crate
local_ipaddress use a reserved IPv4 address, that should never be
assigned.
Also forward the underlying error on failure.

Supersedes: starship#4614
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.

None yet

3 participants