kiwix / kiwix-desktop

Kiwix for Windows and GNU/Linux desktops
https://download.kiwix.org/release/kiwix-desktop/
GNU General Public License v3.0
757 stars 101 forks source link

Failing Kiwix if started from Windows Network Drive (was: Kiwix does not start on Windows: "QT5Core.dll was not found") #885

Closed agentp2-2 closed 3 weeks ago

agentp2-2 commented 2 years ago

I noticed that Kiwix (version: kiwix-desktop_windows_x64_2.3.0) can't start properly from mapped network shares. I got "The code execution cannot proceed because QT5Core.dll was not found. Reinstalling the program may fix this problem." On regular drive everything works fine.

kelson42 commented 2 years ago

So where is the missing dll in your case? Please give the full path if possible.

agentp2-2 commented 2 years ago

.Dll is inside the Kiwix folder but when I'm trying to start Kiwix from network share I got the mentioned error.

For example: Regular drive (works fine): E:\kiwix-desktop_windows_x64_2.3.0\Qt5Core.dll

Mapped network share (got the error): Q:\kiwix-desktop_windows_x64_2.3.0\Qt5Core.dll

C:\Windows\system32>net use Nowe połączenia zostaną zapamiętane. Stan Lokalny Zdalny Sieć OK F: \Openmediavault\backups Microsoft Windows Network OK Q: \Openmediavault!share Microsoft Windows Network OK R: \10.248.248.20\public2 Microsoft Windows Network OK S: \Openmediavault\public Microsoft Windows Network OK \10.248.248.20\IPC$ Microsoft Windows Network Polecenie zostało wykonane pomyślnie.

kelson42 commented 2 years ago

@mgautierfr Any idea, sounds similar to #429 to me.

kelson42 commented 5 months ago

@agentp2-2 I can not reproduce the problem with latest dev version (nightly https://download.kiwix.org/nightly/2024-05-18/kiwix-desktop_windows_x64_2024-05-18.zip). Would you be able to check yourself?

But, even if Kiwix launched, we have another big problem:

It seems we are impacted now by https://bugreports.qt.io/browse/QTBUG-84632

kelson42 commented 4 months ago

@sgourdas Could you please have a look to that one please?

sgourdas commented 4 months ago

@kelson42 I have been able to reproduce both issues, namely:

Also, from what it seems based on the bugreports thread, Chrome is not supporting running from a network drive, based on the last comment from 2016:

This is working as intended. We do not support running Chrome from a network share because doing so would prevent enabling the sandbox.

Trying simulate it with the current Chrome portable, the application just auto exits.

How do we want to proceed with this?

kelson42 commented 4 months ago

@sgourdas We should test with Qt6

sgourdas commented 3 months ago

@kelson42 should I build kiwix-desktop.exe with QtCreator?

kelson42 commented 3 months ago

@kelson42 should I build kiwix-desktop.exe with QtCreator?

I don't understand how this question is related to the issue. Can you please explain?

sgourdas commented 3 months ago

I don't understand how this question is related to the issue. Can you please explain?

I am confused on the way I should go about testing with Qt6 to be honest... Can you provide me some guidance?

kelson42 commented 3 months ago

@sgourdas Everything should be in the README

sgourdas commented 3 months ago

@kelson42 if i am not mistaken, the README contains information on compiling for Linux only

kelson42 commented 3 months ago

@sgourdas Yes, I guess you will have ro reproduce the scenario on Linux.

sgourdas commented 3 months ago

@kelson42 in this case the issue is Windows specific, because of a stricter security around processes running from network locations. Linux cannot reproduce the issue regardless of Qt version AFAIK.

kelson42 commented 2 months ago

@sgourdas The problem seems to be the Chromium sandboxing which indeeds forbids to run from a network drive or as root. Have you tried to deactivate it? See https://doc.qt.io/qt-6/qtwebengine-platform-notes.html#sandboxing-support or https://codereview.qt-project.org/c/yocto/meta-boot2qt/+/232118

sgourdas commented 2 months ago

Yes, sandbox is the reason for these issues and disabling it would fix them.

Is that something that we want to do though? I am not sure of the implications.

kelson42 commented 2 months ago

@sgourdas We have security implications but I'm not sure exactly which one. Would that be possible to add an option on the command line to disable it? So people really wanting to start from network drive still can?

sgourdas commented 2 months ago

Definitely, will try to put it tonight.

kelson42 commented 2 months ago

@sgourdas I reopen the issue as we need to verify with the nightly that it works like expected. Please let us know the result of your check.

sgourdas commented 2 months ago

@kelson42 ok, thank you. It just seems that Windows kiwix-desktop nightly is broken since the 19th of July.

Other than that, current last commit of the PR is a POC so we will need another in any case (to add the prompt)

kelson42 commented 2 months ago

@mgautierfr Like explained last Monday, we really need the nightly for Windows to work.

Edit: maybe we will have a nightly this night because of this fix https://github.com/kiwix/kiwix-desktop/pull/1172

sgourdas commented 2 months ago

@kelson42 @mgautierfr nightly for today is not present still.

mgautierfr commented 2 months ago

This is because #1178 doesn't compile on Windows. See error log at https://ci.appveyor.com/project/Kiwix/kiwix-build/builds/50423338

src\main.cpp(29): error C2664: 'int MessageBoxA(HWND,LPCSTR,LPCSTR,UINT)': cannot convert argument 2 from 'QString' to 'LPCSTR'

Why not using QMessageBox instead of windows low level api MessageBoxA ?

sgourdas commented 2 months ago

This is because https://github.com/kiwix/kiwix-desktop/pull/1178 doesn't compile on Windows.

Wow, it is very weird that I did not see that

Why not using QMessageBox instead of windows low level api MessageBoxA ?

Reason is that we need to do the check before the QApplication is initialized. QMessageBox demands the qapp to be started before being used and its neither a good practice to use something like a dummy qapp.

sgourdas commented 1 month ago

@mgautierfr we are getting this on the build:

main.obj : error LNK2001: unresolved external symbol __imp_MessageBoxExW

Could we maybe link against User32.lib?

mgautierfr commented 1 month ago

Done with #1183

sgourdas commented 1 month ago

@mgautierfr sorry for the bother with this, but I am not able to understand why kiwix-desktop for Windows is not being built on nightly currently.

Edit: It is now available, thank you. I am going to test again.

sgourdas commented 1 month ago

@kelson42 we should reopen this as it is not working as intended, which is very strange because in my independent testing with the below code it seems to work.

#include <windows.h>
#include <string>
#include <iostream>

int main(int argc, char *argv[]) {
    std::string pather = "Z:\\path\\executable.exe";
    std::string driveLetter = pather.substr(0, 3);
    UINT driveType = GetDriveTypeA(driveLetter.c_str());
    if(driveType == DRIVE_REMOTE) {
        const wchar_t* message = L"Do you want to disable the sandbox?";
        const wchar_t* title = L"Kiwix Desktop";
        int msgboxID = MessageBoxExW(NULL, message, title, MB_YESNO | MB_ICONQUESTION, 0);
        if (msgboxID == IDYES) std::cout << "yes" << std::endl;
        else std::cout << "no" << std::endl;
    }
}

I have to look into this again to resolve once and for all.

Testing with the nightly, the behavior is very inconsistent (different when double clicking .exe and when running from cmd). In both cases the popup does not show for some reason.

sgourdas commented 4 weeks ago

When starting kiwix-desktop.exe from a network share the application does not show up at all. The task can be seen starting and failing -- seg fault based on exit code -1073741819 from powershell -- which ofc does not say much.

Reproducing the logic in an independent cpp app with the same code, behaves as expected.

I am blocked by kiwix/kiwix-build#758 so I cannot test further for now.

sgourdas commented 3 weeks ago

@kelson42 so obviously the issue was with using gt for the messages of the popup. The translations are initialized after the KiwixApp init, so they were invalid.

What I propose is to start the application with no sandbox and then notify the user that the application has been started with no sandbox. The user can then choose what to do with that information.

Do you agree with such an approach? This way we can avoid using User32.lib and not doing any workarounds with the KiwixApp initialization.

Adding @veloman-yunkan for visibility and his opinion.

sgourdas commented 3 weeks ago

I will make a proposal PR closer to the initially discussed logic. It will be disabling the sandbox and after launching the app asking the user if he wants to continue or quit. I think it is the best option.