Fix unknown acquisition link type

[hamazaspavetisyan]

Problem

Since 069ef6c ("Parse acquisition-link type in updateFromOpds()"), Book::updateFromOpds() reads the type of each acquisition link. It throws std::runtime_error("Unknown acquisition link type: …") for any type other than:

• application/x-zim
• application/metalink4+xml
• application/x-bittorrent
• application/x-magnet

Before that commit, the type was ignored. Nothing in libkiwix catches the exception, so a single link with an unknown type makes the whole OPDS feed unreadable:

Caller	Effect
kiwix-manage library.opds show	aborts (uncaught exception, exit 134)
kiwix-serve --library library.opds	catches it but rejects the whole library: refuses to start (exit 1), or keeps the old library on a --monitorLibrary reload
kiwix-desktop, online catalog (readOpds() inside QtConcurrent::run)	the exception ends up in a discarded QFuture: the remote library is left with only the books before the bad entry, and the catalog never finishes loading

This affects local library files, especially once OPDS is the only library format (#1377) and users write or convert such files by hand. It also affects remote catalogs: a catalog that starts using a new link type would break every client built with this code.

069ef6c is not in any release yet, so no released version is affected.

How to reproduce

<feed xmlns="http://www.w3.org/2005/Atom">
  <entry>
    <id>urn:uuid:repro-book</id>
    <title>Repro</title>
    <link rel="http://opds-spec.org/acquisition/open-access"
          type="application/octet-stream"
          href="https://example.com/repro.zim"/>
  </entry>
</feed>

kiwix-manage bad_type.opds show
# libc++abi: terminating due to uncaught exception of type std::runtime_error:
#   Unknown acquisition link type: application/octet-stream

Fix

In updateFromOpds(), the call to fromMimeTypeToLinkKind() for a non-empty type is wrapped in a try/catch. A link of an unknown type is now skipped, together with its length, and a warning is printed on stderr:

Book 'repro-book': ignoring acquisition link 'https://example.com/repro.zim': Unknown acquisition link type: application/octet-stream

The book and its other links are kept. fromMimeTypeToLinkKind() itself is unchanged.

With the reproduction file above:

• kiwix-manage … show lists the book (exit 0);
• kiwix-serve loads the library.

Tests

• BookTest.updateFromOPDSSkipsAcquisitionLinkOfUnknownTypeTest: an entry with a link of unknown type (and its own length), followed by a ZIM link and a local path. It checks:
  • the warning;
  • that the following links are still read;
  • that only the known links are stored;
  • that the size comes from the known link.
• ManagerTest.readOpdsKeepsBookWithAcquisitionLinkOfUnknownType: a two-book feed where one book has a link of unknown type. readOpds() succeeds and both books are loaded, with the affected book keeping its ZIM URL.

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.55%. Comparing base (e2c7dad) to head (3a079c9).

Files with missing lines	Patch %	Lines
src/book.cpp	66.66%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1385      +/-   ##
==========================================
+ Coverage   45.48%   45.55%   +0.07%     
==========================================
  Files          58       58              
  Lines        5189     5196       +7     
  Branches     2680     2685       +5     
==========================================
+ Hits         2360     2367       +7     
+ Misses       1027     1026       -1     
- Partials     1802     1803       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[veloman-yunkan]

LGTM, save for one issue in the new test.

[kelson42]

@hamazaspavetisyan I will merge this PR, but I'm really not convinced this is better with the PR than without.

Please open an issue systematically before coding a PR, then we decide first if this is an issue.

XML is machine readable and should be written by machine. The scenario you describe is not a scenario we have at Kiwix, therefore I don't understand why we spend ressources on it now.

The fact that kiwix/kiwix-tools#844 has not moved for a month and is not ready is the problem!

[rgaudin]

XML is machine readable and should be written by machine. The scenario you describe is not a scenario we have at Kiwix, therefore I don't understand why we spend ressources on it now.

Given we've just introduce several new type for links, this PR can be considered allowing future flexibility without crashing. Also, given we use OPDS because it's standard, yet extend it with our own stuff, it's only legitimate to assume another OPDS feed could include our links in order to pass its feed to kiwix-serve without having to remove its custom links because kiwix-serve would crash.