You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ingo Adler <de...@synacon.ch> on 2005/10/16 09:01:37 UTC

"Locked" messages useless

Hi,

I try to lock a file which is already locked and get the message:

svn: Path '/trunk/sprints/Sprints.mdb' is already locked by user 'iar' 
in filesystem 'C:/Daten/svnroot/myrepo/db'

What shall I do with the information "in filesystem 
'C:/Daten/svnroot/myrepo/db'"?

It tells me where the repository is. But I cannot access the repository 
directly. I use Subversion for that. I wouldn't want to access the 
repository directly. Would it help, if I could access it? Probably not. 
I don't even want to know where it is physically - I use svn:// and http://.

Another information would be much more helpful, details about the locker.
- on computer 'ABC' with ip '123.456.678' in location 
'c:\projects\src\sprint\sprint.mdb'.

Then I could start to look for it. For instance I could ask 'iar' and 
point him to the locked file.
I think this information would be more useful than some server internals 
which nobody cares about.

Ingo.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2005-10-16 at 14:59 -0400, John Szakmeister wrote:
> I thought a while back we had decided that the location of the repository on 
> the filesystem was sensitive, and shouldn't be divulged in anything except 
> Apache's access/error logs.  IIRC, several people went through and removed 
> the repository path from a bunch of error messages in mod_dav_svn.

In some specific cases, yes.  But there's no universal way to remove the
repository path from error messages.  If the error message was generated
in an svn_io_* function, for instance, it's very hard for an upper layer
to recognize that and take out the repository path.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by John Szakmeister <jo...@szakmeister.net>.
On Sunday 16 October 2005 12:48, Greg Hudson wrote:
> On Sun, 2005-10-16 at 16:43 +0100, Philip Martin wrote:
> > > svn: Path '/trunk/sprints/Sprints.mdb' is already locked by user 'iar'
> > > in filesystem 'C:/Daten/svnroot/myrepo/db'
> >
> > When using http:// the repository location will get stripped from the
> > error message as a security measure.  Perhaps svnserve should be doing
> > the same?
>
> I'd say that this error message should not be using the filesystem
> location in the first place.
>
> Also, http does not uniformly strip repository locations.  Run "svn log
> -r 80000" in your svn working directory to find out that svn.collab.net
> puts the Subversion repository at /usr/www/repositories/svn/db.

I thought a while back we had decided that the location of the repository on 
the filesystem was sensitive, and shouldn't be divulged in anything except 
Apache's access/error logs.  IIRC, several people went through and removed 
the repository path from a bunch of error messages in mod_dav_svn.

-John

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by "C. Michael Pilato" <cm...@collab.net>.
"Peter N. Lundblad" <pe...@famlundblad.se> writes:

> > (While we're on the subject, shouldn't those two "err.c" files be unified,
> > given that only about 2 of the 25 functions differ at all between them?)
> >
> The question is *where* this file should go. libsvn_subr doesn't seem
> right. Do we want a libsvn_fs_util just for this? (I think this has been
> discussed before.)

Wanna version a symlink from one to the other?  :-)

(Yeah, yeah, the Windows build...)


-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Remove unused functions [was: Sharing libsvn_fs_* internals]

Posted by Greg Hudson <gh...@MIT.EDU>.
On Thu, 2005-11-03 at 01:45 +0000, Julian Foad wrote:
> > Although the FSFS code is based off the BDB code, I don't think there's
> > much code to share (there's a path canonicalization function and some
> > key generation functions, which shouldn't exist).  Much of the shared
> 
> Good grief - stuff that "shouldn't exist"!

Sorry, I have no idea why I wrote "which shouldn't exist" in that
parenthetical.  The key generation functions should exist as far as I
know.  I think I meant to say that those don't amount to much code, and
my brain wandered.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

[PATCH] Remove unused functions [was: Sharing libsvn_fs_* internals]

Posted by Julian Foad <ju...@btopenworld.com>.
Greg Hudson wrote:
> On Wed, 2005-11-02 at 13:33 +0000, Julian Foad wrote:
> 
>>To recap, I'm talking about the two files subversion/libsvn_fs_{base,fs}/err.c 
>>being almost identical.
> 
> Note that most of the functions in libsvn_fs_fs/err.c are not used.

Good grief - so they're not.  And a few in libsvn_fs_base/err.c too.

The attached patch removes them.

> Prior to the addition of lock.c, there were only four uses of the error
> helpers in all of FSFS.
> 
> Also note that having error helper functions degrades the utility of
> file and line numbers in the --enable-maintainer-mode messages, and is
> not a practice we use widely outside the FS code.

Yes - I thought it was a bit odd, but I'm not going to be the one to change it.


>>It's not just those files.  Presumably, if the two filesystems use the same set 
>>of error messages, then there is much in common in their implementation.  I 
>>haven't looked further in detail.
> 
> Although the FSFS code is based off the BDB code, I don't think there's
> much code to share (there's a path canonicalization function and some
> key generation functions, which shouldn't exist).  Much of the shared

Good grief - stuff that "shouldn't exist"!  The attached patch removes the 
completely unused key-generation functions, but doesn't address those that are 
still being used but shouldn't be.

> logic is at the higher levels ("how to open a path"), but calls out to
> different logic at the lower levels ("how to open a node-rev and read
> directory entries").  It's possible to share higher-level logic with
> lower-level differences, but it tends to be more awkward than sharing
> lower-level logic.
> 
> Also, there's a fair amount of cleanup which could be done to the FSFS
> code which would render a lot of the BDB code vestigial.  For instance,
> the dag layer doesn't really do much in FSFS, although eliminating it
> would take some doing.

Well, it would be worth cleaning up the FSFS code now that it's intended to be 
widely used and supported.  Any volunteers?


>>At the moment, while I'd be happy to go the clean, layered, potentially public 
>>route, I don't think the amount of sharing is large enough to require that, and 
>>so I'd be happy to put more shared stuff in libsvn_fs which is simpler in the 
>>short term, as long as we keep our eyes open for the point when it starts to 
>>get unreasonably complex and do the refactoring then.
> 
> We actually did put small amounts of shared code into libsvn_fs
> initially, but ran into problems on OSX and Windows.

Thanks for these words of wisdom and experience, Greg.

- Julian

Re: Sharing libsvn_fs_* internals [was: "Locked" messages useless]

Posted by Greg Hudson <gh...@MIT.EDU>.
On Wed, 2005-11-02 at 13:33 +0000, Julian Foad wrote:
> To recap, I'm talking about the two files subversion/libsvn_fs_{base,fs}/err.c 
> being almost identical.

Note that most of the functions in libsvn_fs_fs/err.c are not used.
Prior to the addition of lock.c, there were only four uses of the error
helpers in all of FSFS.

Also note that having error helper functions degrades the utility of
file and line numbers in the --enable-maintainer-mode messages, and is
not a practice we use widely outside the FS code.

> It's not just those files.  Presumably, if the two filesystems use the same set 
> of error messages, then there is much in common in their implementation.  I 
> haven't looked further in detail.

Although the FSFS code is based off the BDB code, I don't think there's
much code to share (there's a path canonicalization function and some
key generation functions, which shouldn't exist).  Much of the shared
logic is at the higher levels ("how to open a path"), but calls out to
different logic at the lower levels ("how to open a node-rev and read
directory entries").  It's possible to share higher-level logic with
lower-level differences, but it tends to be more awkward than sharing
lower-level logic.

Also, there's a fair amount of cleanup which could be done to the FSFS
code which would render a lot of the BDB code vestigial.  For instance,
the dag layer doesn't really do much in FSFS, although eliminating it
would take some doing.

> At the moment, while I'd be happy to go the clean, layered, potentially public 
> route, I don't think the amount of sharing is large enough to require that, and 
> so I'd be happy to put more shared stuff in libsvn_fs which is simpler in the 
> short term, as long as we keep our eyes open for the point when it starts to 
> get unreasonably complex and do the refactoring then.

We actually did put small amounts of shared code into libsvn_fs
initially, but ran into problems on OSX and Windows.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Sharing libsvn_fs_* internals [was: "Locked" messages useless]

Posted by Branko Čibej <br...@xbc.nu>.
C. Michael Pilato wrote:
> I spent a few hours thinking about the next generation of our
> filesystem last night, and I'm convinced at this point that to undergo
> some sort of major code unification effort today would be a waste.
> Better to begin thinking about how to properly segregate the business
> logic from the storage format peculiarities in the next generation of
> the filesystem;
>   
Oh, yes...

> better still to invest real effort into deciding if
> we've a need for multiple backends at all.
>   
Actually, I think we want to keep multiple backends for aesthetic 
reasons if nothing else. It will help with maintaining the separation 
you mention above.

Of course, we first have to get the separation right.

(And make the BDB backend design sane; I still think that FSFS 
outperforms BDB because its storage model and implementation are 
designed better, not because a filesystem is "intrinsically" faster -- 
at least, for larger repositories, one would expect that a proper 
database with indexes, etc., would scale better.)

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Sharing libsvn_fs_* internals [was: "Locked" messages useless]

Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:
> 
> The thing to consider here is that you can share source files between 
> libsvn_fs_fs and libsvn_fs_base *without* creating a new shared library. 
> So both libraries will be a bit bigger. Big deal. The overhead of 
> loading yet another .so is probably much bigger than the overhead of 
> having a slightly larger .so.

That's a good point which I knew in theory but had overlooked in this 
discussion.  Thanks.  However it seems that there really isn't much that should 
be shared.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Sharing libsvn_fs_* internals [was: "Locked" messages useless]

Posted by Branko Čibej <br...@xbc.nu>.
Julian Foad wrote:
> Peter N. Lundblad wrote:
>> On Wed, 2 Nov 2005, C. Michael Pilato wrote:
>>
>>> I'm fine with the circular dependency.  The error stuff can move into
>>> libsvn_fs, as can the svn_fs_*__canonicalize_abspath().  I caution
>>> against futzing with anything but such low-hanging fruit.
>>
>> How would that work on Windows? Currently, we only have a circular
>> dependency regarding struct definitions. We can't have it on 
>> functions (or
>> other symbols). We could do a hack like wrapper_template.h in libsvn_ra,
>> but I like to avoid such if possible.
>
> I'll take that as a declaration that circular dependencies at the 
> object code level are not allowed in our Windows build.
That's correct.

> That's fine; it's not a good idea to structure code libraries that way 
> anyway.
Oh yes.

> So, that's about the end of it as far as I'm concerned.  We can move 
> source-level-only things to libsvn_fs, but I don't see any of those at 
> the moment.  Anything more drastic, given the rest of the comments in 
> this thread, I'm not going to consider.
The thing to consider here is that you can share source files between 
libsvn_fs_fs and libsvn_fs_base *without* creating a new shared library. 
So both libraries will be a bit bigger. Big deal. The overhead of 
loading yet another .so is probably much bigger than the overhead of 
having a slightly larger .so.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Sharing libsvn_fs_* internals [was: "Locked" messages useless]

Posted by Julian Foad <ju...@btopenworld.com>.
Peter N. Lundblad wrote:
> On Wed, 2 Nov 2005, C. Michael Pilato wrote:
> 
>>I'm fine with the circular dependency.  The error stuff can move into
>>libsvn_fs, as can the svn_fs_*__canonicalize_abspath().  I caution
>>against futzing with anything but such low-hanging fruit.
> 
> How would that work on Windows? Currently, we only have a circular
> dependency regarding struct definitions. We can't have it on functions (or
> other symbols). We could do a hack like wrapper_template.h in libsvn_ra,
> but I like to avoid such if possible.

I'll take that as a declaration that circular dependencies at the object code 
level are not allowed in our Windows build.  That's fine; it's not a good idea 
to structure code libraries that way anyway.

So, that's about the end of it as far as I'm concerned.  We can move 
source-level-only things to libsvn_fs, but I don't see any of those at the 
moment.  Anything more drastic, given the rest of the comments in this thread, 
I'm not going to consider.

Thanks for the discussion.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Sharing libsvn_fs_* internals [was: "Locked" messages useless]

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 2 Nov 2005, C. Michael Pilato wrote:

> Julian Foad <ju...@btopenworld.com> writes:
>
> > potentially public route, I don't think the amount of sharing is large
> > enough to require that, and so I'd be happy to put more shared stuff
> > in libsvn_fs which is simpler in the short term, as long as we keep
> > our eyes open for the point when it starts to get unreasonably complex
> > and do the refactoring then.
> >
...
> I'm fine with the circular dependency.  The error stuff can move into
> libsvn_fs, as can the svn_fs_*__canonicalize_abspath().  I caution
> against futzing with anything but such low-hanging fruit.
>
How would that work on Windows? Currently, we only have a circular
dependency regarding struct definitions. We can't have it on functions (or
other symbols). We could do a hack like wrapper_template.h in libsvn_ra,
but I like to avoid such if possible.

//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Sharing libsvn_fs_* internals [was: "Locked" messages useless]

Posted by "Glenn A. Thompson" <gt...@cdr.net>.
>Let's not get ahead of ourselves, here.  I said exactly as much as I
>meant to say -- no less, no more.
>
>  
>
Okay, Okay:-)

gat


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Sharing libsvn_fs_* internals [was: "Locked" messages useless]

Posted by "C. Michael Pilato" <cm...@collab.net>.
"Glenn A. Thompson" <gt...@cdr.net> writes:

> >There are quite a few similarities in the logic of the backend.  The
> >foremost difference is that in the BDB code, there are 'trail'
> >arguments all over the map, whereas the FSFS code has none.  This is a
> >pretty significant difference, though.
> >
> >I spent a few hours thinking about the next generation of our
> >filesystem last night, and I'm convinced at this point that to undergo
> >some sort of major code unification effort today would be a waste.
> >Better to begin thinking about how to properly segregate the business
> >logic from the storage format peculiarities in the next generation of
> >the filesystem; better still to invest real effort into deciding if
> >we've a need for multiple backends at all.
> >
> >
> Wow! are you serious?  A single backend as in no more vtable at the API?
> What release would you see this at?

Let's not get ahead of ourselves, here.  I said exactly as much as I
meant to say -- no less, no more.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Sharing libsvn_fs_* internals [was: "Locked" messages useless]

Posted by "Glenn A. Thompson" <gt...@cdr.net>.
>There are quite a few similarities in the logic of the backend.  The
>foremost difference is that in the BDB code, there are 'trail'
>arguments all over the map, whereas the FSFS code has none.  This is a
>pretty significant difference, though.
>
>I spent a few hours thinking about the next generation of our
>filesystem last night, and I'm convinced at this point that to undergo
>some sort of major code unification effort today would be a waste.
>Better to begin thinking about how to properly segregate the business
>logic from the storage format peculiarities in the next generation of
>the filesystem; better still to invest real effort into deciding if
>we've a need for multiple backends at all.
>
>  
>
Wow! are you serious?  A single backend as in no more vtable at the API?
What release would you see this at?

gat


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Sharing libsvn_fs_* internals [was: "Locked" messages useless]

Posted by "C. Michael Pilato" <cm...@collab.net>.
Julian Foad <ju...@btopenworld.com> writes:

> To recap, I'm talking about the two files
> subversion/libsvn_fs_{base,fs}/err.c being almost identical.  I've
> just committed cosmetic changes to remove spurious differences so that
> they can be compared side-by-side more easily. Likewise their header
> files err.h.  The remaining differences are tiny.
> 
> It's not just those files.  Presumably, if the two filesystems use the
> same set of error messages, then there is much in common in their
> implementation.  I haven't looked further in detail.

There are quite a few similarities in the logic of the backend.  The
foremost difference is that in the BDB code, there are 'trail'
arguments all over the map, whereas the FSFS code has none.  This is a
pretty significant difference, though.

I spent a few hours thinking about the next generation of our
filesystem last night, and I'm convinced at this point that to undergo
some sort of major code unification effort today would be a waste.
Better to begin thinking about how to properly segregate the business
logic from the storage format peculiarities in the next generation of
the filesystem; better still to invest real effort into deciding if
we've a need for multiple backends at all.

> At the moment, while I'd be happy to go the clean, layered,
> potentially public route, I don't think the amount of sharing is large
> enough to require that, and so I'd be happy to put more shared stuff
> in libsvn_fs which is simpler in the short term, as long as we keep
> our eyes open for the point when it starts to get unreasonably complex
> and do the refactoring then.
> 
> That said, I'm not going to do anything unless those who work in that
> part of the code base want it.

I'm fine with the circular dependency.  The error stuff can move into
libsvn_fs, as can the svn_fs_*__canonicalize_abspath().  I caution
against futzing with anything but such low-hanging fruit.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Sharing libsvn_fs_* internals [was: "Locked" messages useless]

Posted by Julian Foad <ju...@btopenworld.com>.
Peter N. Lundblad wrote:
> On Tue, 1 Nov 2005, Julian Foad wrote:
> 
>>(While we're on the subject, shouldn't those two "err.c" files be unified,
>>given that only about 2 of the 25 functions differ at all between them?)

To recap, I'm talking about the two files subversion/libsvn_fs_{base,fs}/err.c 
being almost identical.  I've just committed cosmetic changes to remove 
spurious differences so that they can be compared side-by-side more easily. 
Likewise their header files err.h.  The remaining differences are tiny.

It's not just those files.  Presumably, if the two filesystems use the same set 
of error messages, then there is much in common in their implementation.  I 
haven't looked further in detail.


> The question is *where* this file should go. libsvn_subr doesn't seem
> right. Do we want a libsvn_fs_util just for this? (I think this has been
> discussed before.)

Definitely not libsvn_subr.  It's some private implementation that's common to 
the two existing FS back-ends.  So, yes, the logical place is a 
"libsvn_fs_util" or "libsvn_fs_impl", and the question is whether that is 
worthwhile "just for this".

I notice that the fs_* back-ends already have a private and circular dependency 
on "../libsvn_fs/fs-loader.h", to share the implementation of "svn_fs_t" etc. 
We have two options:


1) Stick with this private and circular dependency:

     libsvn_fs_base <==> libsvn_fs <==> libsvn_fs_fs

In this case, more shared stuff could also be placed in libsvn_fs.


2) Refactor so that the dependencies come in clean layers:

                 libsvn_fs
                ^    ^    ^
               /     |     \
              /      |      \
      libsvn_fs_base |   libsvn_fs_fs
             ^       |       ^
              \      |      /
               \     |     /
               libsvn_fs_impl

In this case, the definition of "svn_fs_t" and any shared stuff like the error 
handling would go in "libsvn_fs_impl".  This is definitely the "proper" way to 
do it if there is likely to be any complexity.

This structure would give us the option of making the interfaces between them 
public if we so wished.


At the moment, while I'd be happy to go the clean, layered, potentially public 
route, I don't think the amount of sharing is large enough to require that, and 
so I'd be happy to put more shared stuff in libsvn_fs which is simpler in the 
short term, as long as we keep our eyes open for the point when it starts to 
get unreasonably complex and do the refactoring then.

That said, I'm not going to do anything unless those who work in that part of 
the code base want it.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Tue, 1 Nov 2005, Julian Foad wrote:

> Greg Hudson wrote:
> > On Sun, 2005-10-16 at 16:43 +0100, Philip Martin wrote:
> >
> > I'd say that this error message should not be using the filesystem
> > location in the first place.
>
> I think I agree.  Callers, at least up to the level of svn_repos_fs_lock(),
> know what filesystem they are accessing, so if they want to add this
> information to any error that is returned, they can.
>
+1.

> (While we're on the subject, shouldn't those two "err.c" files be unified,
> given that only about 2 of the 25 functions differ at all between them?)
>
The question is *where* this file should go. libsvn_subr doesn't seem
right. Do we want a libsvn_fs_util just for this? (I think this has been
discussed before.)

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Tue, 1 Nov 2005, Daniel Rall wrote:

> On Tue, 01 Nov 2005, Julian Foad wrote:
>
> > So, I vote for removing the path to the repository filesystem from all of
> > the error messages in subversion/libsvn_fs_{base,fs}/err.c
> >
> The route that has been taken in other places is not to remove
> information from the error's message, but to filter out info from or
> replace messages which contain possibly sensitive info at the
> appropriate layer (see mod_dav_svn for an example).  I prefer this
> approach on the principle that error messages should always be
> self-sufficient, and contain as much context as they have available to
> them when the error is encountered.  The fact that a caller may (or
> may not) have this context available is something I prefer not to have
> to consider.  Instead, we have today's situation, which while isn't
> always convenient, is not unreasonable, either.
>
What do you do when you have both useful information and sensitive
information, i.e. "File locked by user 'blah' in filesystem
'/blah/blah2'"?

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by Daniel Rall <dl...@collab.net>.
On Tue, 01 Nov 2005, Julian Foad wrote:

> Greg Hudson wrote:
> >On Sun, 2005-10-16 at 16:43 +0100, Philip Martin wrote:
> >
> >>>svn: Path '/trunk/sprints/Sprints.mdb' is already locked by user 'iar'
> >>>in filesystem 'C:/Daten/svnroot/myrepo/db'
> >
> >>When using http:// the repository location will get stripped from the
> >>error message as a security measure.  Perhaps svnserve should be doing
> >>the same?
> >
> >I'd say that this error message should not be using the filesystem
> >location in the first place.
> 
> I think I agree.  Callers, at least up to the level of svn_repos_fs_lock(), 
> know what filesystem they are accessing, so if they want to add this 
> information to any error that is returned, they can.
> 
> So, I vote for removing the path to the repository filesystem from all of 
> the error messages in subversion/libsvn_fs_{base,fs}/err.c
> 
> Opinions?

The route that has been taken in other places is not to remove
information from the error's message, but to filter out info from or
replace messages which contain possibly sensitive info at the
appropriate layer (see mod_dav_svn for an example).  I prefer this
approach on the principle that error messages should always be
self-sufficient, and contain as much context as they have available to
them when the error is encountered.  The fact that a caller may (or
may not) have this context available is something I prefer not to have
to consider.  Instead, we have today's situation, which while isn't
always convenient, is not unreasonable, either.

> (While we're on the subject, shouldn't those two "err.c" files be unified, 
> given that only about 2 of the 25 functions differ at all between them?)

Yes, I think there's some consolidation that can take place.  I too
noticed quite a bit of similarity between the two FS backends while
originally looking into this issue.
--

Daniel Rall

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by Julian Foad <ju...@btopenworld.com>.
Greg Hudson wrote:
> On Sun, 2005-10-16 at 16:43 +0100, Philip Martin wrote:
> 
>>>svn: Path '/trunk/sprints/Sprints.mdb' is already locked by user 'iar'
>>>in filesystem 'C:/Daten/svnroot/myrepo/db'
> 
>>When using http:// the repository location will get stripped from the
>>error message as a security measure.  Perhaps svnserve should be doing
>>the same?
> 
> I'd say that this error message should not be using the filesystem
> location in the first place.

I think I agree.  Callers, at least up to the level of svn_repos_fs_lock(), 
know what filesystem they are accessing, so if they want to add this 
information to any error that is returned, they can.

So, I vote for removing the path to the repository filesystem from all of the 
error messages in subversion/libsvn_fs_{base,fs}/err.c

Opinions?

(While we're on the subject, shouldn't those two "err.c" files be unified, 
given that only about 2 of the 25 functions differ at all between them?)

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2005-10-16 at 16:43 +0100, Philip Martin wrote:
> > svn: Path '/trunk/sprints/Sprints.mdb' is already locked by user 'iar'
> > in filesystem 'C:/Daten/svnroot/myrepo/db'

> When using http:// the repository location will get stripped from the
> error message as a security measure.  Perhaps svnserve should be doing
> the same?

I'd say that this error message should not be using the filesystem
location in the first place.

Also, http does not uniformly strip repository locations.  Run "svn log
-r 80000" in your svn working directory to find out that svn.collab.net
puts the Subversion repository at /usr/www/repositories/svn/db.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by Philip Martin <ph...@codematters.co.uk>.
Ingo Adler <de...@synacon.ch> writes:

> I try to lock a file which is already locked and get the message:
>
> svn: Path '/trunk/sprints/Sprints.mdb' is already locked by user 'iar'
> in filesystem 'C:/Daten/svnroot/myrepo/db'
>
> What shall I do with the information "in filesystem
> 'C:/Daten/svnroot/myrepo/db'"?
>
> It tells me where the repository is. But I cannot access the
> repository directly. I use Subversion for that. I wouldn't want to
> access the repository directly. Would it help, if I could access it?
> Probably not. I don't even want to know where it is physically - I use
> svn:// and http://.

When using http:// the repository location will get stripped from the
error message as a security measure.  Perhaps svnserve should be doing
the same?

> Another information would be much more helpful, details about the locker.
> - on computer 'ABC' with ip '123.456.678' in location
> 'c:\projects\src\sprint\sprint.mdb'.

The working copy location never gets to the server, and anyway both
the IP address and the wc path could change after the lock is granted.
The server doesn't care which working copy holds any given lock.

> Then I could start to look for it. For instance I could ask 'iar' and
> point him to the locked file.

Subversion makes the person taking the lock responsible for knowing
which working copy holds the lock.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: possibly sensitive - Was: "Locked" messages useless

Posted by Daniel Rall <dl...@collab.net>.
[redirecting mail back to the dev list]

On Fri, 21 Oct 2005, Carl Karsten wrote:

> >This has come up repeatedly for numerous commands and APIs.  The root cause
> >of this problem is that the potentially sensitive information is 
> >appropriate
> >for display by some callers, and thus must be propogated from the lowest
> >levels by the core libraries.
> >
> >A while back, I attempted design of a solution which allowed error 
> >information
> >to be marked as possibly sensitive.  We determined that application of such
> >an API was difficult at best, due to the fact that it is the caller's 
> >context
> >which actually determines what error information to display.  For example,
> >a system administrator working with svn or svnadmin on the repository host
> >itself might very well want to see full paths to the repository, whereas
> >users of the same repository over ra_dav or ra_svn should not have these 
> >paths
> >exposed to them.  The underlying core libraries which access the repository
> >must provide the local file system path when generating an error, but 
> >since all binaries eventually use the same libraries, that sensitive path
> >information must be suppressed by some callers.
> >
> >As of yet, we punt and leave it up to every caller to filter out possibly
> >sensitive information at the appropriate level.  In many places, we are 
> >still
> >not yet doing this filtering of sensitive info.
> 
> If the "possibly sensitive information" is being passed to the client, is 
> it really worth trying to filter it there?  That sounds like such an easy 
> hack: patch the client to display the info.

The information should not be being passed to the client.  That it
sometimes is being passed all the way to the client a bug.

> I can imagine that making it "more" secure may be "much harder", (quotes 
> point out un-quantifiable amounts that get "hard" to compare) - my point is 
> just: why spend any time trying to secure something if the result is still 
> "very" un-secure?

What is unsecure about filtering the possibly sensitive information
out at the appropriate layer (e.g. in libsvn_repos or mod_dav_svn or
svnserve)?

> > The full path to the repositorty is a
> >>>minor security bug that we should fix. 
> 
> Now for some left field ideas:
> 
> Can the client pass in what it knows (in the aboove case, the client passes 
> in what it knows the first half of the path to be) and the server can use 
> it if it is correct?  Example: client passes WCPATH='c:\projects\src\', 
> server sees 'C:/Daten/svnroot/myrepo/db', the two arn't  the same, so no 
> point passing back 'C:/Daten/svnroot/myrepo/db'.
...

The client doesn't pass working copy information to the "server".  It
uses the WC path to extract the repository URL from the WC.  When the
"server" is accessed over the network, it maps the URL used to access
it to the appropriate local path and RA backend; this mapping is an
example of information which should not be exposed to the client.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Sun, 16 Oct 2005, Ingo Adler wrote:

> Ben Collins-Sussman wrote:
> 
> >On 10/16/05, Ingo Adler <de...@synacon.ch> wrote:
...
> >>(2) No, it doesn't tell me which repository it's in. It tells me, where
> >>the repository files are placed on the server. But which server?
> >
> >I agree that this is a bug.  The full path to the repositorty is a
> >minor security bug that we should fix.  And I agree that it doesn't
> >help the user at all.
>
> This was the point. Thank you and good night.

This has come up repeatedly for numerous commands and APIs.  The root cause
of this problem is that the potentially sensitive information is appropriate
for display by some callers, and thus must be propogated from the lowest
levels by the core libraries.

A while back, I attempted design of a solution which allowed error information
to be marked as possibly sensitive.  We determined that application of such
an API was difficult at best, due to the fact that it is the caller's context
which actually determines what error information to display.  For example,
a system administrator working with svn or svnadmin on the repository host
itself might very well want to see full paths to the repository, whereas
users of the same repository over ra_dav or ra_svn should not have these paths
exposed to them.  The underlying core libraries which access the repository
must provide the local file system path when generating an error, but since 
all binaries eventually use the same libraries, that sensitive path
information must be suppressed by some callers.

As of yet, we punt and leave it up to every caller to filter out possibly
sensitive information at the appropriate level.  In many places, we are still
not yet doing this filtering of sensitive info.

- Dan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by Ingo Adler <de...@synacon.ch>.
Ben Collins-Sussman wrote:

>On 10/16/05, Ingo Adler <de...@synacon.ch> wrote:
>  
>
>>(1) It tells the path in the repository. But there is one instance of a
>>locked file in one working copy somewhere. I don't know where it is.
>>    
>>
>
>That's because it's none of your business.  Some user has created a
>lock in the repository;  that user might have 13 different working
>copies, and it's his private business as to which working copy
>actually contains the associated lock-token.  Why do you want to know?
> Why does it matter?
>
>You already know which user locked the file, so either call him and
>ask him to release the lock, or forcibly break (or steal) the lock
>yourself.
>  
>
OK for me.

>>(2) No, it doesn't tell me which repository it's in. It tells me, where
>>the repository files are placed on the server. But which server?
>>
>>    
>>
>
>I agree that this is a bug.  The full path to the repositorty is a
>minor security bug that we should fix.  And I agree that it doesn't
>help the user at all.
>
This was the point. Thank you and good night.

Ingo.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by Ben Collins-Sussman <su...@red-bean.com>.
On 10/16/05, Ingo Adler <de...@synacon.ch> wrote:

> (1) It tells the path in the repository. But there is one instance of a
> locked file in one working copy somewhere. I don't know where it is.

That's because it's none of your business.  Some user has created a
lock in the repository;  that user might have 13 different working
copies, and it's his private business as to which working copy
actually contains the associated lock-token.  Why do you want to know?
 Why does it matter?

You already know which user locked the file, so either call him and
ask him to release the lock, or forcibly break (or steal) the lock
yourself.

> (2) No, it doesn't tell me which repository it's in. It tells me, where
> the repository files are placed on the server. But which server?
>

I agree that this is a bug.  The full path to the repositorty is a
minor security bug that we should fix.  And I agree that it doesn't
help the user at all.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: "Locked" messages useless

Posted by Ingo Adler <de...@synacon.ch>.
Ben Collins-Sussman wrote:

>On 10/16/05, Ingo Adler <de...@synacon.ch> wrote:
>  
>
>>Then I could start to look for it. For instance I could ask 'iar' and
>>point him to the locked file.
>>    
>>
>
>What information do you need?  The error message has already told you
>(1) the locked path, (2) which repository it's in, and (3) the user
>who locked it.  If you need more still more information about the
>lock, you could run 'svn info URL-of-file', and you then know (4) the
>date the lock was created and (5) the lock-comment written by 'iar',
>if he wrote one.
>
>That's 3 pieces of lock information for free, and 2 more if you run
>'svn info URL'.  None of these things are useless 'server internal'
>information.  All of them are designed to let you know about the
>repository lock that iar made, so you can chat with him if need be.
>  
>
(1) It tells the path in the repository. But there is one instance of a 
locked file in one working copy somewhere. I don't know where it is.
(2) No, it doesn't tell me which repository it's in. It tells me, where 
the repository files are placed on the server. But which server?

I don't really need much more information. It would be nice to have 
more, for instance which working copy on which computer. But I really 
don't need useless information - the directory where the repository is 
placed on the server ('C:/Daten/svnroot/myrepo/db'). That's really some 
internal stuff. You could add much of this "free" information to every 
subversion message. But you shouldn't. Just tell the user what he needs 
to solve his problem. The user's name is good. The path of the file is 
OK. The repository path on the server (local to the server) is not OK.

There could be security reasons too to omit this information. I just 
took the end-user's perspective.

Ingo.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: "Locked" messages useless

Posted by Ben Collins-Sussman <su...@red-bean.com>.
On 10/16/05, Ingo Adler <de...@synacon.ch> wrote:

> Then I could start to look for it. For instance I could ask 'iar' and
> point him to the locked file.

What information do you need?  The error message has already told you
(1) the locked path, (2) which repository it's in, and (3) the user
who locked it.  If you need more still more information about the
lock, you could run 'svn info URL-of-file', and you then know (4) the
date the lock was created and (5) the lock-comment written by 'iar',
if he wrote one.

That's 3 pieces of lock information for free, and 2 more if you run
'svn info URL'.  None of these things are useless 'server internal'
information.  All of them are designed to let you know about the
repository lock that iar made, so you can chat with him if need be.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org