Open BPerlakiH opened 1 month ago
@BPerlakiH Which exception is thrown? And what exactly is wrong in the OPDS stream to trigger this exception?
I think that the bug description is not formulated correctly. I believe that it should rather state that Manager::readOpds()
is not fool-proof against malformed input with an example of such input provided. The fix should then come with one or more unit-tests covering such scenarios.
From the screenshot in https://github.com/kiwix/kiwix-apple/issues/851, it seems this is a allocation error (not enough memory ? to much memory allocated ?) inside icu73 (called by kiwix::removeAccents
).
This may be us doing strange things when trying to remove accents somehow.
But we need to fix this issue first. This is not related to this method always returning true
(even if we have to fix this anyway).
I have open the issue https://github.com/kiwix/libkiwix/issues/1101 about the memory allocation error in kiwix::removeAccents
.
I keep this issue open about the method always returning true.
@BPerlakiH Can you please open the corresponding issue lin this repo to clearly state what does not work?
Sorry, I also think that the description was misleading. I did update it, to reflect a more narrow scope: the actual return value of this specific libkiwix function, and how it is described in the compiled header file.
@veloman-yunkan @mgautierfr We need your feedback here. In a way or the other the libkiwix user would benefit to know if the parsing went well or not. The boolean returns seems appropriate for that and this is what the documentation implies. Documentation and testset around this function should probably be updated.
Currently the libkiwix function never returns false:
The code proves the opposite - Manager::readOpds()
returns false
in case of malformed (syntactically incorrect) XML. Yet, it does return true
if the XML content does not represent valid or reasonable OPDS data, but before fixing anything here we have to define how it should behave in those cases. For example, what should it return when
kiwix-server
;kiwix-server
that contains e.g. a string where a number is expected, or contains duplicate entries with the same book ID but different attributes (title, category, etc).
Updated:
On the Apple reader side, we would like to be rest assured, that when invalid data is passed into the
OPDSParser.parse(data:)
function, we can get afalse
value back, so we can handle it gracefully. We already had a unit test on the reader side, but so far it was disabled.Currently the libkiwix function never returns false: https://github.com/kiwix/libkiwix/blob/ece40966f1b568ac6ce06c52d146f9f408435810/src/manager.cpp#L167-L179
Whereas on compiled libkiwix header files, I see this information, which is misleading:
So we either need to return False in case of invalid content, or if that's not possible, update the header file to reflect this special case.