BUG/MAJOR: stream: ensure analysers are always called upon close
A recent issue affecting HTTP/2 + redirect + cache has uncovered an old problem affecting all existing versions regarding the way events are reported to analysers. It happens that when an event is reported, analysers see it and may decide to temporarily pause processing and prevent other analysers from processing the same event. Then the event may be cleared and upon the next call to the analysers, some of them will never see it. This is exactly what happens with CF_READ_NULL if it is received before the request is processed, like during redirects : the first time, some analysers see it, pause, then the event may be converted to a SHUTW and cleared, and on next call, there's nothing to process. In practice it's hard to get the CF_READ_NULL flag during the request because requests have CF_READ_DONTWAIT, preventing the read0 from happening. But on HTTP/2 it's presented along with any incoming request. Also on a TCP frontend the flag is not set and it's possible to read the NULL before the request is parsed. This causes a problem when filters are present because flt_end_analyse needs to be called to release allocated resources and remove the CF_FLT_ANALYZE flag. And the loss of this event prevents the analyser from being called and from removing itself, preventing the connection from ever ending. This problem just shows that the event processing needs a serious revamp after 1.8. In the mean time we can deal with the really problematic case which is that we *want* to call analysers if CF_SHUTW is set on any side ad it's the last opportunity to terminate a processing. It may occasionally result in some analysers being called for nothing in half- closed situations but it will take care of the issue. An example of problematic configuration triggering the bug in 1.7 is : frontend tcp bind :4445 default_backend http backend http redirect location / compression algo identity Then submitting requests which immediately close will have for effect to accumulate streams which will never be freed : $ printf "GET / HTTP/1.1\r\n\r\n" >/dev/tcp/0/4445 This fix must be backported to 1.7 as well as any version where commit c0c672a ("BUG/MINOR: http: Fix conditions to clean up a txn and to handle the next request") was backported. This commit didn't cause the bug but made it much more likely to happen.
This commit is contained in:
parent
e223e3bc85
commit
33982cbdc0
@ -1828,6 +1828,7 @@ struct task *process_stream(struct task *t)
|
||||
/* Analyse request */
|
||||
if (((req->flags & ~rqf_last) & CF_MASK_ANALYSER) ||
|
||||
((req->flags ^ rqf_last) & CF_MASK_STATIC) ||
|
||||
(req->analysers && (req->flags & CF_SHUTW)) ||
|
||||
si_f->state != rq_prod_last ||
|
||||
si_b->state != rq_cons_last ||
|
||||
s->pending_events & TASK_WOKEN_MSG) {
|
||||
@ -1927,6 +1928,7 @@ struct task *process_stream(struct task *t)
|
||||
|
||||
if (((res->flags & ~rpf_last) & CF_MASK_ANALYSER) ||
|
||||
(res->flags ^ rpf_last) & CF_MASK_STATIC ||
|
||||
(res->analysers && (res->flags & CF_SHUTW)) ||
|
||||
si_f->state != rp_cons_last ||
|
||||
si_b->state != rp_prod_last ||
|
||||
s->pending_events & TASK_WOKEN_MSG) {
|
||||
|
Loading…
x
Reference in New Issue
Block a user