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

The core function Directory::GetFiles2 (documentserver-generate-allfonts) does not handle symbolic links #1859

Open
1 task done
emmanuelrosa opened this issue Aug 10, 2022 · 8 comments
Labels
confirmed-bug Issues with confirmed bugs

Comments

@emmanuelrosa
Copy link

This issue is unique.

  • I have used the search tool and did not find an issue describing my bug.

Operating System

Linux (DEB package)

Version information

7.1

Expected Behavior

I expected to be able to use my system fonts when creating/viewing a document.

Actual Behavior

I use NixOS, a Linux distribution, as my operating system. We have a package for ONLYOFFICE, using the dep package. On NixOS ONLYOFFICE cannot find any installed system fonts.

Based on my source code research, ONLYOFFICE reads system fonts from /usr/share/fonts. However, it does not handle the case when /usr/share/fonts, or any files within, are symbolic links (symlinks).

NixOS doesn't have /usr/share/fonts, but I have a fix which creates it as a symlink which points to a directory which contains symlinks to each system font file. I don't want to get distracted on why symlinks are being used. Let's just say, that's how NixOS works.

The root of the issue is this code in the core repo.

GetFiles2() is used when reading /usr/share/fonts, however the "if block" which checks the d_type does not check if the file is a symlink. Therefore, if the font directory or the font files are symlinks, which is 100% valid on Linux, ONLYOFFICE ignores those files/directories.

Reproduction Steps

  1. Create a symlink to a font in /usr/share/fonts
  2. Check to see if ONLYOFFICE can access the font.

Additional information

The code in question is this:

                int nType = 0;
                if(DT_REG == dirp->d_type)
                    nType = 2;
                else if (DT_DIR == dirp->d_type)
                    nType = 1;
                else if (DT_UNKNOWN == dirp->d_type)
                {
                     // XFS problem
                     struct stat buff;
                     std::string sTmp = std::string((char*)pUtf8) + "/" + std::string(dirp->d_name);
                     stat(sTmp.c_str(), &buff);
                     if (S_ISREG(buff.st_mode))
                        nType = 2;
                     else if (S_ISDIR(buff.st_mode))
                        nType = 1;
                }

You can use stat(() with S_ISREG() to handle symlinks, since stat() will operate on the file the symlink points to. You may even be able to simply change one line of code: else if (DT_UNKNOWN == dirp->d_type || DT_LNK == dirp->d_type )

@ShockwaveNN
Copy link
Contributor

Thanks for the very detailed report, I can confirm this issue, I've created issue 58490 in our private issue tracker

Also, this problem is reproducible in DocumentServer, so I'll move it to its repo since it's our base product

@ShockwaveNN ShockwaveNN transferred this issue from ONLYOFFICE/DesktopEditors Aug 11, 2022
@ShockwaveNN ShockwaveNN changed the title The core function Directory::GetFiles2 does not handle symbolic links The core function Directory::GetFiles2 (documentserver-generate-allfonts) does not handle symbolic links Aug 11, 2022
@ShockwaveNN ShockwaveNN added the confirmed-bug Issues with confirmed bugs label Aug 11, 2022
@heinwol
Copy link

heinwol commented Apr 13, 2023

Hello there! I'm sorry for disturbance, but this issue is quite troublesome for us NixOS users. So, if I may ask, what's the state of the issue as of today?

@emmanuelrosa
Copy link
Author

@heinwol I discovered this issue while researching why ONLYOFFICE doesn't have access to the system-wide fonts on NixOS. However, this issue is not the root cause.

ONLYOFFICE traverses /usr/share/fonts to find fonts, but NixOS doesn't store fonts there. Like other Linux distros, NixOS uses fontconfig to install system-wide fonts. fontconfig is effectively a font database for Linux; Administrators use it to install fonts (often indirectly) and applications use the fontconfig library to query for fonts. fontconfig replaces the old practice of installing fonts in /usr/share/fonts, although distros may still store fonts there.

NixOS uses fontconfig exclusively and does not provide the /usr/share/fonts directory. If ONLYOFFICE supports fontconfig in the future, then it will be able to find the system-wide fonts on NixOS (and other Linux distros as well).

In the meantime, I wrote a NixOS module which runs ONLYOFFICE in a FHS environment so that it can find the system fonts. See https://github.com/emmanuelrosa/erosanix/blob/6a2a83b2f0a6a3646bbe3f079a5fa1aad7e71d9f/modules/onlyoffice.nix

One thing I know that doesn't work is opening files from outside of ONLYOFFICE, such as with a file browser. But if you launch ONLYOFFICE from the "application menu" then it works.

@spikespaz
Copy link

Hello, please fix this. Terrible to not use fontconfig, it is standard for good reasons.

@mrtnvgr
Copy link

mrtnvgr commented Jan 14, 2024

+1

@emmanuelrosa
Copy link
Author

I've considered submitting a PR, but the ONLYOFFICE code base is too massive for me to build. Instead, I'll add more detail.

GetFiles2() is a generic directory-traversal function, so it must not delegate to fontconfig. The fontconfig code should go here: https://github.com/ONLYOFFICE/core/blob/master/DesktopEditor/fontengine/ApplicationFonts.cpp#L1799

That's the Linux code which currently calls GetFiles2() with various font directories.

The best example of how to implement a font search on Linux using fontconfig is the utility fc-list (https://gitlab.freedesktop.org/fontconfig/fontconfig/-/blob/main/fc-list/fc-list.c). fc-list uses the fontconfig library to retrieve a list of all registered fonts. It then traverses the list and displays the font names. It should be possible to modify the Linux implementation of CApplicationFonts::GetSetupFontFiles() to retrieve the fonts using fontconfig, and then traverse the font list (FcFontList) to create the std::vector<std::wstring> array.

One issue I can think of is that I don't know how such a change would affect Flatpak. Currently, CApplicationFonts::GetSetupFontFiles() has /run/host/fonts as one of its font directories to search. That's the directory where Flatpak exposes the fonts installed on the system. Flatpak looks in /usr/share/fonts by default, but it also uses the font cache directories /var/cache/fontconfig and /usr/lib/fontconfig/cache. The fact that Flatpak looks at the fontconfig font cache suggests to me that Flatpak integrates with fontconfig, but I don't know the extent of such integration. This is all to say that while I have a high degree of confidence that fontconfig is the ideal way to search for fonts on Linux, I'm not sure if the DesktopEditor would be able to find fonts when it's running as a Flatpak application. I'm assuming that it will work, but I don't have any experience with Flatpak to know for certain.

@alexandru0-dev
Copy link

+1

@igwyd
Copy link
Member

igwyd commented Mar 12, 2024

Hello, I reminded the developers about this, if there are no difficulties, then you will waiting the fix in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs
Projects
None yet
Development

No branches or pull requests

7 participants