You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2008/06/08 10:41:54 UTC
[HEADS-UP] Need more backports to 1.5.0? (was: Re: you're going to
kill me, but... [Re: svn commit: r31625 -) branches/1.5.x]
On Sun, Jun 08, 2008 at 09:49:35AM +0300, Daniel Shahaf wrote:
> David Glasser wrote on Sat, 7 Jun 2008 at 23:11 -0700:
> > Um, I really don't want to drag this process out any longer. But as
> > far as I can tell, without backporting these revisions, every single
> > program written against the svn_repos API before 1.5.x (ie, the API
> > which does not include svn_repos_set_repos_capabilities) which tries
> > to commit anything to a repository with a start-commit hook will
> > dereference NULL (ie, crash) every time.
> >
> > Please please please somebody convince me that I am wrong in that
> > conclusion above. Because if we actually claim to care about API
> > users, that's pretty serious.
> >
>
> The conclusion is correct.
Ooops, I didn't realise that the 1.4.x API was affected.
Thanks for pointing this out.
> The CLIENT_CAPABILITIES member of svn_repos_t is only written by the
> new-in-1.5 svn_repos_remember_client_capabilities function, which isn't
> called by any other svn_repos_* function.
>
> However, svn_repos_fs_begin_txn_for_commit2 passes
> $x=CLIENT_CAPABILITIES to svn_repos__hooks_start_commit(capabilities=$x),
> which passes it to svn_cstring_join(strings=$x), which dereferences
> STRINGS in the termination condition of the first for() loop. Since
> STRINGS ultimately is CLIENT_CAPABILITIES, it is an uninitialised
> pointer being dereferenced.
>
> As Stefan suggested yesterday, another fix could be to initialise
> client_capabilities to an empty array when creating an svn_repos_t
> (and change the internal API in repos.h to disallow that array being
> NULL).
Also, there's another crash bug already waiting for 1.5.1, namely r31223.
Which is about "Fix segfault when svn_ra_open3 is passed a bogus URL such
as 'bogusURL'." This affects 1.4 API users as well, because svn_ra_open2
calls svn_ra_open3, passing the URL parameter through.
r31223 was scheduled for 1.5.1 and not 1.5.0 by Eric because the crash
cannot happen with our svn(1) client. Because this is true also for the
(r31620, r31622) group, I opted for scheduling it for 1.5.1 as well.
Otherwise I would have nominated it for 1.5.0, because yes, crashing APIs
are really bad.
If we put either fix into 1.5.0, it does not make much sense to exclude
the other.
I guess Hyrum's not gonna be happy about an -rc10, but I would not object
putting those fixes into 1.5.0 if more people think it's necessary.
Stefan
Re: [HEADS-UP] Need more backports to 1.5.0?
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Jun 08, 2008 at 12:22:37PM -0400, Mark Phippard wrote:
> On Sun, Jun 8, 2008 at 12:18 PM, Hyrum K. Wright
> <hy...@mail.utexas.edu> wrote:
>
> >> I guess Hyrum's not gonna be happy about an -rc10, but I would not object
> >> putting those fixes into 1.5.0 if more people think it's necessary.
> >
> > It's not that big of a deal for me personally; I'm just as anxious to get
> > 1.5.0 out the door as everybody else.
> >
> > I suggest we move the proposed fixes to the 1.5.0 section of STATUS. We can
> > continue to debate, and if needed, I'll roll rc10 tomorrow.
>
> It looks like both changes already have the votes required. So at
> least that is taken care of already. I agree someone should move them
> both in STATUS, so there is confusion about which changes we are
> including.
I've moved them to the candidate section in r31631.
Not to the approved section yet, because I still expect this discussion
to continue. Technically, the changes have enough votes to go into
the approved section, though.
Daniel and I have also scanned STATUS for other crash bugs, the only
one we could find is r31619, which Lieven said can wait for 1.5.1.
Stefan
Re: [HEADS-UP] Need more backports to 1.5.0?
Posted by Mark Phippard <ma...@gmail.com>.
On Sun, Jun 8, 2008 at 12:18 PM, Hyrum K. Wright
<hy...@mail.utexas.edu> wrote:
>> I guess Hyrum's not gonna be happy about an -rc10, but I would not object
>> putting those fixes into 1.5.0 if more people think it's necessary.
>
> It's not that big of a deal for me personally; I'm just as anxious to get
> 1.5.0 out the door as everybody else.
>
> I suggest we move the proposed fixes to the 1.5.0 section of STATUS. We can
> continue to debate, and if needed, I'll roll rc10 tomorrow.
It looks like both changes already have the votes required. So at
least that is taken care of already. I agree someone should move them
both in STATUS, so there is confusion about which changes we are
including.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [HEADS-UP] Need more backports to 1.5.0?
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Stefan Sperling wrote:
> On Sun, Jun 08, 2008 at 09:49:35AM +0300, Daniel Shahaf wrote:
>> David Glasser wrote on Sat, 7 Jun 2008 at 23:11 -0700:
>>> Um, I really don't want to drag this process out any longer. But as
>>> far as I can tell, without backporting these revisions, every single
>>> program written against the svn_repos API before 1.5.x (ie, the API
>>> which does not include svn_repos_set_repos_capabilities) which tries
>>> to commit anything to a repository with a start-commit hook will
>>> dereference NULL (ie, crash) every time.
>>>
>>> Please please please somebody convince me that I am wrong in that
>>> conclusion above. Because if we actually claim to care about API
>>> users, that's pretty serious.
>>>
>> The conclusion is correct.
>
> Ooops, I didn't realise that the 1.4.x API was affected.
> Thanks for pointing this out.
>
>> The CLIENT_CAPABILITIES member of svn_repos_t is only written by the
>> new-in-1.5 svn_repos_remember_client_capabilities function, which isn't
>> called by any other svn_repos_* function.
>>
>> However, svn_repos_fs_begin_txn_for_commit2 passes
>> $x=CLIENT_CAPABILITIES to svn_repos__hooks_start_commit(capabilities=$x),
>> which passes it to svn_cstring_join(strings=$x), which dereferences
>> STRINGS in the termination condition of the first for() loop. Since
>> STRINGS ultimately is CLIENT_CAPABILITIES, it is an uninitialised
>> pointer being dereferenced.
>>
>> As Stefan suggested yesterday, another fix could be to initialise
>> client_capabilities to an empty array when creating an svn_repos_t
>> (and change the internal API in repos.h to disallow that array being
>> NULL).
>
> Also, there's another crash bug already waiting for 1.5.1, namely r31223.
> Which is about "Fix segfault when svn_ra_open3 is passed a bogus URL such
> as 'bogusURL'." This affects 1.4 API users as well, because svn_ra_open2
> calls svn_ra_open3, passing the URL parameter through.
>
> r31223 was scheduled for 1.5.1 and not 1.5.0 by Eric because the crash
> cannot happen with our svn(1) client. Because this is true also for the
> (r31620, r31622) group, I opted for scheduling it for 1.5.1 as well.
> Otherwise I would have nominated it for 1.5.0, because yes, crashing APIs
> are really bad.
>
> If we put either fix into 1.5.0, it does not make much sense to exclude
> the other.
>
> I guess Hyrum's not gonna be happy about an -rc10, but I would not object
> putting those fixes into 1.5.0 if more people think it's necessary.
It's not that big of a deal for me personally; I'm just as anxious to
get 1.5.0 out the door as everybody else.
I suggest we move the proposed fixes to the 1.5.0 section of STATUS. We
can continue to debate, and if needed, I'll roll rc10 tomorrow.
-Hyrum
Re: [HEADS-UP] Need more backports to 1.5.0? (was: Re: you're
going to kill me, but... [Re: svn commit: r31625 -) branches/1.5.x]
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Jun 08, 2008 at 04:58:06PM -0400, Mark Phippard wrote:
> On Jun 8, 2008, at 4:06 PM, Stefan Sperling <st...@elego.de> wrote:
> > On Sun, Jun 08, 2008 at 12:36:03PM -0700, David Glasser wrote:
> >> Not that I oppose backporting r31223, but there's a difference
> >> between
> >> "crashes instead of throwing an error message on bad input" and
> >> "breaks any program that tries to commit over the repos layer if
> >> there's a start-commit hook".
> >
> > You're right.
> > I'm OK with backporting the (r31620, r31622) group only, and
> > scheduling r312
>
> +1 from me too.
Done in r31653.
Stefan
Re: [HEADS-UP] Need more backports to 1.5.0? (was: Re: you're going to kill me, but... [Re: svn commit: r31625 -) branches/1.5.x]
Posted by Mark Phippard <ma...@gmail.com>.
On Jun 8, 2008, at 4:06 PM, Stefan Sperling <st...@elego.de> wrote:
> On Sun, Jun 08, 2008 at 12:36:03PM -0700, David Glasser wrote:
>> On Sun, Jun 8, 2008 at 3:41 AM, Stefan Sperling <st...@elego.de>
>> wrote:
>>> Also, there's another crash bug already waiting for 1.5.1, namely
>>> r31223.
>>> Which is about "Fix segfault when svn_ra_open3 is passed a bogus
>>> URL such
>>> as 'bogusURL'." This affects 1.4 API users as well, because
>>> svn_ra_open2
>>> calls svn_ra_open3, passing the URL parameter through.
>>>
>>> r31223 was scheduled for 1.5.1 and not 1.5.0 by Eric because the
>>> crash
>>> cannot happen with our svn(1) client. Because this is true also
>>> for the
>>> (r31620, r31622) group, I opted for scheduling it for 1.5.1 as
>>> well.
>>> Otherwise I would have nominated it for 1.5.0, because yes,
>>> crashing APIs
>>> are really bad.
>>>
>>> If we put either fix into 1.5.0, it does not make much sense to
>>> exclude
>>> the other.
>>
>> Not that I oppose backporting r31223, but there's a difference
>> between
>> "crashes instead of throwing an error message on bad input" and
>> "breaks any program that tries to commit over the repos layer if
>> there's a start-commit hook".
>
> You're right.
> I'm OK with backporting the (r31620, r31622) group only, and
> scheduling r312
+1 from me too.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [HEADS-UP] Need more backports to 1.5.0? (was: Re: you're
going to kill me, but... [Re: svn commit: r31625 -) branches/1.5.x]
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Jun 08, 2008 at 12:36:03PM -0700, David Glasser wrote:
> On Sun, Jun 8, 2008 at 3:41 AM, Stefan Sperling <st...@elego.de> wrote:
> > Also, there's another crash bug already waiting for 1.5.1, namely r31223.
> > Which is about "Fix segfault when svn_ra_open3 is passed a bogus URL such
> > as 'bogusURL'." This affects 1.4 API users as well, because svn_ra_open2
> > calls svn_ra_open3, passing the URL parameter through.
> >
> > r31223 was scheduled for 1.5.1 and not 1.5.0 by Eric because the crash
> > cannot happen with our svn(1) client. Because this is true also for the
> > (r31620, r31622) group, I opted for scheduling it for 1.5.1 as well.
> > Otherwise I would have nominated it for 1.5.0, because yes, crashing APIs
> > are really bad.
> >
> > If we put either fix into 1.5.0, it does not make much sense to exclude
> > the other.
>
> Not that I oppose backporting r31223, but there's a difference between
> "crashes instead of throwing an error message on bad input" and
> "breaks any program that tries to commit over the repos layer if
> there's a start-commit hook".
You're right.
I'm OK with backporting the (r31620, r31622) group only, and
scheduling r31223 back to 1.5.1.
Stefan
Re: [HEADS-UP] Need more backports to 1.5.0? (was: Re: you're going to kill me, but... [Re: svn commit: r31625 -) branches/1.5.x]
Posted by David Glasser <gl...@davidglasser.net>.
On Sun, Jun 8, 2008 at 3:41 AM, Stefan Sperling <st...@elego.de> wrote:
> Also, there's another crash bug already waiting for 1.5.1, namely r31223.
> Which is about "Fix segfault when svn_ra_open3 is passed a bogus URL such
> as 'bogusURL'." This affects 1.4 API users as well, because svn_ra_open2
> calls svn_ra_open3, passing the URL parameter through.
>
> r31223 was scheduled for 1.5.1 and not 1.5.0 by Eric because the crash
> cannot happen with our svn(1) client. Because this is true also for the
> (r31620, r31622) group, I opted for scheduling it for 1.5.1 as well.
> Otherwise I would have nominated it for 1.5.0, because yes, crashing APIs
> are really bad.
>
> If we put either fix into 1.5.0, it does not make much sense to exclude
> the other.
Not that I oppose backporting r31223, but there's a difference between
"crashes instead of throwing an error message on bad input" and
"breaks any program that tries to commit over the repos layer if
there's a start-commit hook".
If people really want to release 1.5.0 without this backport, I'd hope
we'd at least include in big letters in the release announcement "Do
not upgrade if you use this with custom programs that commit directly
to your repository". Wording this well might be harder than just
waiting another week though :-)
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org