Skip to content

Remove use of getcwd for resolving paths #242

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.

Sign up for GitHub

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

Merged
k0ekk0ek merged 2 commits into from
Nov 27, 2024
Merged

Remove use of getcwd for resolving paths #242

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4fcb99
Remove use of getcwd for resolving paths
k0ekk0ek Nov 26, 2024
2516b59
Process review comment by @dcepelik
k0ekk0ek Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 13 additions & 78 deletions src/zone.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,89 +115,28 @@ diagnostic_push()
msvc_diagnostic_ignored(4996)

#if _WIN32
nonnull_all
static bool is_separator(int c)
{
return c == '\\' || c == '/';
}

nonnull_all
static bool is_rooted(const char *s)
{
if ((s[0] >= 'A' && s[0] <= 'Z') || (s[0] >= 'a' && s[0] <= 'z'))
return s[1] == ':';
return false;
}

nonnull_all
static bool is_relative(const char *s)
{
// rooted paths can be relative, e.g. C:foo
if (is_rooted(s))
return !is_separator((unsigned char)s[2]);
if (is_separator((unsigned char)s[0]))
return !(s[1] == '?' || is_separator((unsigned char)s[1]));
return false;
}

// The Win32 API offers PathIsRelative, but it requires linking with shlwapi.
// Rewriting a relative path is not too complex, unlike correct conversion of
// Windows paths in general (https://googleprojectzero.blogspot.com/2016/02/).
// Rooted paths, relative or not, unc and extended paths are never resolved
// relative to the includer.
nonnull((2,3))
static int32_t resolve_path(
const char *includer, const char *include, char **path)
nonnull_all
static int32_t resolve_path(const char *include, char **path)
{
// support relative non-rooted paths only
if (*includer && is_relative(include) && !is_rooted(include)) {
assert(!is_relative(includer));
char buffer[16];
int length = snprintf(
buffer, sizeof(buffer), "%s/%s", includer, include);
if (length < 0)
return ZONE_OUT_OF_MEMORY;
char *absolute;
if (!(absolute = malloc(length + 1)))
return ZONE_OUT_OF_MEMORY;
(void)snprintf(
absolute, (size_t)length + 1, "%s/%s", includer, include);
*path = _fullpath(NULL, absolute, 0);
free(absolute);
} else {
*path = _fullpath(NULL, include, 0);
}

if (*path)
if ((*path = _fullpath(NULL, include, 0)))
return 0;
return (errno == ENOMEM) ? ZONE_OUT_OF_MEMORY : ZONE_NOT_A_FILE;
}
#else
nonnull_all
static int32_t resolve_path(
const char *includer, const char *include, char **path)
static int32_t resolve_path(const char *include, char **path)
{
char *resolved;
char buffer[PATH_MAX + 1];

if (*includer && *include != '/') {
assert(*includer == '/');
int length = snprintf(
buffer, sizeof(buffer), "%s/%s", includer, include);
if (length < 0)
return ZONE_OUT_OF_MEMORY;
char *absolute;
if (!(absolute = malloc((size_t)length + 1)))
return ZONE_OUT_OF_MEMORY;
(void)snprintf(
absolute, (size_t)length + 1, "%s/%s", includer, include);
resolved = realpath(absolute, buffer);
free(absolute);
} else {
resolved = realpath(include, buffer);
}
resolved = realpath(include, buffer);

if (!resolved)
if (!(resolved = realpath(include, buffer)))
return (errno == ENOMEM) ? ZONE_OUT_OF_MEMORY : ZONE_NOT_A_FILE;
Copy link

@dcepelik dcepelik Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is significant improvement over the original error handling. Just a small nitpick: this can still translate e.g. EACCESS to "no such file", which is probably going to be confusing. But I guess this is not something that can be fixed easily.

Copy link
Contributor Author

@k0ekk0ek k0ekk0ek Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be pretty easy to add an additional code, but I mainly wanted to distinguish between no memory being available and not being able to open the file figuring further distinctions wouldn't make all that much sense(?) As in, we don't check for them in NSD, but other users of the library might (though I'm not aware of any such users yet).

Copy link

@dcepelik dcepelik Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see :) that makes sense. Perhaps it's just the name that confused me (ZONE_NOT_A_FILE). Maybe something like "ZONE_OPEN_FAILED" would be more descriptive? Because that includes all the other cases :)

assert(resolved == buffer);
size_t length = strlen(buffer);
Expand Down Expand Up @@ -318,24 +257,20 @@ static int32_t open_file(
file->path[0] = '-';
file->path[1] = '\0';
} else {
/* The file is resolved relative to the working directory. */
char workdir[PATH_MAX];
#if _WIN32
if(!_getcwd(workdir, sizeof(workdir)))
return (void)close_file(parser, file), ZONE_NOT_A_FILE;
#else
if(!getcwd(workdir, sizeof(workdir)))
return (void)close_file(parser, file), ZONE_NOT_A_FILE;
#endif
if ((code = resolve_path(workdir, file->name, &file->path)))
// The file is resolved relative to the working directory. The absolute
// path is used to protect against recusive includes. Not for opening the
// file as file descriptors for pipes and sockets the entries will be
// symoblic links whose content is the file type with the inode.
// See NLnetLabs/nsd#380.
if ((code = resolve_path(file->name, &file->path)))
return (void)close_file(parser, file), code;
}

if(strcmp(file->path, "-") == 0) {
file->handle = stdin;
return 0;
} else {
if ((file->handle = fopen(file->path, "rb")))
if ((file->handle = fopen(file->name, "rb")))
return 0;
}

Expand Down
Loading