On Wed, Dec 8, 2021 at 2:42 PM Ruediger Pluem <rpl...@apache.org> wrote:
>
>
> On 12/8/21 2:26 PM, Ruediger Pluem wrote:
> >
> >
> > On 11/3/19 4:48 PM, yla...@apache.org wrote:
> >> Author: ylavic
>
> >>
> >> + rc = ap_proxy_tunnel_run(tunnel);
> >> + if (ap_is_HTTP_ERROR(rc)) {
> >> + /* Don't send an error page if we sent data already */
> >> + if (proxyport && !tunnel->replied) {
> >> + return rc;
> >> }
> >> - } while (!done);
> >> -
> >> - ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
> >> - "finished with poll() - cleaning up");
> >> + /* Custom log may need this, still */
> >> + r->status = rc;
> >
> > I have some lively discussions outside this forum on the consequences the
> > above line has.
> > In fact this means that even though we replied to the client with a 200 on
> > its CONNECT request we might log a different status
> > code if something goes wrong during the tunneling. In contrast to this we
> > don't do this for other other methods e.g. GET or POST:
> > Once we sent the status code to the client we do not log a different one
> > should something go wrong while delivering the response.
> > Removing r->status = rc is IMHO enough to align the behavior again.
> >
> > Thoughts?
> >
>
> BTW: We do a similar thing in mod_proxy_http.c in the backend replied with
> Switching Protocols:
>
> status = ap_proxy_tunnel_run(tunnel);
> if (ap_is_HTTP_ERROR(status)) {
> /* Tunnel always return HTTP_GATEWAY_TIME_OUT on timeout,
> * but we can differentiate between client and backend here.
> */
> if (status == HTTP_GATEWAY_TIME_OUT
> && tunnel->timeout == client_timeout) {
> status = HTTP_REQUEST_TIME_OUT;
> }
> }
> else {
> /* Update r->status for custom log */
> status = HTTP_SWITCHING_PROTOCOLS;
> }
> r->status = status;
>
> Hence we have the same sort of different behavior compared to "ordinary"
> requests here as well. Hence should we align here as well
> and always log HTTP_SWITCHING_PROTOCOLS no matter if there was an error while
> tunneling the protocol?
I think you are right, the goal was to show in the access_log how
tunneling went (primary for the Upgrade case, didn't think too much of
CONNECT actually :/), but if worthy it looks better to have a new
r->async_status for that because we are missing the initial status
now..
Regards;
Yann.
