Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Sources/Containerization/VmnetNetwork.swift
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,10 @@ public struct VmnetNetwork: Network {

private static func configurePrefixV6(_ config: vmnet_network_configuration_ref, prefixV6: CIDRv6) throws {
var p = in6_addr()
inet_pton(AF_INET6, prefixV6.lower.description, &p)
// `inet_pton` rejects a zone suffix, so render the network address on
// its own. A vmnet prefix is never link-scoped, so dropping the zone
// here loses nothing.
inet_pton(AF_INET6, IPv6Address(prefixV6.lower.value).description, &p)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@vyncint

Suggested change
// `inet_pton` rejects a zone suffix, so render the network address on
// its own. A vmnet prefix is never link-scoped, so dropping the zone
// here loses nothing.
inet_pton(AF_INET6, IPv6Address(prefixV6.lower.value).description, &p)
// `inet_pton` rejects a zone suffix, so render the network address on
// its own. A vmnet prefix is never link-scoped, so dropping the zone
// here loses nothing.
guard inet_pton(AF_INET6, IPv6Address(prefixV6.lower.value).description, &p) == 0 else {
throw ContainerizationError(...)
}

I'd prefer we be strict here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 9b9907d. configurePrefixV6 no longer strips the zone: it renders prefixV6.lower as-is and guards on the inet_pton result, so a zoned prefix is rejected instead of quietly coerced, and supplying a zoneless prefix is the caller's job.

One deviation from the suggestion — inet_pton returns 1 on success and 0 when the string doesn't parse, so the guard reads == 1. As written (== 0) it would have thrown on every valid prefix.

Checking the result also closes something older than this PR: it was discarded, so any address inet_pton refused left p zeroed and configured ::.

No call sites needed changing — every in-repo caller already passes a zoneless prefix (CIDRv6("fd00::/64") in the integration suite).


guard vmnet_network_configuration_set_ipv6_prefix(config, &p, prefixV6.prefix.length) == .VMNET_SUCCESS else {
throw ContainerizationError(.internalError, message: "failed to set IPv6 prefix \(prefixV6) for network")
Expand Down
6 changes: 3 additions & 3 deletions Sources/ContainerizationExtras/CIDR.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public enum CIDR: CustomStringConvertible, Equatable, Sendable, Hashable {
case (.v4(let addr, let prefix)):
return .v4(IPv4Address(addr.value & prefix.prefixMask32))
case (.v6(let addr, let prefix)):
return .v6(IPv6Address(addr.value & prefix.prefixMask128))
return .v6(IPv6Address(addr.value & prefix.prefixMask128, zone: addr.zone))
}
}

Expand All @@ -113,9 +113,9 @@ public enum CIDR: CustomStringConvertible, Equatable, Sendable, Hashable {
public func contains(_ ip: IPAddress) -> Bool {
switch (self, ip) {
case (.v4(let network, let prefix), .v4(let ip)):
return network.value == (ip.value & prefix.prefixMask32)
return (network.value & prefix.prefixMask32) == (ip.value & prefix.prefixMask32)
case (.v6(let network, let prefix), .v6(let ip)):
return (network.zone == ip.zone) && (network.value == (ip.value & prefix.prefixMask128))
return (network.zone == ip.zone) && ((network.value & prefix.prefixMask128) == (ip.value & prefix.prefixMask128))
default:
return false
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/ContainerizationExtras/CIDRv6.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public struct CIDRv6: CustomStringConvertible, Equatable, Sendable, Hashable {
/// The lowest address in this CIDR block
@inlinable
public var lower: IPv6Address {
IPv6Address(address.value & prefix.prefixMask128)
IPv6Address(address.value & prefix.prefixMask128, zone: address.zone)
}

/// The highest address in this CIDR block (broadcast address).
Expand Down
96 changes: 96 additions & 0 deletions Tests/ContainerizationExtrasTests/TestCIDR.swift
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,102 @@ struct TestCIDR {
#expect(!cidr.contains(.v6(ip)))
}

// MARK: - Wrapper Agrees With The Concrete Types

// `CIDR` keeps the address exactly as parsed, so a block written from a
// host address ("192.168.1.100/24") stores a value with host bits set.
// Containment has to mask both sides, the way `CIDRv4`/`CIDRv6` do, or the
// block reports that it excludes its own members.
@Test(arguments: [
("192.168.1.100/24", "192.168.1.100"),
("192.168.1.100/24", "192.168.1.0"),
("192.168.1.100/24", "192.168.1.255"),
("10.1.2.3/16", "10.1.99.99"),
("2001:db8::1234/64", "2001:db8::1"),
("2001:db8::1234/64", "2001:db8::1234"),
])
func testWrapperContainsMembersOfAHostAddressBlock(cidr: String, ip: String) throws {
let block = try CIDR(cidr)
#expect(block.contains(try IPAddress(ip)))
}

@Test(arguments: [
("192.168.1.100/24", "192.168.2.1"),
("10.1.2.3/16", "10.2.0.1"),
("2001:db8::1234/64", "2001:db9::1"),
])
func testWrapperStillExcludesNonMembers(cidr: String, ip: String) throws {
let block = try CIDR(cidr)
#expect(!block.contains(try IPAddress(ip)))
}

// The wrapper must never disagree with the type it wraps.
@Test(arguments: ["192.168.1.100/24", "10.1.2.3/16", "192.168.1.0/24", "1.2.3.4/32"])
func testWrapperMatchesCIDRv4(cidr: String) throws {
let wrapper = try CIDR(cidr)
let concrete = try CIDRv4(cidr)
for probe in ["192.168.1.100", "192.168.1.0", "10.1.99.99", "1.2.3.4", "8.8.8.8"] {
let ip = try IPv4Address(probe)
#expect(wrapper.contains(.v4(ip)) == concrete.contains(ip), "disagreed on \(probe) for \(cidr)")
}
#expect(wrapper.lower.description == concrete.lower.description)
#expect(wrapper.upper.description == concrete.upper.description)
}

@Test(arguments: ["2001:db8::1234/64", "fe80::1%eth0/64", "2001:db8::/32"])
func testWrapperMatchesCIDRv6(cidr: String) throws {
let wrapper = try CIDR(cidr)
let concrete = try CIDRv6(cidr)
for probe in ["2001:db8::1", "2001:db8::1234", "fe80::1%eth0", "fe80::1", "2001:db9::1"] {
let ip = try IPv6Address(probe)
#expect(wrapper.contains(.v6(ip)) == concrete.contains(ip), "disagreed on \(probe) for \(cidr)")
}
#expect(wrapper.lower.description == concrete.lower.description)
#expect(wrapper.upper.description == concrete.upper.description)
}

// MARK: - Zone Preservation in Bounds

// `contains` compares zones, so a bound that drops the zone is reported as
// outside its own block. Both bounds have to carry the address's zone for
// the block to contain them.
@Test(arguments: ["fe80::1%eth0/64", "fe80::1%eth0/128", "2001:db8::5%lo0/126"])
func testZonedBlockContainsItsOwnBounds(cidr: String) throws {
let block = try CIDRv6(cidr)
#expect(block.contains(block.lower))
#expect(block.contains(block.upper))
}

@Test(arguments: ["fe80::1%eth0/64", "2001:db8::5%lo0/126"])
func testBoundsAgreeOnZone(cidr: String) throws {
let block = try CIDRv6(cidr)
#expect(block.lower.zone == block.address.zone)
#expect(block.upper.zone == block.address.zone)
}

@Test func testZonedLowerRendersItsZone() throws {
let block = try CIDRv6("2001:db8::5%lo0/126")
#expect(block.lower.description == "2001:db8::4%lo0")
#expect(block.upper.description == "2001:db8::7%lo0")
}

// The same bounds live on the `CIDR` wrapper, which carries its own copy.
@Test func testZonedWrapperContainsItsOwnBounds() throws {
let block = try CIDR("fe80::1%eth0/64")
#expect(block.contains(block.lower))
#expect(block.contains(block.upper))
#expect(block.lower.description == "fe80::%eth0")
}

// An unzoned block must keep rendering without a zone suffix.
@Test func testUnzonedBoundsCarryNoZone() throws {
let block = try CIDRv6("2001:db8::5/126")
#expect(block.lower.zone == nil)
#expect(block.upper.zone == nil)
#expect(block.lower.description == "2001:db8::4")
#expect(try CIDR("2001:db8::5/126").lower.description == "2001:db8::4")
}

// MARK: - Range Constructor

@Test func testRangeConstructorFindsSmallestBlock() throws {
Expand Down