`Dir.readFileAlloc` returns `error.NotDir` but path is valid, correct and exists

Help
1

I got a State with an asset variable asset which holds an open handler to a generic asset directory, whose path is passed as CLI argument:

$ zig build run -- path/to/asset

The goal is to serve all files inside the asset directory.
My solution is the following:

pub fn main() !void {
    var args = std.process.args();
    const cmd = args.next() orelse unreachable;
    const path = args.next() orelse std.debug.panic("too few arguments\nusage: {s} GALLERY_PATH", .{cmd});
    if (args.next() != null) {
        std.debug.panic("too many arguments\nusage: {s} GALLERY_PATH", .{cmd});
    }
    var state = State.init(path);
    defer state.deinit();

    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    const allocator = gpa.allocator();

    var server = try httpz.Server(*State).init(allocator, .{ .port = 3000 }, &state);
    defer {
        server.stop();
        server.deinit();
    }
}

const State = struct {
    asset: *std.fs.Dir,

    const Self = @This();

    fn init(asset_path: []const u8) Self {
        if (std.fs.path.isAbsolute(asset_path)) {
            var dir = std.fs.openDirAbsolute(asset_path, .{}) catch |err| std.debug.panic("failed to open absolute gallery path {s}: {any}", .{ asset_path, err });
            return .{ .asset = &dir };
        } else {
            var dir = std.fs.cwd().openDir(asset_path, .{}) catch |err| std.debug.panic("failed to open relative gallery path {s}: {any}", .{ asset_path, err });
            return .{ .asset = &dir };
        }
    }

    fn deinit(self: Self) void {
        self.asset.close();
    }
};

    var router = try server.router(.{});
    router.get("/static/asset/:name", serve_gallery, .{});

    std.log.info("listening on http://127.0.0.1:3000", .{});
    try server.listen();
}

fn serve_gallery(state: *State, req: *httpz.Request, res: *httpz.Response) !void {
     // Get name of file to serve    
    const name = req.param("name") orelse {
        res.content_type = .TEXT;
        res.status = 400;
        res.body = "request misses file name";
        return;
    };
    res.content_type = httpz.ContentType.forExtension(std.fs.path.extension(name));
    res.body = state.asset.readFileAlloc(res.arena, name, std.math.maxInt(usize)) catch |err| switch (err) {
        error.FileNotFound => {
            res.content_type = .TEXT;
            res.status = 404;
            res.body = "file not found";
            return;
        },
        else => {
            std.log.err("failed to open requested file {s}: {any}", .{ name, err });
            res.content_type = .TEXT;
            res.status = 500;
            res.body = "failed to open requested file";
            return;
        },
    };
}

But request /static/asset/foo it just throws an failed to open requested file foo: error.NotDir.

Keep in mind:

  • path/to/asset exists.
  • I tried deleting and recreating multiple times the directory and its files.
  • name param is correct.
  • CLI argument is correct.
  • The same setup, with the same version of zig (0.15.0-dev.864+75d0ec9c0) and the same version of httpz (httpz-0.0.0-PNVzrE-8BgCUEVpFtYaUOFrWMqXHBu95eyIYRkXbyriy), pointing to the same asset directory works on another project.

Any suggestions?

2

The old thing of returning the address of a stack variable which then is destroyed at the end of the scope so youā€™re returning a dangling pointer. FWIW I think Zig really needs an error for this situation, even plain old C compilers warn about this nowadays.

Since std.fs.Dir is a simple file handle I would not store it by reference, but by value, e.g. change:

const State = struct {
    asset: *std.fs.Dir,
    ...

to:

const State = struct {
    asset: std.fs.Dir,
    ...

PS: in general itā€™s good practice to only use pointers when you absolutely need to, since Zig has no ownership and lifetime tracking, and that makes working with pointers a minefield - instead pass things around by value, or at least store them by value as much as possible.

3

I canā€™t believe it was a dangling pointer, thanks a lot!

Absolutely.

Do you have any idea why on the other project works? Itā€™s the exact same code.

4

Hello,
I would not go this far and say that you should avoid using pointers as much as you can. Pointers in Zig are actually pretty nice, there is a lot of safety baked into them.

You have pointer types that indicate to how many items the pointer points to and even have FAT poiners in terms of Slices. You also get nullability safety for them.

But yes, there 100% should be an error / warning for this and yes you still need to think of the ā€œlifetimeā€ of the data it points to.

1 Like
5

Not sure tbh, only explanation I have is that the data on the stack hasnā€™t been overwritten by something else, so the dangling pointer still accidentially points to the right thing. But OTH I would expect that in debug mode there would be a mechanism which protects from this situation (for instance by ā€˜clearingā€™ the stack frame before function exit), but I guess Iā€™m wrong.

2 Likes
6

Yeah, spatial vs temporal safety basically (e.g. Zig has spatial but no temporal safety in release-safe mode).

But it looks to me like this specific thing (returning address of data on the stack) is a quite common footgun, and might be fairly easy to fix (since as I mentioned, even C compilers have a warning for the most common cases - but it is probably a warning and not an error because it cannot be waterproof without the language becoming another Rust (or Go), and I guess the situation in Zig is the same as in C - but Zig allows no warnings so thereā€™s no room for situations where the compiler might suspect the code is wrong but canā€™t be 100% sure).

7

I heard that there was a PoC and some ideas how to tackle this isssue, but we will have to see.

1 Like
8

Relevant Issue:

1 Like
9

Side note: donā€™t do this:

Dir.openDir (and all other std.fs.Dir functions) can handle both relative and absolute paths just fine, so this will work with any path:

var dir = try std.fs.cwd().openDir(...);

Currently, in the case of openDirAbsolute, it literally just calls cwd().openDir:

(see here for full context about Absolute suffixed functions)

2 Likes
10

Thanks for pointing that out!

1 Like