rust-pcap / pcap

Rust language pcap library
Apache License 2.0
594 stars 138 forks source link

On big-endian systems, test_address_ipv4 fails #347

Closed musicinmybrain closed 2 months ago

musicinmybrain commented 2 months ago

I’m testing an update of the rust-pcap package in Fedora Linux to 1.3.0, and I’m finding that one test fails on the s390x architecture, which is big-endian.

---- device::tests::test_address_ipv4 stdout ----
thread 'device::tests::test_address_ipv4' panicked at src/device.rs:644:9:
assertion `left == right` failed
  left: "66.0.0.10"
 right: "10.0.0.66"

failures:
    device::tests::test_address_ipv4

Clearly, the byte order is getting reversed at some point.

It looks like the set_addr() methods for IPv4 are designed to accept a u32 in network byte order, as the value is passed directly through to the underlying structure:

https://github.com/rust-pcap/pcap/blob/8b7bcfe12c79955c228e5657a1e63cdcce0af3f5/src/device.rs#L365-L367 https://github.com/rust-pcap/pcap/blob/8b7bcfe12c79955c228e5657a1e63cdcce0af3f5/src/device.rs#L379-L381

But in fn sockaddr_ipv4(), the default address is assigned from an integer literal whose byte representation depends on host endianness, and which has been chosen to have the expected value on little-endian systems:

https://github.com/rust-pcap/pcap/blob/8b7bcfe12c79955c228e5657a1e63cdcce0af3f5/src/device.rs#L391

decathorpe commented 2 months ago

Looks like the 0x4200000A literal in the test code just has the wrong byte order on big-endian systems.

musicinmybrain commented 2 months ago

Looks like the 0x4200000A literal in the test code just has the wrong byte order on big-endian systems.

I agree; the actual library code appears to be correct.

cuviper commented 2 months ago

I think the correct way to write such a literal is 0x0A000042.to_be(). This correctly represents the fact that little-endian needs a byte-swap to get in network order.

musicinmybrain commented 2 months ago

I think the correct way to write such a literal is 0x0A000042.to_be(). This correctly represents the fact that little-endian needs a byte-swap to get in network order.

That seems like a correct approach, although we need a concrete numeric type, like 0x0A000042_u32.to_be().

I’m testing this patch and will follow up with a PR tomorrow.

lysliu commented 2 months ago

Yes,this is a common scenario in big-endian

use std::net::Ipv4Addr;

fn main() {
    // Example IPv4 addresses
    let addr_big_endian = [0x12, 0x34, 0x56, 0x78];
    let addr_little_endian = [0x78, 0x56, 0x34, 0x12];

    // Build IPv4 address from big-endian byte array
    let ip_big_endian = Ipv4Addr::from_be_bytes(addr_big_endian);
    println!("IPv4 big-endian address: {}", ip_big_endian);

    // Build IPv4 address from little-endian byte array
    let ip_little_endian = Ipv4Addr::from_le_bytes(addr_little_endian);
    println!("IPv4 little-endian address: {}", ip_little_endian);
}