Before this change `Downloader::startDownload()` might avoid starting a
new download when a download with the specified URI was already present
in its cache. This might be confusing for the following reasons:
1. uri is not the only parameter of `Downloader::startDownload()` - a
target download directory may also be specified through the second
`options` parameter. Thus calling `Downloader::startDownload()` twice
with the same URI but different download directories would not save
files into the second directory.
2. Files of a completed download may be removed, whereupon downloading the same
files again won't be a no-op. However in such a situation
`Downloader` refuses to actually repeat a previous download.
Now the search results page is presented by the backend in the language
controlled by the value of the `userlang` URL query parameter (or, if
the latter is missing, the value of the `Accept-Language:` HTTP header).
Note that the front-end doesn't yet take advantage of this
functionality.
But the question is do we need all of them to be translatable in the
frontend? Maybe only responses to /random, /content and /search endpoints (that
are displayed in the viewer) should be translatable?
Also, the test cases against vulnerabilities in kiwix-serve seem to suggest
that KIWIX_RESPONSE_DATA should be HTML-encoded too.
This commit demonstrates front-end-side translation of an error page
for a URL like /viewer#INVALIDBOOK/whatever (where INVALIDBOOK should
be a book name NOT present in the library).
Known issues:
- This change breaks a couple of subtests in the
ServerTest.Http404HtmlError unit test.
- Changing the UI language while an error page is displayed in the
viewer doesn't retranslate it.
Non-HTML-encoded HTML-template data causes problems in HTML
even when it appears inside JS string (resulting in the <script> tag being
closed by a </script> appearing inside a JS string).
Besides, the KIWIX_RESPONSE_DATA and KIWIX_RESPONSE_TEMPLATE variables
are set on the window object so that they can be accessed from the top
context.
This commit eliminates the need for the `escapeQuote` parameter in
`escapeForJSON()` (that was introduced earlier in this PR) since now it
is set to false in all call contexts. However from the consistency point
of view, the default and intuitive behaviour of `escapeForJSON()` should
be to escape the quote symbols, which justifies the existence of that
parameter.
This is a shortcut change since it doesn't make sense to send the error
page template with every error response (the viewer can fetch it from
the server once but that's slightly more work).
- More familiar escape sequences for tab, newline and carriage return
symbols.
- Quote symbol is escaped by default too, however that behaviour can
be disabled for uses in HTML-related contexts where quotes should then
be replaced with the character entity "
Now when parameterized messages are added to an error response, they are
not immediately instantiated (translated). Instead the message id and
the parameters of the message are recorded. The instantiation of the
messages happens right before generating the final content of the
response.
ContentResponseBlueprint::m_data is now an opaque data member
implemented in the .cpp and ready to be switched from
kainjow::mustache::data to a different implementation.
It turned out that ContentResponse::m_root is no longer used.
At this point, the root parameter is dropped only from the 3-ary variant
of ContentResponse::build(), so that its all call sites are
automatically discovered by the compiler (and updated manually).
Including the other (4-ary) variant of ContentResponse::build() in this
change might result in the semantic change of expressions like
`ContentResponse::build(x, y, z)` and failure to update them.
By moving the nameMapper/library arguments in `getHtml`/`getXml` we avoid
any potential "use after free" of name mapper or library as they are not
stored.
As we now always use Library through a shared_ptr, we can make `Library`
not copiable and not movable.
So we can move back the data in `Library`, we don't care about move
constructor.
The why of this mutex is in `Library` is a bit complex.
It has been introduced in c2927ce when there was only `Library` and no
`std::unique_ptr<Impl>`.
As introducing the mutex imply implementing the move constructor, we have
split all data in `LibraryBase` (and keep a default move constructor here)
and add the mutex in `Library` (and implement a simple move constructor).
Later, in 090c2fd, we have move the `LibraryBase` to `Library::Impl`
(which should have been `Library::Data`).
So at the end, `Library::Impl` is never moved. We can move the `mutex` in
it and still simply implement move constructor for `Library`.
Veloman Yunkan
Begasus
Matthieu Gautier
Emmanuel Engelhart