Re: svn commit: r1845204 - in /subversion/trunk/subversion: mod_authz_svn/mod_authz_svn.c mod_dav_svn/mod_dav_svn.c
On 2018/10/30 21:11:57, Daniel Shahaf wrote: > Ruediger Pluem wrote on Tue, Oct 30, 2018 at 19:57:58 -: > > BTW, I had to apply the following patch to nominate.pl to get it running > > with perl 5.16.3 on CentOS 7: > > > > Index: tools/dist/backport.pl > > === > > --- tools/dist/backport.pl (revision 1845203) > > +++ tools/dist/backport.pl (working copy) > > @@ -791,8 +791,8 @@ > > > > # Add to state votes that aren't '+0' or 'edit' > > $state->{$_->{digest}}++ for grep > > - ($_->{approval} or $_->{vote} =~ > > /^(-1|-0|[+]1)$/), > > - @votesarray; > > + (($_->{approval} or $_->{vote} =~ > > /^(-1|-0|[+]1)$/), > > + @votesarray); > >} > > } > > > > I am happy to commit this if this is fine with later versions of perl which > > I have not at hand for testing. > > Works for me with and without the change, Perl 5.24.1 on Debian stretch. > +1 to commit. > > Consider using «grep +(foo), @bar» or «grep { foo } @bar» instead of > «grep ((foo), bar»: the three alternatives are equivalent, so it's just > a question of which one is more readable/idiomatic. Used the grep +(foo), @bar syntax and committed in r1845312 Regards Rüdiger
Re: svn commit: r1845204 - in /subversion/trunk/subversion: mod_authz_svn/mod_authz_svn.c mod_dav_svn/mod_dav_svn.c
On 2018/10/30 21:03:53, Daniel Shahaf wrote: > Ruediger Pluem wrote on Tue, Oct 30, 2018 at 20:00:01 -: > > > > > > On 2018/10/30 18:09:42, Daniel Shahaf wrote: > > > Branko Čibej wrote on Tue, 30 Oct 2018 10:22 +0100: > > > > Should I add the above to the commit log? Or should I add an entry to > > CHANGES directly? > > I can do either way whatever you prefer. > > In the log message please; release.py write-changelog will pick it from there > when we roll 1.12.0-alpha1. > Adjusted svn:log property of r1845204. Regards Rüdiger
Re: svn commit: r1845204 - in /subversion/trunk/subversion: mod_authz_svn/mod_authz_svn.c mod_dav_svn/mod_dav_svn.c
Ruediger Pluem wrote on Tue, Oct 30, 2018 at 19:57:58 -: > BTW, I had to apply the following patch to nominate.pl to get it running with > perl 5.16.3 on CentOS 7: > > Index: tools/dist/backport.pl > === > --- tools/dist/backport.pl (revision 1845203) > +++ tools/dist/backport.pl (working copy) > @@ -791,8 +791,8 @@ > > # Add to state votes that aren't '+0' or 'edit' > $state->{$_->{digest}}++ for grep > - ($_->{approval} or $_->{vote} =~ > /^(-1|-0|[+]1)$/), > - @votesarray; > + (($_->{approval} or $_->{vote} =~ > /^(-1|-0|[+]1)$/), > + @votesarray); >} > } > > I am happy to commit this if this is fine with later versions of perl which I > have not at hand for testing. Works for me with and without the change, Perl 5.24.1 on Debian stretch. +1 to commit. Consider using «grep +(foo), @bar» or «grep { foo } @bar» instead of «grep ((foo), bar»: the three alternatives are equivalent, so it's just a question of which one is more readable/idiomatic. Cheers, Daniel (Feel free to join us on IRC, by the way. #svn-dev on freenode)
Re: svn commit: r1845204 - in /subversion/trunk/subversion: mod_authz_svn/mod_authz_svn.c mod_dav_svn/mod_dav_svn.c
Ruediger Pluem wrote on Tue, Oct 30, 2018 at 20:00:01 -: > > > On 2018/10/30 18:09:42, Daniel Shahaf wrote: > > Branko Čibej wrote on Tue, 30 Oct 2018 10:22 +0100: > > > It's important enough to be added to CHANGES, in the server-side > > > bugfixes section, so please add a line there, for 1.12.0. > > > > Since we'll have to add this revision four times to CHANGES (on for each > > of 1.9, 1.10, 1.11, 1.12), perhaps we should start using the CHANGES-in-log- > > message convention a bit more? In this case, it would be something like > > . > > [U:server] mod_dav_svn, mod_authz_svn: Fix segfault under mod_http2 > > (SVN-4782) > > . > > Should I add the above to the commit log? Or should I add an entry to CHANGES > directly? > I can do either way whatever you prefer. In the log message please; release.py write-changelog will pick it from there when we roll 1.12.0-alpha1.
Re: svn commit: r1845204 - in /subversion/trunk/subversion: mod_authz_svn/mod_authz_svn.c mod_dav_svn/mod_dav_svn.c
On 2018/10/30 18:09:42, Daniel Shahaf wrote: > Branko Čibej wrote on Tue, 30 Oct 2018 10:22 +0100: > > It's important enough to be added to CHANGES, in the server-side > > bugfixes section, so please add a line there, for 1.12.0. > > Since we'll have to add this revision four times to CHANGES (on for each > of 1.9, 1.10, 1.11, 1.12), perhaps we should start using the CHANGES-in-log- > message convention a bit more? In this case, it would be something like > . > [U:server] mod_dav_svn, mod_authz_svn: Fix segfault under mod_http2 > (SVN-4782) > . Should I add the above to the commit log? Or should I add an entry to CHANGES directly? I can do either way whatever you prefer. Regards Rüdiger
Re: svn commit: r1845204 - in /subversion/trunk/subversion: mod_authz_svn/mod_authz_svn.c mod_dav_svn/mod_dav_svn.c
On 2018/10/30 09:22:56, Branko Čibej wrote: > > [[[ > .../repos/1.11.x$ ../trunk/tools/dist/nominate.pl r1845204 "Prevents a crash > in mod_http2." > Index: STATUS > === > --- STATUS(revision 1845205) > +++ STATUS(working copy) > @@ -48,6 +48,14 @@ Candidate changes: > Votes: > +1: brane > > + * r1845204 > + Fix issue SVN-4782: Do not use (const char*)1 in httpd modules as value > for > + r->notes. > + Justification: > + Prevents a crash in mod_http2. > + Votes: > + +1: brane > + > Veto-blocked changes: > = > > Commit this nomination? > ]]] > > (typing y will commit, anything else will revert the change to > STATUS). > > Our currently maintained branches are 1.9.x, 1.10.x and 1.11.x; fixing a Done for all 3 branches via the perl script as shown above BTW, I had to apply the following patch to nominate.pl to get it running with perl 5.16.3 on CentOS 7: Index: tools/dist/backport.pl === --- tools/dist/backport.pl (revision 1845203) +++ tools/dist/backport.pl (working copy) @@ -791,8 +791,8 @@ # Add to state votes that aren't '+0' or 'edit' $state->{$_->{digest}}++ for grep - ($_->{approval} or $_->{vote} =~ /^(-1|-0|[+]1)$/), - @votesarray; + (($_->{approval} or $_->{vote} =~ /^(-1|-0|[+]1)$/), + @votesarray); } } I am happy to commit this if this is fine with later versions of perl which I have not at hand for testing. Regards Rüdiger
Re: svn commit: r1845204 - in /subversion/trunk/subversion: mod_authz_svn/mod_authz_svn.c mod_dav_svn/mod_dav_svn.c
Branko Čibej wrote on Tue, 30 Oct 2018 10:22 +0100: > It's important enough to be added to CHANGES, in the server-side > bugfixes section, so please add a line there, for 1.12.0. Since we'll have to add this revision four times to CHANGES (on for each of 1.9, 1.10, 1.11, 1.12), perhaps we should start using the CHANGES-in-log- message convention a bit more? In this case, it would be something like . [U:server] mod_dav_svn, mod_authz_svn: Fix segfault under mod_http2 (SVN-4782) . added to the the log message (as documented in tools/dist/release.py:write_changelog()). (Rüdiger, this isn't something you could've known. It so happens that today we start, for the first time in our history, to support three minor lines in parallel; that's why I'm bringing up streamlining the CHANGES process.) > The backport process is similar to APR's and I assume httpd's, we use a > STATUS file for nominations with 3 PMC +1 required for core changes. We > have a script for proposing backports, here's an example: We also have a script for automatically merging backports that have received the required number of votes, which might be of interest to apr/httpd's RM's. (See commits by the role account 'svn-role' to our tree.) Cheers, Daniel
Re: svn commit: r1845204 - in /subversion/trunk/subversion: mod_authz_svn/mod_authz_svn.c mod_dav_svn/mod_dav_svn.c
On 30.10.2018 10:04, rpl...@apache.org wrote: > Author: rpluem > Date: Tue Oct 30 09:04:14 2018 > New Revision: 1845204 > > URL: http://svn.apache.org/viewvc?rev=1845204=rev > Log: > Fix issue SVN-4782: Do not use (const char*)1 in httpd modules as value for > r->notes. > > mod_authz_svn.c and mod_dav_svn.c add keys to r->notes to memorize boolean > states (FORCE_AUTHN_NOTE, IN_SOME_AUTHN_NOTE, authz_svn-anon-ok, > NO_MAP_TO_STORAGE_NOTE). They use (const char*)1 as values for the keys. This > causes any call to apr_table_clone for r->notes to crash with a SEGFAULT, > because (const char*)1 is an invalid address. mod_http2 in httpd calls > apr_table_clone for r->notes and hence the httpd process crashes. > Hence replace the value of (const char*)1 in these cases with a value of "1". > > * subversion/mod_authz_svn/mod_authz_svn.c > (access_checker, check_user_id): Replace value of (const char*)1 with "1" >in apr_table_setn calls for r->notes table for keys FORCE_AUTHN_NOTE, >IN_SOME_AUTHN_NOTE, authz_svn-anon-ok to set a value with an valid address. > > * subversion/mod_authz_svn/mod_dav_svn.c > (dav_svn__translate_name): Replace value of (const char*)1 with "1" >in apr_table_setn calls for r->notes table for keys NO_MAP_TO_STORAGE_NOTE >to set a value with an valid address. Hi Ruediger, This looks perfect, thank you. It's important enough to be added to CHANGES, in the server-side bugfixes section, so please add a line there, for 1.12.0. The backport process is similar to APR's and I assume httpd's, we use a STATUS file for nominations with 3 PMC +1 required for core changes. We have a script for proposing backports, here's an example: [[[ .../repos/1.11.x$ ../trunk/tools/dist/nominate.pl r1845204 "Prevents a crash in mod_http2." Index: STATUS === --- STATUS (revision 1845205) +++ STATUS (working copy) @@ -48,6 +48,14 @@ Candidate changes: Votes: +1: brane + * r1845204 + Fix issue SVN-4782: Do not use (const char*)1 in httpd modules as value for + r->notes. + Justification: + Prevents a crash in mod_http2. + Votes: + +1: brane + Veto-blocked changes: = Commit this nomination? ]]] (typing y will commit, anything else will revert the change to STATUS). Our currently maintained branches are 1.9.x, 1.10.x and 1.11.x; fixing a crash is important enough to backport to all of them. Your vote won't be binding since you're not a PMC member, but there's nothing wrong with keeping it there. Also, please always use the trunk version of the nominate.pl script. -- Brane