Hi Jarno,
On Sat, Feb 11, 2017 at 12:11:47PM +0200, Jarno Huuskonen wrote:
> > I think there's a problem in http_error_message(), there were some
> > conditions to detect certain error cases on reused connections for
> > which we had to remain silent and these conditions have disappeared
> > so we'll return an error instead of silently closing.
>
> I think the problem is with http_return_srv_error, I removed too many
> arguments (and s->txn->flags checks) to http_server_error.
> So we need to silently close if s->txn->flags & TX_NOT_FIRST or
> s->flags & SF_SRV_REUSED ?
The purpose was to avoid returning a 5xx to the client whenever a
connection error was met while sending a request on an already used
connection, so that the client can decide to resend. So I don't remember
all the conditions involved (and am not in front of the code at the
moment) but you get the idea.
> So it might work with (end of http_return_srv_error):
> if (! ((s->txn->flags & TX_NOT_FIRST) || (s->flags & SF_SRV_REUSED)))
> bo_putchk(s->res.buf, http_error_message(s));
>From what I remember, the function also deals with other errors so you
don't want to hide all of them.
X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4
> or (http_return_srv_error) just remove second argument from
> http_error_message call (and no bo_putchk). So http_return_srv_error
> would be something like:
>
> void http_return_srv_error(struct stream *s, struct stream_interface *si)
>
> {
>
> int err_type = si->err_type;
>
>
>
> if (err_type & SI_ET_QUEUE_ABRT)
>
> http_server_error(s, si, SF_ERR_CLICL, SF_FINST_Q,
>
> 503, http_error_message(s));
>
> else if (err_type & SI_ET_CONN_ABRT)
>
> http_server_error(s, si, SF_ERR_CLICL, SF_FINST_C,
>
> 503, (s->txn->flags & TX_NOT_FIRST) ? NULL :
>
> http_error_message(s));
>
> else if (err_type & SI_ET_QUEUE_TO)
>
> http_server_error(s, si, SF_ERR_SRVTO, SF_FINST_Q,
>
> 503, http_error_message(s));
>
> else if (err_type & SI_ET_QUEUE_ERR)
>
> http_server_error(s, si, SF_ERR_SRVCL, SF_FINST_Q,
>
> 503, http_error_message(s));
>
> else if (err_type & SI_ET_CONN_TO)
>
> http_server_error(s, si, SF_ERR_SRVTO, SF_FINST_C,
>
> 503, (s->txn->flags & TX_NOT_FIRST) ? NULL :
>
> http_error_message(s));
>
> else if (err_type & SI_ET_CONN_ERR)
>
> http_server_error(s, si, SF_ERR_SRVCL, SF_FINST_C,
>
> 503, (s->flags & SF_SRV_REUSED) ? NULL :
>
> http_error_message(s));
>
> else if (err_type & SI_ET_CONN_RES)
>
> http_server_error(s, si, SF_ERR_RESOURCE, SF_FINST_C,
>
> 503, (s->txn->flags & TX_NOT_FIRST) ? NULL :
>
> http_error_message(s));
>
> else /* SI_ET_CONN_OTHER and others */
>
> http_server_error(s, si, SF_ERR_INTERNAL, SF_FINST_C,
>
> 500, http_error_message(s));
>
> }
>
> Does it work with this version of http_return_srv_error ?
At first glance I think it should work.
Thanks,
Willy
