Reworked Crate to Hash Bit Instead of Hex Characters

[rvbcldud]

Introduction

I have been trying to use this neat crate as an IP generator in a project of mine and have run into a couple problems:

• Ipv4 compatibility is not functional (addressed in Changed Ipv4-compatible to Ipv4-mapped addresses #3)
• "Uneven" subnet masks are note applied properly (as documented in Netmask is not applied properly #4 and this issue)

The first of these has been resolved in #3 as said, on which this PR depends. The rest of this PR is dedicated to the latter problem.

Changes

Instead of using a hashing system that relies on masking out hex characters (which has its problems as documented in #4), I have ventured to convert all such use of hex characters to bit characters instead.

For example, instead of preserving the first 2 hex characters of an address that has a prefix length of 8, the first 8 bit characters are preserved.

This provides a much more accurate system of generation that I hope improves the popularity and usability of this crate.

[rvbcldud]

Update: It seems to be possible to generate the same IP given different keys due to the first line of the hash function

let len = len / 8;

Changing this to:

let len = (len / 8) + (len % 8 != 0)asusize;

may solve the problem.

Using this test case:

#[test]fntest_ipv4_extensive(){for s in0..1000{let net:IpNetwork = "10.0.0.3/25".parse().unwrap();let addr = crate::ip(&s.to_string(), net).unwrap();println!("addr: {addr:?}");assert!(net.0.contains(addr));}}

the previous change has been verified to work.

However, this causes the main tests to break because now we are generating a different string for those main cases as they expected before (in many cases, the wrong size too).

To remedy this, I propose we switch from a string-based system entirely and use u128s and bit masks. Do you approve this direction @rushmorem?

[rushmorem]

Are the generated IP addresses still in the same subnet @rvbcldud? The original way of generating IPv4 addresses used to work fine for both Rust and Go before Rust changed things around. I knew those changes would probably break this library but I hadn't had a chance to look into it.

It seems to be possible to generate the same IP given different keys

Given your test case, I think that sounds expected to me. 10.0.0.3/25 only has 126 hosts so collisions are inevitable given your inputs.

[rvbcldud]

The above test case generates 10.0.0.0 each time, actually. If it worked, there would likely be collisions for sure ... but not that every time ha

Moving to a bit system would simplify things a lot.

Thanks for the response and allowing me to contribute to your crate!

[rushmorem]

The above test case generates 10.0.0.0 each time, actually

Ah, I see!

but not that every time ha

😆

Moving to a bit system would simplify things a lot.

Yeah, that sounds good to me! As long as the generated IP addresses are within the expected subnet that's fine. We can adjust the spec to match the new way of doing things.

Thanks for the response and allowing me to contribute to your crate!

My pleasure! Thanks for taking time to contribute. I really appreciate it!

[rvbcldud]

I finished the proposed rework using u128s

When the user provides the max subnet mask, instead of throwing an error I provide the only available address. It will only throw an error if it is strictly greater than the max subnet mask. I hope you approve.

Otherwise, I am quite happy with this improvement. Let me know what you think! @rushmorem

[rvbcldud]

If you approve, I can make a PR for the original spec.

[rushmorem]

Thanks @rvbcldud! Would you mind taking care of the merge conflict?

[rvbcldud]

@rushmorem All done!

[rvbcldud]

I am working on rewriting the spec and notice that you opted to return an error if the prefix length is the max value. I changed it to the former option of returning the only address available as it makes more sense in terms of the API.

Just checking that you approve this change?

[rvbcldud]

Hey @rushmorem, I would love to see this merged when you have the time!

[rvbcldud]

@rushmorem Is there a reason this has not been merged? Thanks!