Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Content-Type: text/plain; charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\))
Subject: Re: svn commit: r1750301 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.h proxy_util.c
From: Stefan Eissing <stefan.eissing@greenbytes.de>
In-Reply-To: <CAKQ1sVMqacCGRifeuAOePq9O1-3T0RnN1mLXD6yd9kYeR4R+Dg@mail.gmail.com>
Date: Tue, 28 Jun 2016 15:36:39 +0200
Content-Transfer-Encoding: quoted-printable
Message-Id: <29635005-6B25-4F25-B4A6-D30F752ACE78@greenbytes.de>
References: <20160627080030.E862B3A0096@svn01-us-west.apache.org> <CC3A4E8E-EEC9-459D-8EDE-3906B0260E00@eissing.org> <CAKQ1sVMgd2-KiMwdmX7wDREg7UWBM9Xp+hOmMq30Js=3catt7g@mail.gmail.com> <062A0516-985A-4FC2-B02A-1FCF403763AF@greenbytes.de> <CAKQ1sVODGKXK4trZii_pwdOpdprEXvLfYLoA2KC_j10j_HFWZw@mail.gmail.com> <CAKQ1sVP8fzrNEPRGh6i=E5iGSJBOzOESV4G4N8_K4fHritD+0g@mail.gmail.com> <CAKQ1sVMPPGBsKJd=pkB7q9v8RTjXChOtvjJAXF9nQz-k9Ovekg@mail.gmail.com> <CAKQ1sVPRjMmg3uC6KJ91EbMoJAJqGGpodqWc1=OQUUZ61YzGUQ@mail.gmail.com> <896863B2-7B8E-4808-8606-3849E3F30796@greenbytes.de> <CAKQ1sVMi1hdzG3nGPkm28Tu-BKdyVFH-Ma_s6HMCv-UZu0RnDQ@mail.gmail.com> <4AD4C532-FD57-4522-B332-48ED2409BE5C@greenbytes.de> <13E0E405-29B1-47CC-B236-F61FD9A96020@greenbytes.de> <CAKQ1sVM6R1NC82o=gFtWPDaBXJJxMujBxN0Qh0NHu9JYv6gOXg@mail.gmail.com> <93BA09A6-6623-4B72-A54A-2ADF1982E4B3@greenbytes.de> <CAKQ1sVO9C-cdHA2z0436eXig+ZL7Gq4N9r3m3cT7bEioDXstaA@mail.gmail.com> <CAKQ1sVMUkJQ9qqZV3eX8=b3mdwg+=_uSRzWJfk2t5SEFNWWuTw@mail.gmail.
 com> <82BF462D-48E8-41AA-B90D-3C5CFFC923B5@greenbytes.de> <CAKQ1sVMqacCGRifeuAOePq9O1-3T0RnN1mLXD6yd9kYeR4R+Dg@mail.gmail.com>
To: dev@httpd.apache.org
archived-at: Tue, 28 Jun 2016 13:36:46 -0000

Commited in r1750505.

> Am 28.06.2016 um 15:13 schrieb Yann Ylavic <ylavic.dev@gmail.com>:
>=20
> Jim didn't tag yet AFAICT, did he?
> If not, since it's mod_proxy_http2 scope only, +1 for me.
>=20
> On Tue, Jun 28, 2016 at 3:02 PM, Stefan Eissing
> <stefan.eissing@greenbytes.de> wrote:
>> Thanks. I had to change from r-> to ctx->rbase, but otherwise works =
fine.
>>=20
>> Shall I commit this and potentially break Jim's tagging again?
>>=20
>> -Stefan
>>=20
>>=20
>>=20
>>> Am 28.06.2016 um 13:49 schrieb Yann Ylavic <ylavic.dev@gmail.com>:
>>>=20
>>> Patch as a file attached.
>>>=20
>>> On Tue, Jun 28, 2016 at 1:48 PM, Yann Ylavic <ylavic.dev@gmail.com> =
wrote:
>>>> Maybe if you can test current 2.4.x with this patch and it works as
>>>> expected it could be backported...
>>>>=20
>>>> On Tue, Jun 28, 2016 at 1:46 PM, Yann Ylavic <ylavic.dev@gmail.com> =
wrote:
>>>>> Dunno, the issue is that reused TLS connections where data are
>>>>> immediately available from the backend may be missing some =
bytes...
>>>>>=20
>>>>> On Tue, Jun 28, 2016 at 1:43 PM, Stefan Eissing
>>>>> <stefan.eissing@greenbytes.de> wrote:
>>>>>> Ah, understood. Do you want to squeeze it into 2.4.23 or can it =
wait?
>>>>>>=20
>>>>>>> Am 28.06.2016 um 13:42 schrieb Yann Ylavic =
<ylavic.dev@gmail.com>:
>>>>>>>=20
>>>>>>> I don't think trunk needs it because ap_proxy_connect_backend() =
is
>>>>>>> already doing this work (via ap_proxy_check_backend).
>>>>>>>=20
>>>>>>> That's why I proposed a 2.4.x only patch, but I can commit it to =
trunk
>>>>>>> temporarily if that helps (and until) backport...
>>>>>>>=20
>>>>>>>=20
>>>>>>> On Tue, Jun 28, 2016 at 12:36 PM, Stefan Eissing
>>>>>>> <stefan.eissing@greenbytes.de> wrote:
>>>>>>>> We are talking about adding this to trunk first, right? ^^
>>>>>>>>=20
>>>>>>>>> Am 28.06.2016 um 12:34 schrieb Stefan Eissing =
<stefan.eissing@greenbytes.de>:
>>>>>>>>>=20
>>>>>>>>> I believe so. Highly experimental and all such...
>>>>>>>>>=20
>>>>>>>>>> Am 28.06.2016 um 12:23 schrieb Yann Ylavic =
<ylavic.dev@gmail.com>:
>>>>>>>>>>=20
>>>>>>>>>> I can, but is mod_proxy_h2 CTR (Commit Then Review) like
>>>>>>>>>> mod_h2 ?
>>>>>>>>>>=20
>>>>>>>>>> On Tue, Jun 28, 2016 at 12:15 PM, Stefan Eissing
>>>>>>>>>> <stefan.eissing@greenbytes.de> wrote:
>>>>>>>>>>> Looks good to me. Can you commit this, then I quickly run my =
tests with it...
>>>>>>>>>>>=20
>>>>>>>>>>>> Am 28.06.2016 um 09:50 schrieb Yann Ylavic =
<ylavic.dev@gmail.com>:
>>>>>>>>>>>>=20
>>>>>>>>>>>> On Tue, Jun 28, 2016 at 12:23 AM, Yann Ylavic =
<ylavic.dev@gmail.com> wrote:
>>>>>>>>>>>>>>=20
>>>>>>>>>>>>>> The possible issue if r1750414 were backported, is that =
without
>>>>>>>>>>>>>> r1750392 mod_proxy_http2 may not detect a TLS close =
notify before
>>>>>>>>>>>>>> reusing a backend connection.
>>>>>>>>>>>>>> If it's not backported, it may close a legitimate backend =
connection
>>>>>>>>>>>>>> with (pre-)available data...
>>>>>>>>>>>>>=20
>>>>>>>>>>>>> I meant: it may discard (pre-)available data (not closing =
the connection).
>>>>>>>>>>>>=20
>>>>>>>>>>>> A possible solution for 2.4.x (needed only there AFAICT), =
could be:
>>>>>>>>>>>>=20
>>>>>>>>>>>> Index: modules/http2/mod_proxy_http2.c
>>>>>>>>>>>> =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>>>>>>>>>>> --- modules/http2/mod_proxy_http2.c    (revision 1750453)
>>>>>>>>>>>> +++ modules/http2/mod_proxy_http2.c    (working copy)
>>>>>>>>>>>> @@ -520,11 +520,19 @@ run_connect:
>>>>>>>>>>>> }
>>>>>>>>>>>>=20
>>>>>>>>>>>> ctx->p_conn->is_ssl =3D ctx->is_ssl;
>>>>>>>>>>>> -    if (ctx->is_ssl) {
>>>>>>>>>>>> -        /* If there is still some data on an existing ssl =
connection, now
>>>>>>>>>>>> -         * would be a good timne to get rid of it. */
>>>>>>>>>>>> -        ap_proxy_ssl_connection_cleanup(ctx->p_conn, =
ctx->rbase);
>>>>>>>>>>>> -    }
>>>>>>>>>>>> +    if (ctx->is_ssl && ctx->p_conn->connection) {
>>>>>>>>>>>> +        /* If there are some metadata on the connection =
(e.g. TLS alert),
>>>>>>>>>>>> +         * let mod_ssl detect them, and create a new =
connection below.
>>>>>>>>>>>> +         */
>>>>>>>>>>>> +        apr_bucket_brigade *tmp_bb;
>>>>>>>>>>>> +        tmp_bb =3D apr_brigade_create(r->pool, =
r->connection->bucket_alloc);
>>>>>>>>>>>> +        status =3D =
ap_get_brigade(ctx->p_conn->connection->input_filters, tmp_bb,
>>>>>>>>>>>> +                                AP_MODE_SPECULATIVE, =
APR_NONBLOCK_READ, 1);
>>>>>>>>>>>> +        if (status !=3D APR_SUCCESS && =
!APR_STATUS_IS_EAGAIN(status)) {
>>>>>>>>>>>> +            ctx->p_conn->close =3D 1;
>>>>>>>>>>>> +        }
>>>>>>>>>>>> +        apr_brigade_cleanup(tmp_bb);
>>>>>>>>>>>> +    }
>>>>>>>>>>>>=20
>>>>>>>>>>>> /* Step One: Determine the URL to connect to (might be a =
proxy),
>>>>>>>>>>>> * initialize the backend accordingly and determine the =
server
>>>>>>>>>>>> _
>>>>>>>>>>>>=20
>>>>>>>>>>>> Stefan?
>>>>>>>>>>>=20
>>>>>>>>>=20
>>>>>>>>=20
>>>>>>=20
>>> <httpd-2.4.x-mod_proxy_http2-ssl_reuse_cleanup.patch>
>>=20
>>=20