Sigil crashes when loading file from the internet

[DiapDealer]

I'm guessing it starts to go sideways when the code after the getOpenFileName tries to extract directory information from the url with QFileInfo. One of the tree views in the SelectFiles.cpp dialog turns grey just before Sigil crashes. That could still all point to a bug in Qt6 related to native windows filedialogs, though. I'll try to do some testing this weekend.

[KevinH]

Originally Posted by DiapDealer

I'm guessing it starts to go sideways when the code after the getOpenFileName tries to extract directory information from the url with QFileInfo. One of the tree views in the SelectFiles.cpp dialog turns grey just before Sigil crashes. That could still all point to a bug in Qt6 related to native windows filedialogs, though. I'll try to do some testing this weekend.

So using a non-native QFileDialog box did not help?

[DiapDealer]

Originally Posted by KevinH

So using a non-native QFileDialog box did not help?

I haven't had a chance to try it yet. I'm away from my development machine and just walking the code to try and familiarize myself with what's happening where.

[KevinH]

Both the Mac non-native and the kde plasma supplied QFileDialog box handle the case of attempting an external file via a url while inside that QFileDialog static method. I would think the Windows version should do the same.

I have my fingers crossed!

[BeckyEbook]

I checked it.
Indeed, when changing the fragment in the BookBrowser::AddExisting() function:

#if defined(Q_OS_WIN32) || defined(Q_OS_MAC)
    options = options | QFileDialog::DontUseNativeDialog;
#endif

I get the following message.

[KevinH]

Perfect. That is the right thing to have happen here. So whatever bug is present is inside Qt6 where should properly handle excluding external file urls here. A user can of course use any web browser to download and store locally any number of files if they so desire.

Thank you!

[DiapDealer]

Great. Thanks Becky! Now I can return to my plan to watch baseball all weekend.

[Doitsu]

Originally Posted by KevinH

A user can of course use any web browser to download and store locally any number of files if they so desire.

Shameless plug: or they could use my PasteImage plugin and have it download the file and add it to the epub.
It didn't work for the fish image though, but it worked with pretty much any other Internet image.

[DNSB]

Thanks @BeckyEbook! That modification works here. I did notice the Open stays greyed out and I had to hit enter to try to load the file and get the error message.

[KevinH]

I am wondering is this is the only file dialog that has the bug that causes a crash when hand entering an external url.

I grepped the Sigil source for "getOpenFileNames" and only found this routine in BookBrowser that BeckyEbook provided a fix for above.

But if I instead grep "getOpenFileName" I get the following long list of places it is used in Sigil

kbhend@MacBook-Pro src % grep getOpenFileName `find ./ -name "*.cpp"` 
.//Misc/OpenExternally.cpp:    const QString selectedFile = QFileDialog::getOpenFileName(parent,
.//MainUI/MainWindow.cpp:        QString filename = QFileDialog::getOpenFileName(this,
.//Dialogs/SearchEditor.cpp:    QString filename = QFileDialog::getOpenFileName(this,
.//Dialogs/IndexEditor.cpp:    QString filename = QFileDialog::getOpenFileName(this,
.//Dialogs/ClipEditor.cpp:    QString filename = QFileDialog::getOpenFileName(this,
.//Dialogs/PreferenceWidgets/PluginWidget.cpp:    QString zippath = QFileDialog::getOpenFileName(this, 
.//Dialogs/PreferenceWidgets/PluginWidget.cpp:    QString name = QFileDialog::getOpenFileName(this, 
.//Dialogs/PreferenceWidgets/GeneralSettingsWidget.cpp:    QString xeditorPath = QFileDialog::getOpenFileName(this,
.//Dialogs/EmptyLayout.cpp:    QString inipath = QFileDialog::getOpenFileName(this, 
.//main.cpp:    QString filename = QFileDialog::getOpenFileName(0,

So will someone with access to Windows try one of the other getOpenFileName instances above and try hand typing in an external url and hitting open and verifying if any of these crash. If so, we may need to move to non-native QFileDialogs for Windows throughout the Sigil code. If not, we will have found and fixed the only occurrence.

Looking at that list, the easiest one to test might be to open Sigil Preferences and try typing in an external url to the xeditor path where typically a path to PageEdit is put and see what happens.

Thanks!

[BeckyEbook]

No crash, the link to the jpg file is inserted as a path.

[BeckyEbook]

I have to go to bed because it's almost 3 a.m. here.

But...
When trying to open an EPUB file from the web, e.g. https://github.com/Sigil-Ebook/Sigil...ork_rev15.epub, the file is first checked by the Utility::IsFileReadable function, which prevents a crash.

I suspect that this allows us to use the native window without any problems if we add this check to the BookBrowser::AddExisting() function as well.

I can only check it myself on Sunday, because tomorrow I have a day trip.

[PeterT]

I realize that this might be unimportant in the discussions of this particular URL but when I try a simple wget https://www.adfg.alaska.gov/static/f...ort_Fish.1.jpg I receive back a

<html><head><title>Request Rejected</title></head><body>The requested URL was rejected. Please consult with your administrator.<br><br>Your support ID is: < 6919071033828809802><br><br><a href='javascript:history.back();'>

[KevinH]

I do not quite understand. Note that getOpenFileNames is a different routine from getOpenFileName. The former crashes and the latter you showed does not.

So we need the QFileDialog::getOpenFileNames to get a selection of one or more files. On Windows before we get access to what the user entered (or browsed and selected) in that one routine, it crashes if any external url was input by the user.

So there is no way to check if the file is readable until the user enters the file name, right?

If the crash happens after that method returns (and not during), then yes we could check if the file was readable and prevent the error.

I have no access to test Windows myself until I rebuild my Windows 11 virtual machine.

So does the input of an external url in the getOpenFileNames method cause the crash before that routine exits when open is selected, or does the crash happen only after that routine returns and it gives us the url the user entered?

My understanding is that it is the getOpenGpFileNames routine that is somehow crashing before it returns. Is that correct?

Originally Posted by BeckyEbook

I have to go to bed because it's almost 3 a.m. here.

But...
When trying to open an EPUB file from the web, e.g. https://github.com/Sigil-Ebook/Sigil...ork_rev15.epub, the file is first checked by the Utility::IsFileReadable function, which prevents a crash.

I suspect that this allows us to use the native window without any problems if we add this check to the BookBrowser::AddExisting() function as well.

I can only check it myself on Sunday, because tomorrow I have a day trip.

[KevinH]

Originally Posted by PeterT

I realize that this might be unimportant in the discussions of this particular URL but when I try a simple wget https://www.adfg.alaska.gov/static/f...ort_Fish.1.jpg I receive back a

Interesting. Lots of websites seem to reject wget and curl requests that do not use https: and instead use http: and visa versa since there is no security.

It is my understanding the error in question happens when the user types in any external url, not just that particular one.