Repository: SerenityOS/serenity
File: Userland/Libraries/LibShell/Builtin.cpp (C++)

Code before:
    auto const requested_path_is_absolute = !new_path.is_empty() && new_path.starts_with('/');
    auto real_path_or_error = FileSystem::real_path(new_path);
    if (real_path_or_error.is_error()) {
        warnln("Invalid path '{}'", new_path);
        return 1;
    }
    auto real_path = real_path_or_error.release_value();

    if (cd_history.is_empty() || cd_history.last() != real_path)
        cd_history.enqueue(real_path);

    ByteString path_to_chdir;
    if (!requested_path_is_absolute)
        path_to_chdir = LexicalPath::relative_path(real_path, cwd);
    else
        path_to_chdir = real_path;
    char const* path = path_to_chdir.characters();

    Function<int()> do_chdir = [&] -> int {
        int rc = chdir(path);
        if (rc < 0) {
            if (errno == ENOTDIR) {
                warnln("Not a directory: {}", path);
            } else {
                // Resolve lexically to a relative path, then try again.
                auto new_path = LexicalPath::join(cwd, path_to_chdir).string();
                if (new_path != path_to_chdir && new_path != cwd) {
                    real_path = move(new_path);
                    path = real_path.characters();
                    return do_chdir();
                }
                warnln("chdir({}) failed: {}", path, strerror(errno));
            }
            return 1;
        }
        return 0;
    };
    if (do_chdir() != 0)
        return 1;

    setenv("OLDPWD", cwd.characters(), 1);
    cwd = move(real_path);
    setenv("PWD", cwd.characters(), 1);
    return 0;
}

ErrorOr<int> Shell::builtin_cdh(Main::Arguments arguments)
{
    int index = -1;

    Core::ArgsParser parser;

Review comment (bug):
I don't think we should do this if `path_to_chdir` is absolute.

If you do `cd /root` as a non-root user now, this code causes a stack overflow. (Don't know if (unbounded) recursion is better here tbh)

Code after:
    auto const requested_path_is_absolute = !new_path.is_empty() && new_path.starts_with('/');
    auto real_path_or_error = FileSystem::real_path(new_path);
    if (real_path_or_error.is_error()) {
        warnln("Invalid path '{}'", new_path);
        return 1;
    }
    auto real_path = real_path_or_error.release_value();

    if (cd_history.is_empty() || cd_history.last() != real_path)
        cd_history.enqueue(real_path);

    ByteString path_to_chdir;
    if (!requested_path_is_absolute)
        path_to_chdir = LexicalPath::relative_path(real_path, cwd);
    else
        path_to_chdir = real_path;
    char const* path = path_to_chdir.characters();

    Function<int(bool)> do_chdir = [&](bool tried_lexical_resolution) -> int {
        int rc = chdir(path);
        if (rc < 0) {
            if (errno == ENOTDIR) {
                warnln("Not a directory: {}", path);
            } else {
                if (!tried_lexical_resolution && !path_to_chdir.starts_with('/')) {
                    // Resolve lexically then try again.
                    auto new_path = LexicalPath::join(cwd, path_to_chdir).string();
                    if (new_path != path_to_chdir) {
                        real_path = move(new_path);
                        path = real_path.characters();
                        return do_chdir(true);
                    }
                }
                warnln("chdir({}) failed: {}", path, strerror(errno));
            }
            return 1;
        }
        return 0;
    };
    if (do_chdir(false) != 0)
        return 1;

    setenv("OLDPWD", cwd.characters(), 1);
    cwd = move(real_path);
    setenv("PWD", cwd.characters(), 1);
    return 0;
}

ErrorOr<int> Shell::builtin_cdh(Main::Arguments arguments)
{
    int index = -1;