Ping, any objection if I commit this and add it to the backport proposal?

On Tue, Jul 3, 2018 at 10:36 AM, Yann Ylavic <[email protected]> wrote:
> On Tue, Jul 3, 2018 at 8:58 AM,  <[email protected]> wrote:
>>
>> +++ httpd/httpd/branches/2.4.x/STATUS Tue Jul  3 06:58:55 2018
>> @@ -179,7 +179,11 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>>       2.4.x patch: svn merge -c 1645529 ^/httpd/httpd/trunk .
>>       +1: ylavic, druggeri
>>       druggeri: Why no +1, jallietc36?
>> -
>> +     rpluem: I think the patch is wrong here and in trunk. This causes
>> +     ap_pass_brigade to return APR_SUCCESS in ap_proxy_pass_brigade. The 
>> error
>> +     bucket inserted by ssl_io_filter_error IMHO makes no sense because it
>> +     would be sent to the origin server (the proxy backend) and not to our
>> +     client. Further discussion should possibly happen on dev@.
>
> Agreed, but r1645529 looks right to me, I'd rather fix
> ssl_io_filter_error() to return EGENERAL (and no error brigade) in
> this case.
>
> Something like this:
> Index: modules/ssl/ssl_engine_io.c
> ===================================================================
> --- modules/ssl/ssl_engine_io.c    (revision 1834106)
> +++ modules/ssl/ssl_engine_io.c    (working copy)
> @@ -1008,14 +1008,10 @@ static apr_status_t ssl_io_filter_error(bio_filter
>              break;
>
>      case MODSSL_ERROR_BAD_GATEWAY:
> -        /* Send an error bucket, though the proxy currently has no
> -         * special handling for error buckets and ignores this. */
> -        bucket = ap_bucket_error_create(HTTP_BAD_GATEWAY, NULL,
> -                                        f->c->pool,
> -                                        f->c->bucket_alloc);
>          ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, f->c, APLOGNO(01997)
>                        "SSL handshake failed: sending 502");
> -        break;
> +        f->c->aborted = 1;
> +        return APR_EGENERAL;
>
>      default:
>          return status;
> ?

Reply via email to