Archive pages limit uncovers interesting breakage

[th-h]

Since merging of #665 - which was backported and released with 2.3.3 - serendipity_plugin_history breaks with a SQL error for all archive pages but the first:

You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '-10, 10' at line 38

This is due to the fact that serendipity_fetchEntries() in include/functions_entries.inc.php is not just called to display entries on pages, but also to get a list of entries for many other use cases so the calculation of $totalEntries may not always return the result that you expected. Please bear with me, it's a bit of convoluted.

• In this case, we're showing archive page 2 of many. On this page, serendipity_plugin_history tries to show up to 10 older entries that have been posted about a year ago (let's say at least 360, but not more than 370 days ago). It does that by calling serendipity_fetchEntries() with a range and a limit of 10. But today, there are no historic entries to show.

• The code in serendipity_fetchEntries() will construct $serendipity['fullCountQuery'] as always with all constraints from the serendipity_fetchEntries() call and pass it to serendipity_getTotalEntries(); the result will be 0 this time, so $totalEntries will be 0, too. That means we have zero archive pages ($totalPages) , too. The archive page limiting code will set $serendipity['GET']['page'] to zero accordingly.

• And further on, the paginating code kicks in and constructs the LIMIT query. It does that by setting the offset to the number of entries that have already been displayed by subtracting 1 from the current page and multiplying that with the limit of entries to show, i.e. on page 2 when showing 10 entries on every page, it gives (2-1)*10 = 10. But we have forced $serendipity['GET']['page'] to zero, so now it is (0-1)*10 = -10 which is invalid.

We could roll back that change, but I think we don't consider the possibility that we don't use $limit to paginate here, i.e. that serendipity_fetchEntries() is called for another purpose than displaying an archive page, at all. It would also be wrong to display the "historic" entries in reverse order if $serendipity['archiveSortStable']) ist set to true.

So I think the archive limiting code is not wrong per se but has uncovered a design problem. That's why I think we should keep that code to uncover other problems like this.

We could add another parameter to serendipity_fetchEntries() to check if we need to paginate, but the parameter list already is impressingly long. So I'll fix the problem in the calling code; we'll have to keep that in mind for other places where that will creep up, so I'll let this issue open a bit for discussion.