search

nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.25k stars 321 forks source link

fs: Accept relative paths in nxt_fs_mkdir_all() #1232

Closed alejandro-colomar closed 2 months ago

alejandro-colomar commented 2 months ago

v2 changes:

$ git range-diff master gh/relative relative 
1:  d5723a27 = 1:  d5723a27 fs: Accept relative paths in nxt_fs_mkdir_all()
2:  57c33dfb ! 2:  351573ee fs: Accept path names of length 1
    @@ src/nxt_fs.c: nxt_fs_mkdir_all(const u_char *dir, mode_t mode)
          dirlen = nxt_strlen(dir);

     -    nxt_assert(dirlen < PATH_MAX && dirlen > 1);
    -+    nxt_assert(dirlen < PATH_MAX);
    ++    nxt_assert(dirlen < PATH_MAX && dirlen > 0);

          dst = path;
          start = (char *) dir;
alejandro-colomar commented 2 months ago

Bogus ruby test.

ac000 commented 2 months ago

I'm not sure denying mkdir("", ...); is really worth worrying about, it just returns ENOENT... but i'm assuming it would be a simple

if (*dir == '\0')
    return NXT_ERROR;

type thing?

alejandro-colomar commented 2 months ago

I'm not sure denying mkdir("", ...); is really worth worrying about, it just returns ENOENT... but i'm assuming it would be a simple

if (*dir == '\0')
    return NXT_ERROR;

type thing?

If I remove that assert, the code will return NXT_OK, while mkdir(2) returns an error. I prefer reporting NXT_ERROR for "" (so far, agree with you), which I'll do by transforming the while into a do-while. But I need you to apply the other patches to avoid a conflict in the merge.

So yeah, I have plans for it. :-)

And the do-while would not only not introduce a branch, but it would skip a test in the first iteration.

alejandro-colomar commented 2 months ago

I'm not sure denying mkdir("", ...); is really worth worrying about, it just returns ENOENT... but i'm assuming it would be a simple

if (*dir == '\0')
    return NXT_ERROR;

type thing?

If I remove that assert, the code will return NXT_OK, while mkdir(2) returns an error. I prefer reporting NXT_ERROR for "" (so far, agree with you), which I'll do by transforming the while into a do-while. But I need you to apply the other patches to avoid a conflict in the merge.

So yeah, I have plans for it. :-)

And the do-while would not only not introduce a branch, but it would skip a test in the first iteration.

Ahh, no, the +1 won't allow me. Didn't think of it. So yeah, just keep the assert. I drop my plans for "". :)

alejandro-colomar commented 2 months ago

Closing in favour of https://github.com/nginx/unit/pull/1235