On 12/8/21 2:49 PM, Yann Ylavic wrote:
> 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..
Should I take care of adjusting mod_proxy_connect and mod_proxy_http to align
this again with standard flows and in a second step
we take care of that r->async_status idea?
Regards
Rüdiger
