-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
Opening corruped file leads to crash #838
Comments
@BPerlakiH Only explanation Insee is that the libzim exception is not catch properly |
I have added an extra catch for Although it is still crashing with this ZIM file:
@mgautierfr maybe you have an idea, what type is not yet caught on the reader side? |
I don't know. Are you sure that this is the place where you have to catch But anyway, |
Thank you @mgautierfr, I had a second look at it, and maybe this trace will give us some hint: The interesting parts are:
|
btw it fails at this entry:
|
@BPerlakiH The exception handling you have shown us seems to be only for getmetadata(). You should protect all accesses to the ZIM. I suspect the issue is there. |
@kelson42 @mgautierfr I am a bit lost at this... So far on the Apple reader side, we were not using the validation of the ZIM files at all. I tested how it would look with validation added to avoid opening corrupted files, well...
Do you think we should have a similar offset check in |
@BPerlakiH There is no ZIM validation at opening time. What we expect is that stdandart exception are caught and managed properly for any call impliying the libzim/libkiwix. |
@kelson42 the problem we have is that we already have a try catch where we are handling std::exception, but it is still crashing inside that, so it is not a std::exception. @mgautierfr I dig further into this, and this is what I have found, it stops on one of the
This is in GuardMalloc:
can we do a std::exception instead of the ASSERT? |
Do you have the call trace when the ASSERT is happening ? The ASSERT is good here, but it means that we don't call the method with the right arguments. @kelson42, Are we speaking of the same corrupted zim file than openzim/libzim#893 ? |
Yes |
This is the backtrace:
|
Yes, it is the same ZIM file, taken from: openzim/libzim#893 (comment) |
The very same crash scenario was visible also on |
@mgautierfr Any feedback? |
@mgautierfr We need your help |
@veloman-yunkan Can you please help here? |
Is the issue still present with latest |
@veloman-yunkan currently we are using: https://download.kiwix.org/release/libkiwix/libkiwix_xcframework-13.1.0-4.tar.gz Which is libzim 9.2.2 underneath. Is there a newer one we can test with ? |
@BPerlakiH Lets wait libkiwix14 release. |
@BPerlakiH Now that we use libkiwix14, do we still have this bug? |
See openzim/libzim#893, but with using latest libzim/libkiwix it should not #835
The text was updated successfully, but these errors were encountered: