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: <CAKQ1sVM6R1NC82o=gFtWPDaBXJJxMujBxN0Qh0NHu9JYv6gOXg@mail.gmail.com>
Date: Tue, 28 Jun 2016 13:43:49 +0200
Content-Transfer-Encoding: quoted-printable
Message-Id: <93BA09A6-6623-4B72-A54A-2ADF1982E4B3@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>
To: dev@httpd.apache.org
archived-at: Tue, 28 Jun 2016 11:43:56 -0000

Ah, understood. Do you want to squeeze it into 2.4.23 or can it wait?

> 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