Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aim to correct resMSendRC current implementation #22324

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Gealber
Copy link

@Gealber Gealber commented Dec 26, 2024

Zig version

$ zig version
0.14.0-dev.2563+af5e73172

OS

$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 22.04.5 LTS
Release:	22.04
Codename:	jammy

Issues that might be related

How to reproduce

src/main.zig

const std = @import("std");

test "resolve address" {
    const allocator = std.testing.allocator;
    const port = 80;

    {
        const result = std.net.getAddressList(allocator, "example.com", port) catch |err| {
            return err;
        };
        result.deinit();
   }
}

test output

$ zig test src/main.zig 
1/1 main.test.resolve address...FAIL (TemporaryNameServerFailure)
/home/gulolio/zig/lib/std/net.zig:63:9: 0x1052d15 in parseIp (test)
        return error.InvalidIPAddressFormat;
        ^
/home/gulolio/zig/lib/std/net.zig:93:32: 0x105323c in parseExpectingFamily (test)
            posix.AF.UNSPEC => return parseIp(name, port),
                               ^
/home/gulolio/zig/lib/std/net.zig:1474:48: 0x105b153 in linuxLookupNameFromDns (test)
    if (ap[0].len < 4 or (ap[0][3] & 15) == 2) return error.TemporaryNameServerFailure;
                                               ^
/home/gulolio/zig/lib/std/net.zig:1418:5: 0x105c5ae in linuxLookupNameFromDnsSearch (test)
    return linuxLookupNameFromDns(addrs, canon, name, family, rc, port);
    ^
/home/gulolio/zig/lib/std/net.zig:1057:17: 0x105d30e in linuxLookupName (test)
                try linuxLookupNameFromDnsSearch(addrs, canon, name, family, port);
                ^
/home/gulolio/zig/lib/std/net.zig:994:9: 0x104f9ad in getAddressList (test)
        try linuxLookupName(&lookup_addrs, &canon, name, family, .{ .NUMERICSERV = true }, port);
        ^
/home/gulolio/Documents/Zig/Practice/getaddr/src/main.zig:14:13: 0x104d9d5 in test.resolve address (test)
            return err;
            ^
0 passed; 0 skipped; 1 failed.
error: the following test command failed with exit code 1:
/home/gulolio/Documents/Zig/Practice/getaddr/.zig-cache/o/37acb9f2ed18bad6dd24884bd8e17b01/test --seed=0x196f4ae8

Expected behaviour

To be able to determine the IP address list correctly without TemporaryNameServerFailure error.

Solution

Firstly let me clarify that my experience with zig is literally 4 days, so review the pull request carefully in case I made some mistake or there's a better or more idiomatic way to do this. Debugging the previous error I arrived to the conclusion that the main issue was on the method resMSendRC, looking at the definition of this method can be noticed that two distinct buffers are passed to store the answer of the queries, answers, and answer_bufs. Later these two buffers are are accessed and written indistinctly inside the outer: while tag. During the whole loop iterations the value of answer_bufs after the call to recvfrom(fd, answer_bufs[next], 0, &sa.any, &sl_copy) was always [::1]:53 while the amount of name servers retrieved was more than one.

I suspect the issue was in the allocation or the way the slices were passed, in some way the values of answer[next] remains the same during the iterations. To avoid that, I allocated a buffer inside this method and use that to store the value from recvfrom instead, and later copy that to the answers array. As I say before I'm not so familiar with zig so maybe here there's an error that can be fixed easily without this amount of code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant