[BUG] session: don't stop forwarding of data upon last packet
If a read shutdown is encountered on the first packet of a connection
right after the data and the last analyser is unplugged at the same
time, then that last data chunk may never be forwarded. In practice,
right now it cannot happen on requests due to the way they're scheduled,
nor can it happen on responses due to the way their analysers work.
But this behaviour has been observed with new response analysers being
developped.
The reason is that when the read shutdown is encountered and an analyser
is present, data cannot be forwarded but the BF_SHUTW_NOW flag is set.
After that, the analyser gets called and unplugs itself, hoping that
process_session() will automatically forward the data. This does not
happen due to BF_SHUTW_NOW.
Simply removing the test on this flag is not enough because then aborted
requests still get forwarded, due to the forwarding code undoing the
abort.
The solution here consists in checking BF_SHUTR_NOW instead of BF_SHUTW_NOW.
BF_SHUTR_NOW is only set on aborts and remains set until ->shutr() is called.
This is enough to catch recent aborts but not prevent forwarding in other
cases. Maybe a new special buffer flag "BF_ABORT" might be desirable in the
future.
This patch does not need to be backported because older versions don't
have the analyser which make the problem appear.
diff --git a/src/session.c b/src/session.c
index 9f3ccec..ae73c8f 100644
--- a/src/session.c
+++ b/src/session.c
@@ -1703,9 +1703,11 @@
/* If noone is interested in analysing data, it's time to forward
* everything. We configure the buffer to forward indefinitely.
+ * Note that we're checking BF_SHUTR_NOW as an indication of a possible
+ * recent call to buffer_abort().
*/
if (!s->req->analysers &&
- !(s->req->flags & (BF_HIJACK|BF_SHUTW|BF_SHUTW_NOW)) &&
+ !(s->req->flags & (BF_HIJACK|BF_SHUTW|BF_SHUTR_NOW)) &&
(s->req->prod->state >= SI_ST_EST) &&
(s->req->to_forward != BUF_INFINITE_FORWARD)) {
/* This buffer is freewheeling, there's no analyser nor hijacker
@@ -1717,11 +1719,10 @@
buffer_auto_close(s->req);
buffer_flush(s->req);
- /* If the producer is still connected, we'll enable data to flow
- * from the producer to the consumer (which might possibly not be
- * connected yet).
+ /* We'll let data flow between the producer (if still connected)
+ * to the consumer (which might possibly not be connected yet).
*/
- if (!(s->req->flags & (BF_SHUTR|BF_SHUTW|BF_SHUTW_NOW)))
+ if (!(s->req->flags & (BF_SHUTR|BF_SHUTW_NOW)))
buffer_forward(s->req, BUF_INFINITE_FORWARD);
}
@@ -1825,9 +1826,11 @@
/* If noone is interested in analysing data, it's time to forward
* everything. We configure the buffer to forward indefinitely.
+ * Note that we're checking BF_SHUTR_NOW as an indication of a possible
+ * recent call to buffer_abort().
*/
if (!s->rep->analysers &&
- !(s->rep->flags & (BF_HIJACK|BF_SHUTW|BF_SHUTW_NOW)) &&
+ !(s->rep->flags & (BF_HIJACK|BF_SHUTW|BF_SHUTR_NOW)) &&
(s->rep->prod->state >= SI_ST_EST) &&
(s->rep->to_forward != BUF_INFINITE_FORWARD)) {
/* This buffer is freewheeling, there's no analyser nor hijacker
@@ -1837,7 +1840,11 @@
buffer_auto_read(s->rep);
buffer_auto_close(s->rep);
buffer_flush(s->rep);
- if (!(s->rep->flags & (BF_SHUTR|BF_SHUTW|BF_SHUTW_NOW)))
+
+ /* We'll let data flow between the producer (if still connected)
+ * to the consumer.
+ */
+ if (!(s->rep->flags & (BF_SHUTR|BF_SHUTW_NOW)))
buffer_forward(s->rep, BUF_INFINITE_FORWARD);
}