New logic for generating HTTP-redirects #827
Conversation
Before this fix the root URL for a book was assumed to resolve to the main page. This was not true for ZIM files containing an entry at an empty path or with a path equal to "/", resulting in issue #826. The logic behind this behaviour is found in `kiwix::getEntryFromPath()`. The fix to that issue is a little more general and will result in an HTTP redirect in any case where `kiwix::getEntryFromPath(zim, path)` returns an entry with a real path different from the requested one. In particular, this will affect the behaviour on ZIM files with the old namespace scheme, where the requested resource - if not found - is also looked up in the 'A', 'I', 'J', and/or '-' namespaces. Now instead of returning the contents of that other resource an HTTP redirect response will be sent.
Codecov ReportBase: 70.51% // Head: 70.51% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #827 +/- ##
=======================================
Coverage 70.51% 70.51%
=======================================
Files 53 53
Lines 3653 3653
Branches 2042 2042
=======================================
Hits 2576 2576
Misses 1075 1075
Partials 2 2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
For information, the test on urlStr.empty() was made when using "old" namespace schema.
At this time (and still with old zim files), the main entry returned by getEntryFromPath was directly the actual main page without a redirection. We had to do the redirection ourselves. And it was the only case where libzim returned a different entry than requested.
Such condition (urlStr != entry.getPath()) can only appear with old zim files
(It can appear also with main page in new zim file, but in this case entry is also a redirect).
So this condition is now somehow a compatibility feature with old zim files only. When (if) we stop support old files, we can remove it.
* [API Break] Remove wrapper around libzim (@mgautierfr #789) * Allow kiwix-serve to use custom resource files (@veloman-yunkan #779) * Properly handle searchProtocolPrefix when rendering search result (@veloman-yunkan #823) * Prevent search on multi language content (@veloman-yunkan #838) * Use new `zim::Archive::getMediaCount` from libzim (@mgautierfr #836) * Catalog: - Include tags in free text catalog search (@veloman-yunkan #802) - Illustration's url is based on book's uuid (@veloman-yunkan #804) - Cleanup of the opds-dumper (@veloman-yunkan #829) - Allow filtering of catalog content using multiple languages (@veloman-yunkan #841) - Make opds-dumper respect the namemapper (@mgautierfr #837) * Server: - Correctly handle `\` in suggestion json generation (@veloman-yunkan #843) - Better http caching (@veloman-yunkan #833) - Make `/suggest` endpoint thread-safe (@veloman-yunkan #834) - Better redirection of main page (@veloman-yunkan #827) - Remove jquery (@mgautierfr @juuz0 #796) - Better Viewer of zim content : . Introduce `/content` endpoints (@veloman-yunkan #806) . Switch to iframe based content viewer (@veloman-yunkan #716) - Optimised design of the welcome page: . Alignement (@juuz0 @kelson42 #786) . Exit download modal on pressing escape key (@Juzz0 #800) . Add favicon for different devices (@Juzz0 #805) . Fix auto hidding of the toolbar (@veloman-yunkan #821) . Allow user to filter books by tags in the front page (@juuz0 #711) * CI : - Trigger CI on pull_request (@kelson42 #791) - Drop Ubuntu Impish packaging (@legoktm #825) - Add Ubuntu Kinetic packaging (@legoktm #801) * Testing: - Test ICULanguageInfo (@veloman-yunkan #795) - Introduce fake `test` language to test i18n (@veloman-yunkan #848) * Fix documentation (@kelson42 #816) * Udpate translation (#787 #839 #847)
Fixes #826 (with some side-effects).
Before this fix the root URL for a book was assumed to resolve to the main page. This was not true for ZIM files containing an entry at an empty path or with a path equal to "/", leading to #826. The logic behind this behaviour is found in
kiwix::getEntryFromPath().The fix to that issue by this PR is a little more general and will result in an HTTP redirect in any case where
kiwix::getEntryFromPath(zim, path)returns an entry with a real path different from the requested one. In particular, this will affect the behaviour on ZIM files with the old namespace scheme, where the requested resource - if not found - is also looked up in the 'A', 'I', 'J', and/or '-' namespaces. Now instead of returning the contents of that other resource an HTTP redirect response will be sent.