From d09a1026b74da68e40795f3bd630a0625b5935df Mon Sep 17 00:00:00 2001 From: Aditya Singh <60082699+adityasingh2400@users.noreply.github.com> Date: Tue, 26 May 2026 13:38:05 -0700 Subject: [PATCH] Fix incorrect byte shift in IPv4Address byte-array initializer (#754) The `IPv4Address(_ bytes: [UInt8])` initializer in ContainerizationExtras shifts the third octet by 16 bits instead of 8: ```swift self.value = (UInt32(bytes[0]) << 24) | (UInt32(bytes[1]) << 16) | (UInt32(bytes[2]) << 16) // should be << 8 | UInt32(bytes[3]) ``` Because `bytes[2]` lands in the same bit range as `bytes[1]`, the second octet gets corrupted by the OR, the third octet is dropped, and bits 8 through 15 are always left zero. Concretely, decoding `[192, 168, 1, 1]` yields `192.169.0.1` instead of `192.168.1.1`, and `[18, 52, 86, 120]` yields `18.118.0.120` instead of `18.52.86.120`. This went unnoticed because the `bytes` computed property getter uses the correct `>> 8` for the third octet, but there was no test exercising the byte-array initializer, so the encode and decode paths were never checked against each other. The sibling `IPv6Address(_ bytes:)` initializer uses the correct descending shifts (`<< 120, << 112, ... << 8, << 0`), which is what the IPv4 version should mirror. The fix changes the third octet shift to 8 bits so the initializer is the exact inverse of the `bytes` property. I also added two tests to the initializer suite: a valid-input test that asserts both the resulting `value` and that `init(bytes).bytes == bytes` round-trips, and an invalid-length test. The round-trip test fails on the current code and passes with the fix. Verification: `swift test --filter ContainerizationExtrasTests` passes 221 tests in 26 suites (the IPv4Address suite goes from 23 to 25 tests). The new round-trip test fails before the one-line change and passes after. Signed-off-by: Aditya Singh --- .../ContainerizationExtras/IPv4Address.swift | 2 +- .../TestIPv4Address.swift | 32 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/Sources/ContainerizationExtras/IPv4Address.swift b/Sources/ContainerizationExtras/IPv4Address.swift index 1aea7a01..eb7546b1 100644 --- a/Sources/ContainerizationExtras/IPv4Address.swift +++ b/Sources/ContainerizationExtras/IPv4Address.swift @@ -39,7 +39,7 @@ public struct IPv4Address: Sendable, Hashable, CustomStringConvertible, Equatabl self.value = (UInt32(bytes[0]) << 24) | (UInt32(bytes[1]) << 16) - | (UInt32(bytes[2]) << 16) + | (UInt32(bytes[2]) << 8) | UInt32(bytes[3]) } diff --git a/Tests/ContainerizationExtrasTests/TestIPv4Address.swift b/Tests/ContainerizationExtrasTests/TestIPv4Address.swift index 13fe9432..416f263a 100644 --- a/Tests/ContainerizationExtrasTests/TestIPv4Address.swift +++ b/Tests/ContainerizationExtrasTests/TestIPv4Address.swift @@ -85,6 +85,38 @@ struct IPv4AddressTests { try IPv4Address(invalidAddress) } } + + @Test( + "byte array initializer - valid addresses", + arguments: [ + ([UInt8(127), 0, 0, 1], UInt32(0x7F00_0001)), // localhost + ([UInt8(0), 0, 0, 0], UInt32(0x0000_0000)), // zero address + ([UInt8(255), 255, 255, 255], UInt32(0xFFFF_FFFF)), // broadcast + ([UInt8(192), 168, 1, 1], UInt32(0xC0A8_0101)), // private network + ([UInt8(0x12), 0x34, 0x56, 0x78], UInt32(0x1234_5678)), // byte order test + ] + ) + func testByteArrayInitializerValid(bytes: [UInt8], expectedValue: UInt32) throws { + let address = try IPv4Address(bytes) + #expect(address.value == expectedValue) + // The byte array initializer must round-trip with the bytes property. + #expect(address.bytes == bytes) + } + + @Test( + "byte array initializer - invalid lengths", + arguments: [ + [], // empty + [UInt8(1)], // too short + [UInt8(1), 2, 3], // too short + [UInt8(1), 2, 3, 4, 5], // too long + ] + ) + func testByteArrayInitializerInvalid(bytes: [UInt8]) { + #expect(throws: AddressError.self) { + try IPv4Address(bytes) + } + } } // MARK: - Property Tests