cgit 1.2.3: lighttpd 1.4.57, AlpineLinux [edge]: using cache breaks delivery

[Steffen Nurpmeso]

Hello!

Steffen Nurpmeso wrote in
 <20201222135537.ovyl4%steffen at sdaoden.eu>:
 |gs-cgit-lists.zx2c4.com at gluelogic.com wrote in
 | <20201222061206.GA54419 at xps13>:
 ||>Steffen Nurpmeso wrote in
 ||> <20201221193127.zbZeP%steffen at sdaoden.eu>:
 ||>|John Keeping wrote in
 ||>| <X+DiDgGaPaynnocI at john.keeping.me.uk>:
 ||>||On Mon, Dec 21, 2020 at 05:26:19PM +0100, Steffen Nurpmeso wrote:
 ||>||> I discovered today that cgit no longer delivers pages, and it must
 ||>||> have been like that for some time.  The server looks show
 ||>||> successful delivery, the cgit cache is populated and rotated just
 ||>||> correctly, but all cgit delivers is that final error of main() as
 ||>||> 
 ||>||>   <div class='error'>Error processing page: Invalid argument \
 ||>||>   (22)</div\
 ||>||>>
 ||> ...
 ||>||> I am pretty sure cgit delivered some weeks ago, the most notable
 ||>||> difference is that AlpineLinux switched to Lighttpd 1.4.56 then
 ||>||> .57, which seems to have brought tremendous changes under the
 ||> ...
 ||>|But the file was generated normally:
 ||>|
 ||>|  # ll /var/lib/lighttpd/cgit/b1000000
 ||>|  -rw-------    1 lighttpd lighttpd     23417 Dec 21 20:22 /var/lib/ligh\
 ||>|  t\
 ||>|  tpd/cgit/b1000000
 ||> ...
 ||>
 ||>Slightly resorted:
 ||>
 ||> ...
 ||>||Have you looked at the log output?
 ||>|  [cgit] error printing cache /var/lib/lighttpd/cgit/b1000000: Invalid \
 ||>|  argument (22)
 ||> ...
 ||>||and this may be caused by sendfile(2) failing due to some difference in
 ||>||how the web server is setting up the output file descriptor.  You may
 ||>||want to rebuild CGit without HAVE_LINUX_SENDFILE and see if that works.
 ||>
 ||>So i build it with
 | ...
 ||>  -       CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE
 ||>  +       #CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE
 | ...
 ||>and .. it works.
 ||>
 ||>Thank you, i will open an AlpineLinux bug report.  And lighttpd.
 | ...
 |
 ||I would like to propose an alternative and more portable solution:
 ||
 ||cgit cache should fallback to lseek, xread, xwrite if sendfile fails.
 |
 |(Yes.)
 ....

  diff --git a/cache.c b/cache.c
  index 2c70be7..7fbab66 100644
  --- a/cache.c
  +++ b/cache.c
  @@ -85,6 +85,7 @@ static int close_slot(struct cache_slot *slot)
   /* Print the content of the active cache slot (but skip the key). */
   static int print_slot(struct cache_slot *slot)
   {
  +       ssize_t i, j;
   #ifdef HAVE_LINUX_SENDFILE
          off_t start_off;
          int ret;
  @@ -97,12 +98,13 @@ static int print_slot(struct cache_slot *slot)
                  if (ret < 0) {
                          if (errno == EAGAIN || errno == EINTR)
                                  continue;
  +                       if (start_off == slot->keylen + 1)
  +                               break;

I can confirm this hunk fixes the bug.
If i remove it i get the error, with it you are fine.

                          return errno;
                  }
                  return 0;
          } while (1);
  -#else
  -       ssize_t i, j;
  +#endif
  
          i = lseek(slot->cache_fd, slot->keylen + 1, SEEK_SET);
          if (i != slot->keylen + 1)
  @@ -118,7 +120,6 @@ static int print_slot(struct cache_slot *slot)
                  return errno;
          else
                  return 0;
  -#endif
   }
  
   /* Check if the slot has expired */
  
Thanks :), and ciao from Germany,

--steffen
|
|Der Kragenbaer,                The moon bear,
|der holt sich munter           he cheerfully and one by one
|einen nach dem anderen runter  wa.ks himself off
|(By Robert Gernhardt)