Macchina-CLI / libmacchina

A library providing access to all sorts of system information.
https://crates.io/crates/libmacchina
MIT License
68 stars 20 forks source link

Local IP showing v6 addr by default #115

Closed luckman212 closed 2 years ago

luckman212 commented 2 years ago

I think this commit https://github.com/Macchina-CLI/libmacchina/commit/31ed08b26b56cf977b36952cf85d6c2dd11c5bf6 might have introduced a problem on macOS. Now when I run macchina v6.0.5 it reports only the v6

CleanShot 2022-01-05 at 22 51 28@2x

I saw the if-addrs crate has an Ifv4Addr struct that looks like it could be more specific.. not quite sure how to implement though.

grtcdr commented 2 years ago

As far as I know (I'm no expert in networking), IPv6 covers a bajillion more IPs than IPv4, which means not all IPV6s addresses can be represented as IPv4 addresses. I don't think think we can safely convert to IPv4 (get_if_address doesn't even offer such a functionality).

This doesn't look like a problem per se, it might have something to do with the interface. Can you confirm this?

One more thing, we've only downgraded back to 0.6.7 because the crate is failing on Android.

What do you propose?

luckman212 commented 2 years ago

There's no way to "convert" an IPv6 into an IPv4. They are distinct and very different things.

The problem is the IP being reported is just the link-local one (non routable) and is probably not what the user wants. If an IPv4 is present, it should be preferred.

Or, better yet I would suggest separating the identifier into LocalIPv4 and LocalIPv6.

grtcdr commented 2 years ago

There isn't much I can do about this, unfortunately. Is this the case for all your interfaces?

luckman212 commented 2 years ago

Yes. I'm trying to understand where this info is even being sourced from. The dependencies are a bit over my head, it comes from here, no? (crate: if-addrs)?

https://github.com/Macchina-CLI/libmacchina/blob/7b22084245d6298ac53cd49c56944535ada87655/src/shared/mod.rs#L310

grtcdr commented 2 years ago

Yes, get_if_addrs returns a list of interfaces, if we get a match between the provided interface and one of those interfaces, we immediately return its IP address.

luckman212 commented 2 years ago

It returns a list of interfaces or IP addresses?

grtcdr commented 2 years ago

Interfaces.

https://github.com/Macchina-CLI/libmacchina/blob/7b22084245d6298ac53cd49c56944535ada87655/src/shared/mod.rs#L315

This returns the interface's IP.

luckman212 commented 2 years ago

I figured out a way to return only IPv4. It might not be the most efficient...

Test

Cargo.toml

[package]
name = "v4test"
version = "1.0.0"
edition = "2018"

[dependencies]
libmacchina = "6.0.2"
if-addrs = "0.7.0"

main.rs

use libmacchina::traits::ReadoutError;
use if_addrs::{get_if_addrs, IfAddr};

fn ipv4_address(ifname: &str) -> Result<String, ReadoutError> {
    for iface in get_if_addrs()?
        .into_iter()
        .filter(|i| !i.addr.is_loopback() && i.name.eq(ifname))
        .filter_map(|i| {
            if let IfAddr::V4(v4_addr) = i.addr {
                Some(v4_addr)
            } else {
                None
            }
        })
    {
        return Ok((iface.ip).to_string());
    }
    return Err(ReadoutError::Other(String::from(
        "Unable to get local IPv4 address.",
    )));
}

fn main() {
    let _ip = ipv4_address("en0").unwrap();
    println!("IPv4: {:#?}", _ip);
}
grtcdr commented 2 years ago

It might not be the most efficient

I can't determine its efficiency, but I think it's very readable and to the point. If this fixes your issue, I'm happy to accept a PR! (I can never get an IPv6 address to show up, for any interface)

grtcdr commented 2 years ago

Benchmarks do however show good performance, so no need to worry :)

luckman212 commented 2 years ago

@grtcdr Thanks, but not sure I'm up to the challenge of submitting a proper PR to this project. Hopefully someone who really knows their way around Rust can pick up the torch here.

grtcdr commented 2 years ago

I've included your code in https://github.com/Macchina-CLI/libmacchina/commit/0292436b02068936a32bec3c7587014d6789d159

Thank you for all your help :D

grtcdr commented 2 years ago

Hi! Sorry for the delay - macchina v6.0.6 has just been released, so hopefully that fixes this issue :)