You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@apache.org> on 2018/10/30 09:22:56 UTC
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, rpluem@apache.org wrote:
> Author: rpluem
> Date: Tue Oct 30 09:04:14 2018
> New Revision: 1845204
>
> URL: http://svn.apache.org/viewvc?rev=1845204&view=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<enter> 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
Re: svn commit: r1845204 - in /subversion/trunk/subversion: mod_authz_svn/mod_authz_svn.c mod_dav_svn/mod_dav_svn.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 2018/10/30 21:03:53, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Ruediger Pluem wrote on Tue, Oct 30, 2018 at 20:00:01 -0000:
> >
> >
> > On 2018/10/30 18:09:42, Daniel Shahaf <d....@daniel.shahaf.name> 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
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Ruediger Pluem wrote on Tue, Oct 30, 2018 at 20:00:01 -0000:
>
>
> On 2018/10/30 18:09:42, Daniel Shahaf <d....@daniel.shahaf.name> 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
Posted by Ruediger Pluem <rp...@apache.org>.
On 2018/10/30 18:09:42, Daniel Shahaf <d....@daniel.shahaf.name> 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
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
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
Posted by Ruediger Pluem <rp...@apache.org>.
On 2018/10/30 21:11:57, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Ruediger Pluem wrote on Tue, Oct 30, 2018 at 19:57:58 -0000:
> > 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
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Ruediger Pluem wrote on Tue, Oct 30, 2018 at 19:57:58 -0000:
> 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
Posted by Ruediger Pluem <rp...@apache.org>.
On 2018/10/30 09:22:56, Branko Čibej <br...@apache.org> 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<enter> 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