Neels Janosch Hofmeyr | 1 Aug 2008 02:17
Picon

Re: [Issue 1796] defective or malicious client can corrupt repository log messages

A number of weeks ago, there was a discussion on validation of the
commit log messages on their journey from client to server and back.

It was said, that:

Neels Janosch Hofmeyr wrote:
> So, right now, there is only *one* place where props get
> normalised/checked for consistence:
> - where the svn client receives a log message from the user
> 
> The places, where checking the props is, supposedly, missing, are:
> - where the server receives props from a client out there.
> - where the server reads props from the repository file system.
> - where the svn client reads props from a server out there.

The first of the latter three has been fixed (issue 1796).
The last two are still lurking.

Since, I've had a discussion on the implications of fixing these latter
two, with stsp.

Imagine that someone has a repository containing log messages with CR or
non-UTF8 sequences. Then, *we* come along and make the server validate
log messages read from the file system, plus make the client validate
log messages received from the server. In effect, the user isn't able to
simply *look* at the log message anymore.

It struck us as a rather dumb situation, and I am since of the opinion
that the part of a log message's journey going in the direction towards
the user should not have prohibitive log message validation.
(Continue reading)

Hyrum K. Wright | 1 Aug 2008 04:12
Picon

Re: How to get notified after an operation happend on repository

Questions such as these should be directed to users <at> subversion.tigris.org

Thanks,
-Hyrum

Lenina konjeti wrote:
> Hi,
> 
>  
> 
> We are using SVN as version control in our project.
> 
>  
> 
> We have  a requirement, trigger the build whenever there is a modification
> on SVN repository.
> 
>  
> 
> Initially I am thinking of SVNKit ISVNEventHandler.handleEvent() is the
> right place to write code for my requirement, later I came to know that is,
> SVNKit is a client package.
> 
>  
> 
> Please let me know how do I approach to get notified immediately whenever
> there are any changes on SVN Repo.
> 
>  
> 
(Continue reading)

Hyrum K. Wright | 1 Aug 2008 07:39
Picon

Improved mime-type guessing (issue #1233)

While performing a random walk through the issue tracker, I came across issue 
#1233, which includes a patch to use the /etc/mime.types file on *nix systems to 
improve our mime-type guessing.  It looked interesting, so I started digging in, 
only to realize that there were more issues at play than at first blush.

Before go too deeply into the code, I have a few questions I was hoping people 
could answer:

  * Does this even matter?  Would adding this functionality be a benefit to our
    users, and if so, does the overhead of adding and maintaining it outweigh the
    derived benefit?

  * Is there a better place to do this?  APR?  Would it be reasonable to move the
    mime-type handling code out of httpd into APR where Subversion could make use
    of it?

  * Is there equivalent MIME type guessing functionality available on Windows,
    either through an API or via a system file?

Hopefully the answers to these questions will help us resurrect this patch, or 
decide on a reasonable fate for issue #1233.

-Hyrum

Julian Foad | 1 Aug 2008 11:20

Re: [Issue 1796] defective or malicious client can corrupt repository log messages

On Fri, 2008-08-01 at 02:17 +0200, Neels Janosch Hofmeyr wrote:
> A number of weeks ago, there was a discussion on validation of the
> commit log messages on their journey from client to server and back.
> 
> It was said, that:
> 
> Neels Janosch Hofmeyr wrote:
> > So, right now, there is only *one* place where props get
> > normalised/checked for consistence:
> > - where the svn client receives a log message from the user
> > 
> > The places, where checking the props is, supposedly, missing, are:
> > - where the server receives props from a client out there.
> > - where the server reads props from the repository file system.
> > - where the svn client reads props from a server out there.
> 
> The first of the latter three has been fixed (issue 1796).
> The last two are still lurking.
> 
> Since, I've had a discussion on the implications of fixing these latter
> two, with stsp.
> 
> Imagine that someone has a repository containing log messages with CR or
> non-UTF8 sequences. Then, *we* come along and make the server validate
> log messages read from the file system, plus make the client validate
> log messages received from the server. In effect, the user isn't able to
> simply *look* at the log message anymore.
> 
> It struck us as a rather dumb situation, and I am since of the opinion
> that the part of a log message's journey going in the direction towards
(Continue reading)

Julian Foad | 1 Aug 2008 11:25

Re: Improved mime-type guessing (issue #1233)

On Thu, 2008-07-31 at 22:39 -0700, Hyrum K. Wright wrote:
> While performing a random walk through the issue tracker, I came across issue 
> #1233, which includes a patch to use the /etc/mime.types file on *nix systems to 
> improve our mime-type guessing.  It looked interesting, so I started digging in, 
> only to realize that there were more issues at play than at first blush.
> 
> Before go too deeply into the code, I have a few questions I was hoping people 
> could answer:
> 
>   * Does this even matter?  Would adding this functionality be a benefit to our
>     users, and if so, does the overhead of adding and maintaining it outweigh the
>     derived benefit?

Theoretically, yes, it matters. It's ugly for a supposedly data-neutral
storage system to contain a set of hard-coded rules about what types of
data it treats in what way.

Practically, I cannot quantify the cost and benefit. It's certain that
there would be a benefit to some people, because we have several times
had requests to add this or that MIME type to the list. Let's see how
much interface code would be required to do this.

>   * Is there a better place to do this?  APR?  Would it be reasonable to move the
>     mime-type handling code out of httpd into APR where Subversion could make use
>     of it?

What sort of MIME-type handling APIs?

>   * Is there equivalent MIME type guessing functionality available on Windows,
>     either through an API or via a system file?
(Continue reading)

Kamesh Jayachandran | 1 Aug 2008 17:28
Favicon

[PATCH] svn_dso_initialize can *silently* fail to create lock

Hi All,

The svn_dso_initialize can *silently* fail to create lock causing
failures later.

Attached patch fixes it.

Want to know what others would think about it.

With regards
Kamesh Jayachandran
Fix 'svn_dso_initialize' which was *silently* failing to create lock
and causing later accessors to fail.

* subversion/libsvn_subr/dso.c
  (svn_dso_initialize): When 'apr_thread_mutex_create' is not successful,
   abort.
Index: subversion/libsvn_subr/dso.c
===================================================================
--- subversion/libsvn_subr/dso.c	(revision 32351)
+++ subversion/libsvn_subr/dso.c	(working copy)
 <at>  <at>  -44,13 +44,17  <at>  <at> 
 void
 svn_dso_initialize()
 {
+  apr_status_t status;
(Continue reading)

Stefan Sperling | 1 Aug 2008 19:20
Picon

Re: [PATCH] svn_dso_initialize can *silently* fail to create lock

On Fri, Aug 01, 2008 at 08:58:57PM +0530, Kamesh Jayachandran wrote:
> Hi All,
> 
> The svn_dso_initialize can *silently* fail to create lock causing
> failures later.
> 
> Attached patch fixes it.
> 
> Want to know what others would think about it.

Looks good to me.

A curiosity: The return value of apr_thread_mutex_create is
not checked consistently even within APR itself.

Have you seen crashes because of this
In apr-1.3.2, the function can only return an error it if fails
to allocate memory (at least the way we're calling it with
APR_THREAD_MUTEX_DEFAULT). Anyway, checking the error code is a good
idea. We don't want to rely on implementation details like that.

I would suggest using

  SVN_ERR_ASSERT(status == APR_SUCCESS);

instead of

  if (status)
    abort();

(Continue reading)

Erik Huelsmann | 1 Aug 2008 19:32
Picon
Gravatar

Re: svn log hangs munching cpu

On 7/31/08, dave_rodgman <at> fastmail.co.uk <dave_rodgman <at> fastmail.co.uk> wrote:
> Hi,
>
> following is top output:
>
>  PID USER      PR  NI  VIRT  RES  SHR S %CPU %MEM    TIME+  COMMAND
> 31151 plgtest   25   0  7316 2564 2024 R   64  0.1   1215:37 svn
>  6416 plgtest   25   0  7316 2564 2024 R   55  0.1   1489:08 svn
> 24938 plgtest   25   0  7316 2564 2024 R   50  0.1   1538:18 svn
> 32493 plgtest   25   0  7312 2556 2024 R   49  0.1 746:08.08 svn
>  800 plgtest   25   0  7316 2564 2024 R   47  0.1 747:46.63 svn
>  5718 plgtest   25   0  7312 2560 2024 R   46  0.1   1318:43 svn
>  2382 plgtest   25   0  7312 2560 2024 R   45  0.1   1268:37 svn
>
> the command lines for the hung svn processes are of the form:
>
> svn log --incremental http://... -r$REV
>
> and, usually (even on the same box), running the exact same command

exactly the same command, or using a different protocol? (ie svn log
--incremental file://...)?

> is fast (0.08 seconds), so this seems not reproducable. It may have
> been triggered by network problems, but there's no way to know now.
>
> Is this a known problem? Should I raise an issue?

Yes, it's a known issue. Please don't raise one.

(Continue reading)

Kamesh Jayachandran | 1 Aug 2008 20:57
Favicon

RE: [PATCH] svn_dso_initialize can *silently* fail to create lock

Thanks Stefan for the review, committed at r32353.


>Looks good to me.

>A curiosity: The return value of apr_thread_mutex_create is
>not checked consistently even within APR itself.

>Have you seen crashes because of this
>In apr-1.3.2, the function can only return an error it if fails
>to allocate memory (at least the way we're calling it with
>APR_THREAD_MUTEX_DEFAULT). Anyway, checking the error code is a good
>idea. We don't want to rely on implementation details like that.

I did see the crash with apr-0.9.x(I think 0.9.12)


In my local box(apr-1.2.12) I could see 'apr_thread_mutex_create' handling error properly.
<snip>
        rv = pthread_mutex_init(&new_mutex->mutex, NULL);

    if (rv) {
#ifdef PTHREAD_SETS_ERRNO
        rv = errno;
#endif
        return rv;
    }
</snip>

>I would suggest using

>  SVN_ERR_ASSERT(status == APR_SUCCESS);

It will give a compiler warning as the caller here is a void function and SVN_ERR_ASSERT would evaluate to 'return svn_error_t *'.


With regards
Kamesh Jayachandran

Blair Zajac | 1 Aug 2008 20:59
Gravatar

Re: svn commit: r32353 - trunk/subversion/libsvn_subr

kameshj <at> tigris.org wrote:
> Author: kameshj
> Date: Fri Aug  1 11:48:16 2008
> New Revision: 32353
> 
> Log:
> Fix 'svn_dso_initialize' which was *silently* failing to create lock
> and causing later accessors to fail.

>  
>  #if APR_HAS_THREADS
> -  apr_thread_mutex_create(&dso_mutex, APR_THREAD_MUTEX_DEFAULT, dso_pool);
> +  status = apr_thread_mutex_create(&dso_mutex, 
> +                                   APR_THREAD_MUTEX_DEFAULT, dso_pool);
> +  if (status)
> +    abort();

We're trying to get away from abort()'s in our library code.

How about creating svn_dso_initialize2() that returns an apr_status_t and the 
caller can decide how to handle it.  Then svn_dso_initialize() can call 
svn_dso_initialize2() and then abort if it fails.  But this gives our users a 
way to upgrade and void abort()'s.

Blair

Gmane