You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2008/06/03 16:20:27 UTC
SVN-DEV HELP NEEDED: What to do about the ra-get-log interface (Was:
Clarification for paths argument to svn_ra_get_log2)
C. Michael Pilato wrote:
> C. Michael Pilato wrote:
>> Martin von Gagern wrote:
>>> Resending, as I forgot to cc the list... sorry.
>>>
>>> C. Michael Pilato wrote:
>>> | I seem to recall that recently I realized that NULL and empty had two
>>> | different interpretations in 'svn log'. I think empty resulted in
>>> (b),
>>> | and (a surprise to me) NULL in (c).
>>>
>>> If that is indeed the case and intended this way, it would certainly
>>> merit documentation.
>>
>> Okay, looking in svn_repos_get_logs4(), the code does the following
>> pretty early:
>>
>> if (! paths)
>> paths = apr_array_make(pool, 0, sizeof(const char *));
>>
>> So it would seem that NULL and empty have the same meaning.
>
> Ohohoh. But that code isn't in the 1.4.x line. I see what you mean,
> now. (And maybe this explains what I *thought* I saw in testing recently.)
>
> So, in 1.4.x, svn_repos_get_logs3() does *not* have the code above. In
> fact, it explicitly treats a NULL 'paths' array as the same as "Gimme
> the logs for all revisions, regardless of what was changed in them."
> Somewhere along the 1.5.x way, that got dropped (probably by me, but
> maybe by Hyrum, as the 'log -g' stuff was being added). And that's bad.
>
> So, first of all, there's a bug in the trunk and in 1.5.x regarding the
> handling of NULL paths in svn_repos_get_logs4().
>
> Interestingly thouth, in 1.4.x there doesn't *appear* to be any way via
> the WebDAV RA layers to pass NULL paths. You can't do it through
> mod_dav_svn, because mod_dav_svn's log REPORT parser always instantiates
> an empty 'paths' array and puts provided paths (if any) into it. (And
> it doesn't seem to care if it never find paths in the REPORT.) svnserve
> and ra_local do the same. So, unless I'm overlooking something, in
> 1.4.x, you cannot make use of this special behavior regarding NULL paths
> via the RA interfaces.
>
> Looking at the 1.5.x codebase, I see the same basic behaviors as in 1.4.x.
>
> So I remain confused both about your compatability concern and the
> behavior I thought I saw the other day.
>
> By the way, I remember where I saw this discrepency now. It was when I
> was writing libsvn_client/merge.c:remove_noop_merge_ranges().
> Originally I passed NULL into the svn_ra_get_logs2() call, but I thought
> I was seeing every single revision coming back through the log_entry
> receiver. So I instead passed an empty array. I was testing against
> svn.collab.net, which was running a 1.5.x RC at the time.
>
Okay, in IRC, Martin clarified some things, namely that it isn't just a NULL
'paths' array in svn_repos_get_logs3() that get special treatment -- it's
also the case where 'paths' is non-NULL and contains exactly one empty path
(which refers to the root of the repository).
So while there is no known way of getting NULL through the RA APIs, there
*was* a way of getting a single root-pointing path there. In the past, some
third-party consumers of our APIs (such as bzr-svn) were violating the
svn_ra_get_logs2() interface by passing absolute paths instead of paths
relative to the session URL through this interface. The RA implementations
were sloppy in their handling, and threw everything through svn_path_join(),
which of course effectively discard the base path when the to-be-added bit
is absolute. This effectively meant that you could request logs of any path
in the repository with any arbitrary session URL.
Some time many months ago, I got frustrated by the sheer number of bugs that
were coming out of inconsistent use of our RA layer within our own codebase,
so I taught the RA interface how to enforce the rules it claimed to have had
since its inception. We now know one side-effect of that change: those
folks who were misusing this API are no longer able to do so, and it is
causing complications for them.
Martin tells me that bzr-svn was banking on this ability to request all
revision logs from any location, because reparenting to the root URL and
fetching logs from there might not be permissible due to access
restrictions. So, if we assume a scenario where I'm allowed to read /trunk
but not /, presumably bzr-svn would use /trunk for the session URL (which is
allowed), but then ask for logs on /. Because we only validate the returned
logs, this would work fine -- we'd just wind up return incomplete metadata
for any revisions that contained changed to paths outside of /trunk. As a
general statement about the usability of Subversion in auth-restricted
scenarios, this kinda makes sense. (We've seen other situations where, say,
I can't do a move of /trunk/foo to /trunk/bar because we try to anchor the
commit editor at /. Those kinds of things are easy to explain to a
Subversion developer, but users are left bewildered.)
So, what to do? Do we explicitly allow absolute paths through all of our RA
interfaces? (Please don't make inconsistent policy here and give per-method
exceptions.) Do we stick with the published APIs and send our best wishes
and a vase of flowers to folks who were previously misusing the APIs and
have now been caught doing so?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
Posted by Karl Fogel <kf...@red-bean.com>.
"C. Michael Pilato" <cm...@collab.net> writes:
> For the purposes of the 1.5.0 release, I agree. I tightened that
> sloppy code up for a reason, and found a bunch of bugs as a result.
> I'd like *not* to loosen it up again just to cater to folks who happen
> to like the way one of our bugs pans out.
>
> Now, that our current RA model causes many operations that needn't
> fail to do so is another problem altogether. But for API consumers
> that have been well-behaved, that has always been true, so we can (if
> we so desire) address those matters in a future release.
Also, having empty 'paths' do one thing and NULL 'paths' do a different
thing would just be bad API design, IMHO. +1 on leaving it as-is in
1.5.0.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
(Was: Clarification for paths argument to svn_ra_get_log2)
Posted by "C. Michael Pilato" <cm...@collab.net>.
Mark Phippard wrote:
> On Tue, Jun 3, 2008 at 12:20 PM, C. Michael Pilato <cm...@collab.net> wrote:
>
>> So, what to do? Do we explicitly allow absolute paths through all of our RA
>> interfaces? (Please don't make inconsistent policy here and give per-method
>> exceptions.) Do we stick with the published APIs and send our best wishes
>> and a vase of flowers to folks who were previously misusing the APIs and
>> have now been caught doing so?
>
> I imagine you are trying to be helpful and sympathetic here to the
> bzr-svn team, but don't we essentially have to stick with the
> published API? It seems like we have always chosen that as our
> answer, and I do not see why this would be different.
>
> We do not want to get into a "Microsoft" situation here where we try
> to support our bugs forever because we discover there are consumers
> that depended on them in their usage of the API. I realize that is an
> exaggeration since we would really just be expanding what the API can
> do, but it is an unknown amount of new code to actually modify the
> API's to support this new concept.
For the purposes of the 1.5.0 release, I agree. I tightened that sloppy
code up for a reason, and found a bunch of bugs as a result. I'd like *not*
to loosen it up again just to cater to folks who happen to like the way one
of our bugs pans out.
Now, that our current RA model causes many operations that needn't fail to
do so is another problem altogether. But for API consumers that have been
well-behaved, that has always been true, so we can (if we so desire) address
those matters in a future release.
(And yes, I was trying to be sympathic. In a recent self-exam of my
Spiritual Gifts, I scored a disappointing 3 out of 25 possible points in the
area of Mercy.)
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface (Was: Clarification for paths argument to svn_ra_get_log2)
Posted by Mark Phippard <ma...@gmail.com>.
On Tue, Jun 3, 2008 at 12:20 PM, C. Michael Pilato <cm...@collab.net> wrote:
> So, what to do? Do we explicitly allow absolute paths through all of our RA
> interfaces? (Please don't make inconsistent policy here and give per-method
> exceptions.) Do we stick with the published APIs and send our best wishes
> and a vase of flowers to folks who were previously misusing the APIs and
> have now been caught doing so?
I imagine you are trying to be helpful and sympathetic here to the
bzr-svn team, but don't we essentially have to stick with the
published API? It seems like we have always chosen that as our
answer, and I do not see why this would be different.
We do not want to get into a "Microsoft" situation here where we try
to support our bugs forever because we discover there are consumers
that depended on them in their usage of the API. I realize that is an
exaggeration since we would really just be expanding what the API can
do, but it is an unknown amount of new code to actually modify the
API's to support this new concept.
--
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: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
Posted by "C. Michael Pilato" <cm...@collab.net>.
C. Michael Pilato wrote:
> C. Michael Pilato wrote:
>> C. Michael Pilato wrote:
>>> Martin von Gagern wrote:
>>>> Resending, as I forgot to cc the list... sorry.
>>>>
>>>> C. Michael Pilato wrote:
>>>> | I seem to recall that recently I realized that NULL and empty had two
>>>> | different interpretations in 'svn log'. I think empty resulted in
>>>> (b),
>>>> | and (a surprise to me) NULL in (c).
>>>>
>>>> If that is indeed the case and intended this way, it would certainly
>>>> merit documentation.
>>>
>>> Okay, looking in svn_repos_get_logs4(), the code does the following
>>> pretty early:
>>>
>>> if (! paths)
>>> paths = apr_array_make(pool, 0, sizeof(const char *));
>>>
>>> So it would seem that NULL and empty have the same meaning.
>>
>> Ohohoh. But that code isn't in the 1.4.x line. I see what you mean,
>> now. (And maybe this explains what I *thought* I saw in testing
>> recently.)
Just to tie up this line of thought, the 1.4.x svn_repos_get_logs3() logic
special-cases:
paths == NULL
paths->nelts == 1 && paths[0] == ""
The 1.5.x svn_repos_get_logs4() logic special-cases:
paths == NULL
paths->nelts == 0 <--- this is new
paths->nelts == 1 && paths[0] == ""
I'm not positive, but I think that in 1.4.x, if you passed an empty array,
you got no logs. In 1.5.x., that's not the case -- passing an empty paths
array results in getting revisions for the root of the repository. So there
*was* a real change of behavior to the API.
Moving to the RA layers, we have four implementations, but they all share
the common feature that none of them allows a NULL 'paths' array to hit the
svn_repos_get_logs* functions. They all start with an empty array, and add
to it any paths requested. This means, though, that there's still a
discrepancy between 1.4.x and 1.5.x -- the handling of the empty paths array.
Backing all the way up to the client layer, things get okay again, because
svn_client_log4() ensures that there is always at least one path in the array.
So, to summarize:
* The repos layer behaves differently in 1.5.x than in 1.4.x when handed
an empty 'paths' array.
* The RA layer behaves differently in 1.5.x than in 1.4.x when handed
either an empty or a NULL 'paths' array (because both result ultimately
in an empty paths array being handed to the repos layer).
* The client layer behaves the same in 1.5.x and 1.4.x because it always
provides a non-empty paths array to the RA layer.
Fortunately (for us), I think the docstrings in the RA and repos layers are
sufficiently vague as to not imply *what* would happen if you passed an
empty array. So, in my opinion, we have the opportunity in 1.5.0 to define
that behavior (which we effectively have: empty paths == NULL paths ==
paths with one "" member), and (now) in 1.5.1 to fix the docstrings for the
repos and RA functions so that this is all made clear.
(Attached is a Python script I've been using against svn.collab.net and
svn.red-bean.com to sanity-check some of this stuff.)
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
Posted by Karl Fogel <kf...@red-bean.com>.
Stefan Sperling <st...@elego.de> writes:
> On Wed, Jun 04, 2008 at 08:56:31AM -0400, Mark Phippard wrote:
>> On Wed, Jun 4, 2008 at 8:48 AM, C. Michael Pilato <cm...@collab.net> wrote:
>> > We do not anticipate making a change here, and I'd love to see RC9 rolled
>> > ASAP (though there appears to be an obvious-fix item still in STATUS).
>>
>> RC9 was rolled last night. When I looked at STATUS there were no
>
> I added a totally obvious fix for the INSTALL file today (mention
> neon-0.28 support).
For what it's worth:
When the time comes to do the real release, I'd think it would be fine
for Hyrum to incorporate whatever INSTALL or other doc changes he wants.
We can diff the final release against RC9 and see that there are no
changes in sensitive areas, after all (and I would be doing so anyway).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Jun 04, 2008 at 08:56:31AM -0400, Mark Phippard wrote:
> On Wed, Jun 4, 2008 at 8:48 AM, C. Michael Pilato <cm...@collab.net> wrote:
> > We do not anticipate making a change here, and I'd love to see RC9 rolled
> > ASAP (though there appears to be an obvious-fix item still in STATUS).
>
> RC9 was rolled last night. When I looked at STATUS there were no
I added a totally obvious fix for the INSTALL file today (mention
neon-0.28 support).
Stefan
Re: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
Posted by Mark Phippard <ma...@gmail.com>.
On Wed, Jun 4, 2008 at 8:48 AM, C. Michael Pilato <cm...@collab.net> wrote:
> We do not anticipate making a change here, and I'd love to see RC9 rolled
> ASAP (though there appears to be an obvious-fix item still in STATUS).
RC9 was rolled last night. When I looked at STATUS there were no
remaining items in it.
--
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: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
Posted by "C. Michael Pilato" <cm...@collab.net>.
Mark Phippard wrote:
> On Tue, Jun 3, 2008 at 3:45 PM, Justin Erenkrantz <ju...@erenkrantz.com> wrote:
>> On Tue, Jun 3, 2008 at 12:16 PM, C. Michael Pilato <cm...@collab.net> wrote:
>>> I'm not saying its a sane configuration, of course. But prior to
>>> mod_authz_svn being created, it was through Apache configury like this that
>>> we instructed folks to do their access control.
>> Not permitting access into the special URI space is kinda pointless -
>> hence, mod_authz_svn. I don't think we should bend over backwards to
>> support broken configs. -- justin
>
> Is it "safe" for Hyrum to roll RC9 tonight with the fix for
> depth=empty and that changelist fix? Or do we anticipate making a
> change here? It sounds like we are not making any changes, but I
> wanted to be sure.
We do not anticipate making a change here, and I'd love to see RC9 rolled
ASAP (though there appears to be an obvious-fix item still in STATUS).
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
Posted by Mark Phippard <ma...@gmail.com>.
On Tue, Jun 3, 2008 at 3:45 PM, Justin Erenkrantz <ju...@erenkrantz.com> wrote:
> On Tue, Jun 3, 2008 at 12:16 PM, C. Michael Pilato <cm...@collab.net> wrote:
>> I'm not saying its a sane configuration, of course. But prior to
>> mod_authz_svn being created, it was through Apache configury like this that
>> we instructed folks to do their access control.
>
> Not permitting access into the special URI space is kinda pointless -
> hence, mod_authz_svn. I don't think we should bend over backwards to
> support broken configs. -- justin
Is it "safe" for Hyrum to roll RC9 tonight with the fix for
depth=empty and that changelist fix? Or do we anticipate making a
change here? It sounds like we are not making any changes, but I
wanted to be sure.
--
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: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Tue, Jun 3, 2008 at 12:16 PM, C. Michael Pilato <cm...@collab.net> wrote:
> I'm not saying its a sane configuration, of course. But prior to
> mod_authz_svn being created, it was through Apache configury like this that
> we instructed folks to do their access control.
Not permitting access into the special URI space is kinda pointless -
hence, mod_authz_svn. I don't think we should bend over backwards to
support broken configs. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
Posted by "C. Michael Pilato" <cm...@collab.net>.
Karl Fogel wrote:
> "C. Michael Pilato" <cm...@collab.net> writes:
>>> Why should access restrictions prevent someone from merely anchoring an
>>> RA session at a particular URL, and then running log requests based from
>>> that anchor? The access controls should, of course, limit what
>>> responses come back from the request, but I don't see why they should
>>> prevent what Martin assumes "might not be permissible".
>> Consider a situation in which mod_dav_svn (not mod_authz_svn) is
>> configured to disallow any read access to paths outside of /trunk.
>> Before Subversion even gets the chance to field a log REPORT request
>> aimed at the root of the repository, mod_dav_svn prevents the request
>> from succeeding.
>
> That's what I'm questioning. Why does mod_dav_svn behave like this?
> Because it's easier to implement?
Does it have a choice?
I think we're thinking of different deployment scenarios. I'm talking about
something like:
<Location /repo>
DAV svn
SVNPath /var/svn/repo
### bits that disallow access here
</Location>
<Location /repo/trunk
### bits that allow access to trunk here
</Location>
IIUC, a request to /repo (or to /repo/something-not-trunk) wouldn't even get
to mod_dav_svn for processing because it fails the higher-level Apache authz
requirements.
I'm not saying its a sane configuration, of course. But prior to
mod_authz_svn being created, it was through Apache configury like this that
we instructed folks to do their access control.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
Posted by Karl Fogel <kf...@red-bean.com>.
"C. Michael Pilato" <cm...@collab.net> writes:
>> Why should access restrictions prevent someone from merely anchoring an
>> RA session at a particular URL, and then running log requests based from
>> that anchor? The access controls should, of course, limit what
>> responses come back from the request, but I don't see why they should
>> prevent what Martin assumes "might not be permissible".
>
> Consider a situation in which mod_dav_svn (not mod_authz_svn) is
> configured to disallow any read access to paths outside of /trunk.
> Before Subversion even gets the chance to field a log REPORT request
> aimed at the root of the repository, mod_dav_svn prevents the request
> from succeeding.
That's what I'm questioning. Why does mod_dav_svn behave like this?
Because it's easier to implement?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
Posted by "C. Michael Pilato" <cm...@collab.net>.
Karl Fogel wrote:
> "C. Michael Pilato" <cm...@collab.net> writes:
>> Martin tells me that bzr-svn was banking on this ability to request
>> all revision logs from any location, because reparenting to the root
>> URL and fetching logs from there might not be permissible due to
>> access restrictions. So, if we assume a scenario where I'm allowed to
>> read /trunk but not /, presumably bzr-svn would use /trunk for the
>> session URL (which is allowed), but then ask for logs on /. Because
>> we only validate the returned logs, this would work fine -- we'd just
>> wind up return incomplete metadata for any revisions that contained
>> changed to paths outside of /trunk. As a general statement about the
>> usability of Subversion in auth-restricted scenarios, this kinda makes
>> sense. (We've seen other situations where, say, I can't do a move of
>> /trunk/foo to /trunk/bar because we try to anchor the commit editor at
>> /. Those kinds of things are easy to explain to a Subversion
>> developer, but users are left bewildered.)
>>
>> So, what to do? Do we explicitly allow absolute paths through all of
>> our RA interfaces? (Please don't make inconsistent policy here and
>> give per-method exceptions.) Do we stick with the published APIs and
>> send our best wishes and a vase of flowers to folks who were
>> previously misusing the APIs and have now been caught doing so?
>
> Why should access restrictions prevent someone from merely anchoring an
> RA session at a particular URL, and then running log requests based from
> that anchor? The access controls should, of course, limit what
> responses come back from the request, but I don't see why they should
> prevent what Martin assumes "might not be permissible".
Consider a situation in which mod_dav_svn (not mod_authz_svn) is configured
to disallow any read access to paths outside of /trunk. Before Subversion
even gets the chance to field a log REPORT request aimed at the root of the
repository, mod_dav_svn prevents the request from succeeding.
However, if the request was aimed at /trunk but its contents instructed
Subversion to grab all revision logs, then the "internal GET request" logic
we use in mod_dav_svn would still work correctly -- the nested GETs would
pass back through mod_dav_svn, which would permit stuff in and under /trunk
and disallow the rest -- and the resulting log stream would be as you'd
expect: all revisions present, but with metadata "cleaned up" per our
security rules.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: SVN-DEV HELP NEEDED: What to do about the ra-get-log interface
Posted by Karl Fogel <kf...@red-bean.com>.
"C. Michael Pilato" <cm...@collab.net> writes:
> Martin tells me that bzr-svn was banking on this ability to request
> all revision logs from any location, because reparenting to the root
> URL and fetching logs from there might not be permissible due to
> access restrictions. So, if we assume a scenario where I'm allowed to
> read /trunk but not /, presumably bzr-svn would use /trunk for the
> session URL (which is allowed), but then ask for logs on /. Because
> we only validate the returned logs, this would work fine -- we'd just
> wind up return incomplete metadata for any revisions that contained
> changed to paths outside of /trunk. As a general statement about the
> usability of Subversion in auth-restricted scenarios, this kinda makes
> sense. (We've seen other situations where, say, I can't do a move of
> /trunk/foo to /trunk/bar because we try to anchor the commit editor at
> /. Those kinds of things are easy to explain to a Subversion
> developer, but users are left bewildered.)
>
> So, what to do? Do we explicitly allow absolute paths through all of
> our RA interfaces? (Please don't make inconsistent policy here and
> give per-method exceptions.) Do we stick with the published APIs and
> send our best wishes and a vase of flowers to folks who were
> previously misusing the APIs and have now been caught doing so?
Why should access restrictions prevent someone from merely anchoring an
RA session at a particular URL, and then running log requests based from
that anchor? The access controls should, of course, limit what
responses come back from the request, but I don't see why they should
prevent what Martin assumes "might not be permissible".
If there is a bug here, perhaps it's in the way the server enforces
access restrictions?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org