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

Rime plugin 需求可执行权限的必要性 #520

Closed
hosiet opened this issue Jan 16, 2022 · 3 comments
Closed

Rime plugin 需求可执行权限的必要性 #520

hosiet opened this issue Jan 16, 2022 · 3 comments
Assignees

Comments

@hosiet
Copy link
Contributor

hosiet commented Jan 16, 2022

(本 issue 来源于 #518 ,以及 libera.chat #fcitx 频道 2021-12-30 的讨论)

我在作为 Debian Developer 维护 Debian 中的 librime 时注意到以下代码:

const fs::perms exe_perms = (fs::owner_exe | fs::group_exe | fs::others_exe);
ModuleManager& mm = ModuleManager::instance();
if (!fs::is_directory(plugins_dir)) {
return;
}
LOG(INFO) << "loading plugins from " << plugins_dir;
for (fs::directory_iterator iter(plugins_dir), end; iter != end; ++iter) {
fs::path plugin_file = iter->path();
if (plugin_file.extension() == boost::dll::shared_library::suffix()) {
fs::file_status plugin_file_status = fs::status(plugin_file);
if (fs::is_regular_file(plugin_file_status) &&
fs::status(plugin_file).permissions() & exe_perms) {

如上所示,目前 librime 仅在 plugin 文件具有可执行权限时才尝试加载。但现代 Linux 系统中并无 shared library 一定具有可执行权限的规定。特别地对于 deb 系发行版,Debian Policy ( https://www.debian.org/doc/debian-policy/ch-sharedlibs.html ) 有 shared library 不应添加 executable bit 的规定:

Shared libraries should not be installed executable, since the dynamic linker does not require this and trying to execute a shared library usually results in a core dump.

当前 librime 明确检查插件文件是否可执行,这可能导致部分 Linux 发行版的插件无法得到识别。且如不考虑发行版问题,检查是否可执行的意义不明确,似乎是多此一举。如果检查插件是否可执行没有特别的意义,我建议采纳 #518 的修改。

参考:

@Arfrever
Copy link

Arfrever commented Jan 16, 2022

As original author of #431, maybe I thought that executable permission is needed (for plugins loaded by dlopen function which is internally used by boost::dll::shared_library), but apparently it may be dependent on operating system (according to discussion from above URL, executable permission would be needed on HP-UX operating system).

Instead of dropping check for permissions, it may be better to check for readable permission.

@lotem lotem self-assigned this Jan 26, 2022
@lotem
Copy link
Member

lotem commented Jan 26, 2022

Merged be0ea83

Thanks.

On checking readable permission:
Non-readable plugin executable shouldn't be considered a supported case. I prefer logging an error to silently skipping it.

@lotem lotem closed this as completed Jan 26, 2022
@Arfrever
Copy link

I also wonder if maybe it would make sense to ignore hidden files (whose names start with .):

    if (plugin_file.filename().string()[0] != '.' &&
        plugin_file.extension() == boost::dll::shared_library::suffix()) {

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

No branches or pull requests

3 participants