AW: svn commit: r1834924 - /httpd/httpd/branches/2.4.x/STATUS
> -Ursprüngliche Nachricht- > Von: Graham Leggett > Gesendet: Mittwoch, 10. Oktober 2018 19:12 > An: dev@httpd.apache.org > Betreff: Re: svn commit: r1834924 - /httpd/httpd/branches/2.4.x/STATUS > > On 28 Aug 2018, at 12:17, Ruediger Pluem wrote: > > >> On 07/20/2018 02:49 PM, Yann Ylavic wrote: > >>> Ping, any objection if I commit this and add it to the backport > proposal? > >> > >> Hmm. Looks like MODSSL_ERROR_BAD_GATEWAY is only used when the proxy > connects to the backend. > >> So the patch should be fine. > > > > > > Do you want to commit and update the backport proposal? > > Quick ping, are you happy with the updated proposal? Fine now. r18843533 Regards Rüdiger
Re: svn commit: r1834924 - /httpd/httpd/branches/2.4.x/STATUS
On Tue, Aug 28, 2018 at 12:17 PM Ruediger Pluem wrote: > > On 07/20/2018 03:07 PM, Ruediger Pluem wrote: > > > > On 07/20/2018 02:49 PM, Yann Ylavic wrote: > >> Ping, any objection if I commit this and add it to the backport proposal? > > > > Hmm. Looks like MODSSL_ERROR_BAD_GATEWAY is only used when the proxy > > connects to the backend. > > So the patch should be fine. > > Do you want to commit and update the backport proposal? Done (r1839442 fox the fix and r1839443. for the proposal). Thanks for the reminder. Regards, Yann.
Re: svn commit: r1834924 - /httpd/httpd/branches/2.4.x/STATUS
On 07/20/2018 03:07 PM, Ruediger Pluem wrote: > > > On 07/20/2018 02:49 PM, Yann Ylavic wrote: >> Ping, any objection if I commit this and add it to the backport proposal? > > Hmm. Looks like MODSSL_ERROR_BAD_GATEWAY is only used when the proxy connects > to the backend. > So the patch should be fine. Do you want to commit and update the backport proposal? Regards Rüdiger
Re: svn commit: r1834924 - /httpd/httpd/branches/2.4.x/STATUS
On 07/20/2018 02:49 PM, Yann Ylavic wrote: > Ping, any objection if I commit this and add it to the backport proposal? Hmm. Looks like MODSSL_ERROR_BAD_GATEWAY is only used when the proxy connects to the backend. So the patch should be fine. Regards Rüdiger > > On Tue, Jul 3, 2018 at 10:36 AM, Yann Ylavic wrote: >> On Tue, Jul 3, 2018 at 8:58 AM, 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; >> ? >
Re: svn commit: r1834924 - /httpd/httpd/branches/2.4.x/STATUS
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 wrote: > On Tue, Jul 3, 2018 at 8:58 AM, 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; > ?
Re: svn commit: r1834924 - /httpd/httpd/branches/2.4.x/STATUS
On Tue, Jul 3, 2018 at 8:58 AM, 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; ?
