Split the Address struct into an IpAddress and SocketAddress struct #2259
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,10 +60,10 @@ const Socket = struct { | |
| posix.close(self.socketID); | ||
| } | ||
|
|
||
| fn send(self: Socket, data: []const u8, destination: SocketAddress) void { | ||
| const addr = posix.sockaddr.in{ | ||
| .port = @byteSwap(destination.port), | ||
| .addr = destination.ip.address, | ||
| }; | ||
| if(builtin.os.tag == .windows) { // TODO: Upstream error, fix after next Zig update after #24466 is merged | ||
| const sendto = struct { | ||
|
|
@@ -83,7 +83,7 @@ const Socket = struct { | |
| } | ||
| } | ||
|
|
||
| fn receive(self: Socket, buffer: []u8, timeout: i32, resultAddress: *SocketAddress) ![]u8 { | ||
| if(builtin.os.tag == .windows) { // Of course Windows always has it's own special thing. | ||
| var pfd = [1]posix.pollfd{ | ||
| .{.fd = self.socketID, .events = std.c.POLL.RDNORM | std.c.POLL.RDBAND, .revents = undefined}, | ||
|
|
@@ -111,15 +111,15 @@ const Socket = struct { | |
| var addr: posix.sockaddr.in = undefined; | ||
| var addrLen: posix.socklen_t = @sizeOf(posix.sockaddr.in); | ||
| const length = try posix.recvfrom(self.socketID, buffer, 0, @ptrCast(&addr), &addrLen); | ||
| resultAddress.ip = .{.address = addr.addr}; | ||
| resultAddress.port = @byteSwap(addr.port); | ||
| return buffer[0..length]; | ||
| } | ||
|
|
||
| fn resolveIP(addr: []const u8) !IpAddress { | ||
| const list = try std.net.getAddressList(main.stackAllocator.allocator, addr, settings.defaultPort); | ||
| defer list.deinit(); | ||
| return .{.address = list.addrs[0].in.sa.addr}; | ||
| } | ||
|
|
||
| fn getPort(self: Socket) !u16 { | ||
|
|
@@ -145,24 +145,62 @@ pub fn init() void { | |
| } | ||
| } | ||
|
|
||
| pub const IpAddress = struct { | ||
| address: u32, | ||
|
|
||
| pub const localHost = IpAddress{.address = 0x0100007f}; | ||
|
|
||
| pub fn format(self: IpAddress, writer: anytype) !void { | ||
| try writer.print("{}.{}.{}.{}", .{self.address & 255, self.address >> 8 & 255, self.address >> 16 & 255, self.address >> 24}); | ||
| } | ||
|
|
||
| pub fn parse(string: []const u8) ?IpAddress { | ||
| var parts = std.mem.splitScalar(u8, string, '.'); | ||
| var address: u32 = 0; | ||
| while(parts.next()) |part| { | ||
| const octet = std.fmt.parseInt(u8, part, 10) catch return null; | ||
| address >>= 8; | ||
| if(address >> 24 > 0) return null; | ||
| address |= @as(u32, octet) << 24; | ||
| } | ||
| return .{.address = address}; | ||
IntegratedQuantum marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| }; | ||
|
|
||
| pub const SocketAddress = struct { | ||
IntegratedQuantum marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ip: IpAddress, | ||
| port: u16, | ||
| isSymmetricNAT: bool = false, | ||
|
|
||
| pub fn format(self: SocketAddress, writer: anytype) !void { | ||
| if(self.isSymmetricNAT) { | ||
| try writer.print("{f}:?{}", .{self.ip, self.port}); | ||
| } else { | ||
| try writer.print("{f}:{}", .{self.ip, self.port}); | ||
| } | ||
| } | ||
|
|
||
| pub fn parse(string: []const u8) ?SocketAddress { | ||
codemob-dev marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| var parts = std.mem.splitScalar(u8, string, ':'); | ||
| const ipString = parts.next() orelse return null; | ||
| var portString = parts.next() orelse return null; | ||
| if(parts.next() != null) return null; | ||
| var isSymmetricNAT = false; | ||
| if(portString.len == 0) return null; | ||
| if(portString[0] == '?') { | ||
| isSymmetricNAT = true; | ||
| portString = portString[1..]; | ||
| } | ||
| return .{ | ||
| .ip = IpAddress.parse(ipString) orelse return null, | ||
| .isSymmetricNAT = isSymmetricNAT, | ||
| .port = std.fmt.parseInt(u16, portString, 10) catch return null, | ||
| }; | ||
| } | ||
| }; | ||
|
|
||
| const Request = struct { | ||
| address: SocketAddress, | ||
| data: []const u8, | ||
| requestNotifier: std.Thread.Condition = std.Thread.Condition{}, | ||
| }; | ||
|
|
@@ -272,8 +310,8 @@ const STUN = struct { // MARK: STUN | |
| const XOR_MAPPED_ADDRESS: u16 = 0x0020; | ||
| const MAGIC_COOKIE = [_]u8{0x21, 0x12, 0xA4, 0x42}; | ||
|
|
||
| fn requestAddress(connection: *ConnectionManager) SocketAddress { | ||
| var oldAddress: ?SocketAddress = null; | ||
| var seed: [std.Random.DefaultCsprng.secret_seed_length]u8 = @splat(0); | ||
| std.mem.writeInt(i128, seed[0..16], std.time.nanoTimestamp(), builtin.cpu.arch.endian()); // Not the best seed, but it's not that important. | ||
| var random = std.Random.DefaultCsprng.init(seed); | ||
|
|
@@ -290,7 +328,7 @@ const STUN = struct { // MARK: STUN | |
|
|
||
| var splitter = std.mem.splitScalar(u8, server, ':'); | ||
| const ip = splitter.first(); | ||
| const serverAddress = SocketAddress{ | ||
| .ip = Socket.resolveIP(ip) catch |err| { | ||
| std.log.warn("Cannot resolve stun server address: {s}, error: {s}", .{ip, @errorName(err)}); | ||
| continue; | ||
|
|
@@ -309,7 +347,7 @@ const STUN = struct { // MARK: STUN | |
| }; | ||
| if(oldAddress) |other| { | ||
| std.log.info("{f}", .{result}); | ||
| if(other.ip.address == result.ip.address and other.port == result.port) { | ||
| return result; | ||
| } else { | ||
| result.isSymmetricNAT = true; | ||
|
|
@@ -322,10 +360,10 @@ const STUN = struct { // MARK: STUN | |
| std.log.warn("Couldn't reach STUN server: {s}", .{server}); | ||
| } | ||
| } | ||
| return SocketAddress{.ip = Socket.resolveIP("127.0.0.1") catch unreachable, .port = settings.defaultPort}; // TODO: Return ip address in LAN. | ||
| } | ||
|
|
||
| fn findIPPort(_data: []const u8) !SocketAddress { | ||
| var data = _data[20..]; // Skip the header. | ||
| while(data.len > 0) { | ||
| const typ = std.mem.readInt(u16, data[0..2], .big); | ||
|
|
@@ -345,9 +383,9 @@ const STUN = struct { // MARK: STUN | |
| addressData[4] ^= MAGIC_COOKIE[2]; | ||
| addressData[5] ^= MAGIC_COOKIE[3]; | ||
| } | ||
| return SocketAddress{ | ||
| .port = std.mem.readInt(u16, addressData[0..2], .big), | ||
| .ip = .{.address = std.mem.readInt(u32, addressData[2..6], builtin.cpu.arch.endian())}, // Needs to stay in big endian → native. | ||
| }; | ||
| } else if(data[1] == 0x02) { | ||
| data = data[(len + 3) & ~@as(usize, 3) ..]; // Pad to 32 Bit. | ||
|
|
@@ -380,7 +418,7 @@ pub const ConnectionManager = struct { // MARK: ConnectionManager | |
| socket: Socket = undefined, | ||
| thread: std.Thread = undefined, | ||
| threadId: std.Thread.Id = undefined, | ||
| externalAddress: SocketAddress = undefined, | ||
| online: Atomic(bool) = .init(false), | ||
| running: Atomic(bool) = .init(true), | ||
|
|
||
|
|
@@ -401,7 +439,7 @@ pub const ConnectionManager = struct { // MARK: ConnectionManager | |
|
|
||
| const PacketSendRequest = struct { | ||
| data: []const u8, | ||
| target: SocketAddress, | ||
| time: i64, | ||
|
|
||
| fn compare(_: void, a: PacketSendRequest, b: PacketSendRequest) std.math.Order { | ||
|
|
@@ -464,7 +502,7 @@ pub const ConnectionManager = struct { // MARK: ConnectionManager | |
| } | ||
| } | ||
|
|
||
| pub fn send(self: *ConnectionManager, data: []const u8, target: SocketAddress, nanoTime: ?i64) void { | ||
| if(nanoTime) |time| { | ||
| self.mutex.lock(); | ||
| defer self.mutex.unlock(); | ||
|
|
@@ -478,7 +516,7 @@ pub const ConnectionManager = struct { // MARK: ConnectionManager | |
| } | ||
| } | ||
|
|
||
| pub fn sendRequest(self: *ConnectionManager, allocator: NeverFailingAllocator, data: []const u8, target: SocketAddress, timeout_ns: u64) ?[]const u8 { | ||
| self.socket.send(data, target); | ||
| var request = Request{.address = target, .data = data}; | ||
| { | ||
|
|
@@ -514,7 +552,7 @@ pub const ConnectionManager = struct { // MARK: ConnectionManager | |
| self.mutex.lock(); | ||
| defer self.mutex.unlock(); | ||
| for(self.connections.items) |other| { | ||
| if(other.remoteAddress.ip.address == conn.remoteAddress.ip.address and other.remoteAddress.port == conn.remoteAddress.port) return error.AlreadyConnected; | ||
| } | ||
| self.connections.append(conn); | ||
| } | ||
|
|
@@ -538,12 +576,12 @@ pub const ConnectionManager = struct { // MARK: ConnectionManager | |
| } | ||
| } | ||
|
|
||
| fn onReceive(self: *ConnectionManager, data: []const u8, source: SocketAddress) void { | ||
| std.debug.assert(self.threadId == std.Thread.getCurrentId()); | ||
| self.mutex.lock(); | ||
|
|
||
| for(self.connections.items) |conn| { | ||
| if(conn.remoteAddress.ip.address == source.ip.address) { | ||
| if(conn.bruteforcingPort) { | ||
| conn.remoteAddress.port = source.port; | ||
| conn.bruteforcingPort = false; | ||
|
|
@@ -559,15 +597,15 @@ pub const ConnectionManager = struct { // MARK: ConnectionManager | |
| defer self.mutex.unlock(); | ||
| // Check if it's part of an active request: | ||
| for(self.requests.items) |request| { | ||
| if(request.address.ip.address == source.ip.address and request.address.port == source.port) { | ||
| request.data = main.globalAllocator.dupe(u8, data); | ||
| request.requestNotifier.signal(); | ||
| return; | ||
| } | ||
| } | ||
| if(self.online.load(.acquire) and source.ip.address == self.externalAddress.ip.address and source.port == self.externalAddress.port) return; | ||
| } | ||
| if(self.allowNewConnections.load(.monotonic) or source.ip.address == IpAddress.localHost.address) { | ||
| if(data.len != 0 and data[0] == @intFromEnum(Connection.ChannelId.init)) { | ||
| const ip = std.fmt.allocPrint(main.stackAllocator.allocator, "{f}", .{source}) catch unreachable; | ||
| defer main.stackAllocator.free(ip); | ||
|
|
@@ -593,7 +631,7 @@ pub const ConnectionManager = struct { // MARK: ConnectionManager | |
| while(self.running.load(.monotonic)) { | ||
| main.heap.GarbageCollection.syncPoint(); | ||
| self.waitingToFinishReceive.broadcast(); | ||
| var source: SocketAddress = undefined; | ||
| if(self.socket.receive(&self.receiveBuffer, 1, &source)) |data| { | ||
| self.onReceive(data, source); | ||
| } else |err| { | ||
|
|
@@ -1891,7 +1929,7 @@ pub const Connection = struct { // MARK: Connection | |
| manager: *ConnectionManager, | ||
| user: ?*main.server.User, | ||
|
|
||
| remoteAddress: SocketAddress, | ||
| bruteforcingPort: bool = false, | ||
| bruteForcedPortRange: u16 = 0, | ||
|
|
||
|
|
@@ -2342,3 +2380,41 @@ const ProtocolTask = struct { | |
| main.globalAllocator.destroy(self); | ||
| } | ||
| }; | ||
|
|
||
| test "Parse address" { | ||
|
Member
There was a problem hiding this comment. Please also add a test for resolve
Contributor
Author
There was a problem hiding this comment. How would I do that? Domain names can change IPs, should I just test to make sure that the parsing functions match the resolve functions for IPs?
Member
There was a problem hiding this comment. Not all of them. localhost is one such example
Contributor
Author
There was a problem hiding this comment.
Depending on the computer's config it might resolve differently.
Member
There was a problem hiding this comment. Yeah, I could edit it in
Collaborator
There was a problem hiding this comment.
Yeah it would be great to test if Cubyz works w/o internet connection right? But most of the time we care about name resolution in the context of working with an internet connection, so I am proposing positive test cases first.
Member
There was a problem hiding this comment. You should be able to run all test cases without an internet connection.
Collaborator
There was a problem hiding this comment. Test cases should just mark themselves as skipped if they can't be runner in current environment. With your requirement you will never be able to test anything platform specific, as you can't be running both windows and Linux as host os at once. Test cases may not be ran every time, not all at once, but they have to be checked at least in CI. It's normal, it's everywhere, let's not reinvent the wheel.
Member
There was a problem hiding this comment. That would be good, yes. But I would still rather avoid adding another dependency on a random service. We haven't used google for anything, so why add a dependency on it for some stupid test case?
Member
There was a problem hiding this comment. Plus any online connection increases the chance sporadic CI failures. |
||
| const address = IpAddress.parse("127.0.0.1").?; | ||
|
|
||
| try std.testing.expectEqual(IpAddress.localHost, address); | ||
| const socketAddress = SocketAddress.parse("127.0.0.1:1234").?; | ||
| try std.testing.expectEqual(SocketAddress{.ip = IpAddress.localHost, .port = 1234}, socketAddress); | ||
codemob-dev marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| const symmetricSocketAddress = SocketAddress.parse("127.0.0.1:?1234").?; | ||
| try std.testing.expectEqual(SocketAddress{.ip = IpAddress.localHost, .isSymmetricNAT = true, .port = 1234}, symmetricSocketAddress); | ||
| } | ||
|
|
||
| test "Format address" { | ||
| const addresses: [4][]const u8 = .{ | ||
| "127.0.0.1", | ||
| "123.1.111.222", | ||
| "1.1.1.1", | ||
| "0.0.0.0", | ||
| }; | ||
| for(addresses) |addressStr| { | ||
| const address = IpAddress.parse(addressStr).?; | ||
| const reformattedAddress = std.fmt.allocPrint(main.heap.testingAllocator.allocator, "{f}", .{address}) catch unreachable; | ||
| defer main.heap.testingAllocator.free(reformattedAddress); | ||
| try std.testing.expectEqualStrings(addressStr, reformattedAddress); | ||
| } | ||
| const socketAddresses: [4][]const u8 = .{ | ||
| "127.0.0.1:1234", | ||
| "123.1.111.222:?11111", | ||
| "1.1.1.1:255", | ||
| "0.0.0.0:?3333", | ||
| }; | ||
| for(socketAddresses) |addressStr| { | ||
| const address = SocketAddress.parse(addressStr).?; | ||
| const reformattedAddress = std.fmt.allocPrint(main.heap.testingAllocator.allocator, "{f}", .{address}) catch unreachable; | ||
| defer main.heap.testingAllocator.free(reformattedAddress); | ||
| try std.testing.expectEqualStrings(addressStr, reformattedAddress); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.