You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rainer Jung <ra...@kippdata.de> on 2017/11/01 10:02:08 UTC
Re: svn commit: r1813027 - /httpd/httpd/branches/2.4.x/STATUS
Hi Bill,
Am 31.10.2017 um 21:29 schrieb William A Rowe Jr:
> On Mon, Oct 23, 2017 at 10:17 AM, <yl...@apache.org> wrote:
>> Author: ylavic
>> Date: Mon Oct 23 15:17:02 2017
>> New Revision: 1813027
>>
>> URL: http://svn.apache.org/viewvc?rev=1813027&view=rev
>> Log:
>> Update comment according to patch version (v5).
>>
>> Modified:
>> httpd/httpd/branches/2.4.x/STATUS
>>
>> @@ -138,7 +137,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>> +1: ylavic,
>> wrowe: Suspect that this is an MMN Major bump, not minor without some
>> additional detection/workaround of legacy 2.4 compiled modules.
>> - ylavic: Changed to v4 (+r1812193) which hopefully should address compat
>> + ylavic: Changed to v5 (+r1812193) which hopefully should address compat
>> issues, Bill?
>
> That seems to help with the config parser.
>
> This looks problematic; it is undoubtedly a binary ABI change to a
> public interface.
>
> -APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler, (request_rec *r,
> - proxy_worker *worker, proxy_server_conf
> *conf, char *url,
> - const char *proxyhost, apr_port_t proxyport))
> -APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler, (request_rec *r,
> - char *url))
> [...]
> +APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler,
> + (request_rec *r, proxy_worker *worker,
> + proxy_server_conf *conf, char *url,
> + const char *proxyhost, apr_port_t proxyport))
> +APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler,
> + (request_rec *r, char *url))
>
This part you cited to me looks like just a reformat (position of line
breaks changed), but I couldn't spot a functional change there.
Regards,
Rainer
Re: svn commit: r1813027 - /httpd/httpd/branches/2.4.x/STATUS
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Nov 3, 2017 at 4:00 AM, Stefan Eissing
<st...@greenbytes.de> wrote:
> Can we count that as a +1?
Once I've tested. My initial reviews were ensuring compatibility.
Like you, I don't have a repro case yet. I see there is some activity within
httpd test framework that may cover it, just need to test it, if someone
else is further along with their review you may have a +1 faster than
I can get to it.
Re: svn commit: r1813027 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Stefan Eissing <st...@greenbytes.de>.
Can we count that as a +1?
> Am 01.11.2017 um 18:34 schrieb William A Rowe Jr <wr...@rowe-clan.net>:
>
> You are right, thanks. With the new _ex entry points the backport looks ABI clean, nicely done Yann.
>
> On Nov 1, 2017 06:19, "Marion & Christophe JAILLET" <ch...@wanadoo.fr> wrote:
> Same analysis here. The 2 declarations are the same.
>
> CJ
>
>
> Le 01/11/2017 à 11:02, Rainer Jung a écrit :
> Hi Bill,
>
> Am 31.10.2017 um 21:29 schrieb William A Rowe Jr:
> On Mon, Oct 23, 2017 at 10:17 AM, <yl...@apache.org> wrote:
> Author: ylavic
> Date: Mon Oct 23 15:17:02 2017
> New Revision: 1813027
>
> URL: http://svn.apache.org/viewvc?rev=1813027&view=rev
> Log:
> Update comment according to patch version (v5).
>
> Modified:
> httpd/httpd/branches/2.4.x/STATUS
>
> @@ -138,7 +137,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
> +1: ylavic,
> wrowe: Suspect that this is an MMN Major bump, not minor without some
> additional detection/workaround of legacy 2.4 compiled modules.
> - ylavic: Changed to v4 (+r1812193) which hopefully should address compat
> + ylavic: Changed to v5 (+r1812193) which hopefully should address compat
> issues, Bill?
>
> That seems to help with the config parser.
>
> This looks problematic; it is undoubtedly a binary ABI change to a
> public interface.
>
> -APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler, (request_rec *r,
> - proxy_worker *worker, proxy_server_conf
> *conf, char *url,
> - const char *proxyhost, apr_port_t proxyport))
> -APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler, (request_rec *r,
> - char *url))
> [...]
> +APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler,
> + (request_rec *r, proxy_worker *worker,
> + proxy_server_conf *conf, char *url,
> + const char *proxyhost, apr_port_t proxyport))
> +APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler,
> + (request_rec *r, char *url))
>
>
> This part you cited to me looks like just a reformat (position of line breaks changed), but I couldn't spot a functional change there.
>
> Regards,
>
> Rainer
>
>
Re: svn commit: r1813027 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Nov 1, 2017 at 6:34 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> You are right, thanks. With the new _ex entry points the backport looks ABI
> clean, nicely done Yann.
Thanks Bill, by taking a last look at the proposed patch, I've noticed
that v5 partially reverted r1747069, a change that you made and
backported to 2.4.21 (IIRC), but which happened later in time than
than my SSLProxy changes in trunk.
I missed that when resolving the conflict around this code in v5.
So I've just updated the proposal to v6 with this (only) change:
--- httpd-2.4.x-r1740928_and_co-v5.patch
+++ httpd-2.4.x-r1740928_and_co-v6.patch
@@ -2069,13 +2046,13 @@ Index: modules/ssl/ssl_engine_io.c
}
}
- else if ((sc->proxy_ssl_check_peer_cn == SSL_ENABLED_TRUE) &&
-+ else if ((dc->proxy->ssl_check_peer_cn != FALSE) &&
++ else if ((dc->proxy->ssl_check_peer_cn == TRUE) &&
hostname_note) {
const char *hostname;
int match = 0;
_
which restores the existing (and expected) behaviour here.
Regards,
Yann.
Re: svn commit: r1813027 - /httpd/httpd/branches/2.4.x/STATUS
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
You are right, thanks. With the new _ex entry points the backport looks ABI
clean, nicely done Yann.
On Nov 1, 2017 06:19, "Marion & Christophe JAILLET" <
christophe.jaillet@wanadoo.fr> wrote:
> Same analysis here. The 2 declarations are the same.
>
> CJ
>
>
> Le 01/11/2017 à 11:02, Rainer Jung a écrit :
>
>> Hi Bill,
>>
>> Am 31.10.2017 um 21:29 schrieb William A Rowe Jr:
>>
>>> On Mon, Oct 23, 2017 at 10:17 AM, <yl...@apache.org> wrote:
>>>
>>>> Author: ylavic
>>>> Date: Mon Oct 23 15:17:02 2017
>>>> New Revision: 1813027
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1813027&view=rev
>>>> Log:
>>>> Update comment according to patch version (v5).
>>>>
>>>> Modified:
>>>> httpd/httpd/branches/2.4.x/STATUS
>>>>
>>>> @@ -138,7 +137,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>>>> +1: ylavic,
>>>> wrowe: Suspect that this is an MMN Major bump, not minor
>>>> without some
>>>> additional detection/workaround of legacy 2.4 compiled
>>>> modules.
>>>> - ylavic: Changed to v4 (+r1812193) which hopefully should address
>>>> compat
>>>> + ylavic: Changed to v5 (+r1812193) which hopefully should address
>>>> compat
>>>> issues, Bill?
>>>>
>>>
>>> That seems to help with the config parser.
>>>
>>> This looks problematic; it is undoubtedly a binary ABI change to a
>>> public interface.
>>>
>>> -APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler,
>>> (request_rec *r,
>>> - proxy_worker *worker, proxy_server_conf
>>> *conf, char *url,
>>> - const char *proxyhost, apr_port_t proxyport))
>>> -APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler,
>>> (request_rec *r,
>>> - char *url))
>>> [...]
>>> +APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler,
>>> + (request_rec *r, proxy_worker *worker,
>>> + proxy_server_conf *conf, char *url,
>>> + const char *proxyhost, apr_port_t proxyport))
>>> +APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler,
>>> + (request_rec *r, char *url))
>>>
>>>
>> This part you cited to me looks like just a reformat (position of line
>> breaks changed), but I couldn't spot a functional change there.
>>
>> Regards,
>>
>> Rainer
>>
>>
>
Re: svn commit: r1813027 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Marion & Christophe JAILLET <ch...@wanadoo.fr>.
Same analysis here. The 2 declarations are the same.
CJ
Le 01/11/2017 à 11:02, Rainer Jung a écrit :
> Hi Bill,
>
> Am 31.10.2017 um 21:29 schrieb William A Rowe Jr:
>> On Mon, Oct 23, 2017 at 10:17 AM, <yl...@apache.org> wrote:
>>> Author: ylavic
>>> Date: Mon Oct 23 15:17:02 2017
>>> New Revision: 1813027
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1813027&view=rev
>>> Log:
>>> Update comment according to patch version (v5).
>>>
>>> Modified:
>>> httpd/httpd/branches/2.4.x/STATUS
>>>
>>> @@ -138,7 +137,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>>> +1: ylavic,
>>> wrowe: Suspect that this is an MMN Major bump, not minor
>>> without some
>>> additional detection/workaround of legacy 2.4
>>> compiled modules.
>>> - ylavic: Changed to v4 (+r1812193) which hopefully should
>>> address compat
>>> + ylavic: Changed to v5 (+r1812193) which hopefully should
>>> address compat
>>> issues, Bill?
>>
>> That seems to help with the config parser.
>>
>> This looks problematic; it is undoubtedly a binary ABI change to a
>> public interface.
>>
>> -APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler,
>> (request_rec *r,
>> - proxy_worker *worker, proxy_server_conf
>> *conf, char *url,
>> - const char *proxyhost, apr_port_t proxyport))
>> -APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler,
>> (request_rec *r,
>> - char *url))
>> [...]
>> +APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler,
>> + (request_rec *r, proxy_worker *worker,
>> + proxy_server_conf *conf, char *url,
>> + const char *proxyhost, apr_port_t
>> proxyport))
>> +APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler,
>> + (request_rec *r, char *url))
>>
>
> This part you cited to me looks like just a reformat (position of line
> breaks changed), but I couldn't spot a functional change there.
>
> Regards,
>
> Rainer
>