[Varnish] #1642: Assert error in VGZ_Ibuf()

[Varnish]

#1642: Assert error in VGZ_Ibuf()
--------------------------+---------------------
 Reporter:  llavaud       |       Owner:  martin
     Type:  defect        |      Status:  new
 Priority:  normal        |   Milestone:
Component:  varnishd      |     Version:  trunk
 Severity:  major         |  Resolution:
 Keywords:  assert error  |
--------------------------+---------------------

Comment (by martin):

 I have analyzed this ticket, and found the problem to stem from error
 returns from VDP_bytes being ignored in ESI_Deliver() on lines 308, 350,
 421 and 423. When gunzip is in effect this causes problems.

 The problem starts in VDP_gunzip(). What happens is that when a VDP_bytes
 call from lower in the stack returns error (e.g. write error when client
 has gone away), the gunzip input buffer is left with bytes still present.
 On the next call with more input data to this layer in the VDP, the
 assertion is thrown because the buffer still has data in it. Normally this
 doesn't cause problems, as the normal delivery path will abort the
 delivery immediately upon VDP_bytes() returning error, and thus not call
 VDP_bytes() again. But ESI_deliver() doesn't behave the same way.

 Looking at ESI_deliver() the code is inconsistent with regards to the
 treatment of VDP_bytes() errors. Some places it is dealt with, others it
 isn't. The right behavior is an open question though. I see merit in
 making ESI_deliver() ignore delivery errors so that the side effects of
 delivery is applied (fetching uncached ESI includes). The right strategy
 here needs to be decided.

 Possible ways to solve this:
 1)
 Latch VDP_bytes errors as a status on req->vdp_error. Whenever VDP_bytes()
 causes an error the flag is set, and all calls to VDP_bytes() when the
 flag is set immediately returns error without descending into the VDP
 stack. Also fix ESI_deliver() to always ignore VDP_bytes() errors to be
 consistent and get the ESI include side effects applied.
 2)
 Fix ESI_deliver() to always check the return value from VDP_bytes().
 3)
 Change VDP_gunzip() to flush the input buffer on errors. Though I am
 afraid that this would mask delivery errors as gunzip errors because we
 then deliberately mess with the gzip stream.

 Martin

-- 
Ticket URL: <https://www.varnish-cache.org/trac/ticket/1642#comment:3>
Varnish <https://varnish-cache.org/>
The Varnish HTTP Accelerator