Re: Authz on Collection of Repositories

2012-11-14 Thread Ivan Zhakov
On Mon, Nov 12, 2012 at 4:23 PM, Ivan Zhakov i...@visualsvn.com wrote:
 On Mon, Nov 12, 2012 at 2:28 AM, Thomas Åkesson
 thomas.akes...@simonsoft.se wrote:

 On 9 nov 2012, at 18:45, Ivan Zhakov wrote:

 On Thu, Nov 8, 2012 at 6:49 PM, Thomas Åkesson
 thomas.akes...@simonsoft.se wrote:

 Parentpath on /svn/ and Satisfy Any:

 - Access without auth displays repositories with anonymous access, auth is 
 not requested.
 - Access with auth displays filtered list. Works well when browser has 
 previously
 been on an authenticated path. This is the situation when Satisfy Any and 
 filtered
 Collection of Repositories does not work well.
 That's why mixing anonymous and authenticated access is not good thing.

 Yes, I am just trying to cover all bases including the possibility that 
 people are depending on the inconsistency that we are addressing.


 - Did a test with AuthzSVNAnonymous Off, which gave the quite surprising 
 result
 that all content was listed both on Collection of Repositories and within 
 the
 repositories. I doubt this is the intended behaviour?!?
 I agree, this is really strange behavior. Could you check this
 behavior with my patch? It's very low chance that my patch changes
 this behavior.

 I have tested both with and without your patch. As expected, the patch has 
 no impact on the AuthzSVNAnonymous issue.

 There seems to be an issue when AuthzSVNAnonymous Off is combined with 
 Satisfy Any; opens up the fort completely. Neither authn nor authz is 
 required.

 I think the problem is with access_checker, perhaps this part (has changed a 
 few times during the years):
   if (!conf-anonymous
   || (! (conf-access_file || conf-repo_relative_access_file)))
 return DECLINED;

 I am not quite sure how a DECLINE manages to bypass Require valid-user 
 though. I understand how an OK would though.


 - What is going on with AuthzSVNAnonymous Off? I will do more analysis of 
 the
 code (focusing on access_checker in mod_authz_svn.c) but it would be great 
 if
 someone could elaborate a bit on the intent.

 It would be nice if you confirm that my patch does not change
 AuthzSVNAnonymous Off behavior in this case I'll commit my patch and
 we may focus on this issue.

 Confirmed as far as my testing goes (did not test short_circuit). I suggest
 committing the patch with GET subrequest and potentially change all to
 HEAD in a separate commit if there is consensus.

 Committed in r1408184.

I doubt about backporting this fix to 1.7.x.

Pro:
* This is regression from 1.6.x:  It was possible to restrict access
to Collection of Repositories by controlling access to [/], while
access to individual repositories were controlled by [repoN:/]. This
might not have been by design, bit still a very useful feature.

* We already ported similar fix to hide unreadable dirs to 1.6.x (r996884)

Cons:
* Security behavior changes in patches is not good thing from my point view


Any opinions?

-- 
Ivan Zhakov


Re: fork/exec for hooks scripts with a large FSFS cache

2012-11-14 Thread Philip Martin
Stefan Fuhrmann stefan.fuhrm...@wandisco.com writes:

 philip.mar...@wandisco.comwrote:

 Perhaps we could start up a separate hook script process before
 allocating the large FSFS cache and then delegate the fork/exec to that
 smaller process?


 I wonder whether there is a way to pass a different
 cache setting to the sub-process.

I don't think this would work.  It's the fork that is failing, the child
process never exists so it cannot use a smaller memory footprint.

Having hooks run in a separate process is complicated.  The process
would need to be multi-threaded, or multi-process, to avoid hooks
running in serial.  stdin/out/err would need to be handled
somehow. Pipes perhaps?  By passing file descriptors across a Unix
domain socket?

For now I think we just have to recommend that the system has sufficient
swap for the fork to work.  Once the child execs the hook the memory
footprint of the process goes down.  So as far as I can tell on my Linux
system nothing gets written to swap, it just has to exist when fork is
called.

-- 
Certified  Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download


RE: svn commit: r1409186 - /subversion/trunk/subversion/libsvn_client/add.c

2012-11-14 Thread Bert Huijben


 -Original Message-
 From: phi...@apache.org [mailto:phi...@apache.org]
 Sent: woensdag 14 november 2012 13:47
 To: comm...@subversion.apache.org
 Subject: svn commit: r1409186 -
 /subversion/trunk/subversion/libsvn_client/add.c
 
 Author: philip
 Date: Wed Nov 14 12:46:40 2012
 New Revision: 1409186
 
 URL: http://svn.apache.org/viewvc?rev=1409186view=rev
 Log:
 * subversion/libsvn_client/add.c
   (svn_client__get_all_auto_props): Don't use iterpool when values must
persist.
 
 Modified:
 subversion/trunk/subversion/libsvn_client/add.c
 
 Modified: subversion/trunk/subversion/libsvn_client/add.c
 URL:
 http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/a
 dd.c?rev=1409186r1=1409185r2=1409186view=diff
 ==
 
 --- subversion/trunk/subversion/libsvn_client/add.c (original)
 +++ subversion/trunk/subversion/libsvn_client/add.c Wed Nov 14 12:46:40
 2012
 @@ -737,7 +737,7 @@ svn_client__get_all_auto_props(apr_hash_
err = svn_client_propget5(props, inherited_config_auto_props,
  SVN_PROP_INHERITABLE_AUTO_PROPS, path_or_url,
  rev, rev, NULL, svn_depth_empty, NULL, ctx,
 -scratch_pool, iterpool);
 +scratch_pool, scratch_pool);

Hmm, since when do our functions allocate results in their scratch_pool, when 
they also have a result pool?

Bert 




Re: svn commit: r1409186 - /subversion/trunk/subversion/libsvn_client/add.c

2012-11-14 Thread Philip Martin
Bert Huijben b...@qqmail.nl writes:

err = svn_client_propget5(props, inherited_config_auto_props,
  SVN_PROP_INHERITABLE_AUTO_PROPS, 
 path_or_url,
  rev, rev, NULL, svn_depth_empty, NULL, 
 ctx,
 -scratch_pool, iterpool);
 +scratch_pool, scratch_pool);

 Hmm, since when do our functions allocate results in their
 scratch_pool, when they also have a result pool?

Hmm, it looks like svn_client_propget5 doesn't call svn_wc__get_iprops
correctly.

-- 
Certified  Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download


Re: fork/exec for hooks scripts with a large FSFS cache

2012-11-14 Thread Greg Stein
On Wed, Nov 14, 2012 at 8:49 AM, Philip Martin
philip.mar...@wandisco.com wrote:
...
 Having hooks run in a separate process is complicated.  The process
 would need to be multi-threaded, or multi-process, to avoid hooks
 running in serial.  stdin/out/err would need to be handled
 somehow. Pipes perhaps?  By passing file descriptors across a Unix
 domain socket?

We could do whatever mod_cgid is doing.

But with that said: most hooks don't generate stdout or stderr. We
could ship over parameters and a stdin blob, and run the hook. This
simplified model would only work if it was acceptable to *not* return
stdout/err to the client. (anything could still be logged on the
server)

You don't really need multiprocess or multithread, if you run an async
network loop such as serf does. The child exit signal would pop the
network loop, allowing for examination of the result. The daemon would
get a hook request, fork/exec, and return the exit code. (heck, if the
stdout/err is small enough, it could be captured and returned in the
response)

IIRC, Apache httpd even has a subsystem to monitor these kinds of
daemons and keep them running.

Cheers,
-g


Re: Authz on Collection of Repositories

2012-11-14 Thread Thomas Åkesson
On 14 nov 2012, at 11:53, Ivan Zhakov i...@visualsvn.com wrote:

 
 Confirmed as far as my testing goes (did not test short_circuit). I suggest
 committing the patch with GET subrequest and potentially change all to
 HEAD in a separate commit if there is consensus.
 Committed in r1408184.
 I doubt about backporting this fix to 1.7.x.
 
 Pro:
 * This is regression from 1.6.x:  It was possible to restrict access
 to Collection of Repositories by controlling access to [/], while
 access to individual repositories were controlled by [repoN:/]. This
 might not have been by design, bit still a very useful feature.
 
 * We already ported similar fix to hide unreadable dirs to 1.6.x (r996884)
 
 Cons:
 * Security behavior changes in patches is not good thing from my point view
 
 
 Any opinions?

I think it makes sense to release in 1.8 (no backport). Provides a better 
opportunity to explain the change. Admins on 1.6 who can not have open access 
to Collection of Repositories will have to skip 1.7. 

I can try to draft something for the change notes, next week.

/Thomas Å. 

Re: [serf-dev] Re: Double compression over HTTPS

2012-11-14 Thread Greg Stein
On Nov 14, 2012 1:44 AM, Lieven Govaerts l...@apache.org wrote:
...
 Implemented in serf r1692, with an API for applications to enable SSL
 compression. Which to be clear, I don't propose svn to use.

This API would go into a 1.2 and 2.0, but if nobody is asking for it, then
I'd suggest we don't spin those up yet.

Cheers,
-g


Re: [serf-dev] Re: Double compression over HTTPS

2012-11-14 Thread Lieven Govaerts
On Thu, Nov 15, 2012 at 1:01 AM, Greg Stein gst...@gmail.com wrote:

 On Nov 14, 2012 1:44 AM, Lieven Govaerts l...@apache.org wrote:
...


 Implemented in serf r1692, with an API for applications to enable SSL
 compression. Which to be clear, I don't propose svn to use.

 This API would go into a 1.2 and 2.0, but if nobody is asking for it, then
 I'd suggest we don't spin those up yet.


Agreed on not making a release only for this change, but I do plan to
prepare a 1.2 release pretty soon, when the issue fixing work in
progress has been finalized.

Lieven


Re: [PATCH] Implement '--include-externals' option to 'svn list'

2012-11-14 Thread vijay

On Tuesday 13 November 2012 06:45 PM, Bert Huijben wrote:




-Original Message-
From: vijay [mailto:vi...@collab.net]
Sent: dinsdag 13 november 2012 14:06
To: Subversion Development
Subject: Re: [PATCH] Implement '--include-externals' option to 'svn list'

On Tuesday 13 November 2012 03:02 AM, Stefan Sperling wrote:

Attached the updated patch and log message.



+  /* Notify that we're about to handle an external. */
+  SVN_ERR(list_func(baton, NULL, NULL, NULL, NULL,
+externals_parent_url,
+item-target_dir, iterpool));


The docstring of svn_client_list_func2_t doesn't say if it is valid
for path or dirent to be NULL.

However, you're passing NULL for these parameters before listing the
external. Do we really need these two extra list_func() calls before
and after listing the external? I was expecting them to go away.



If we are removing these list_func() calls, how can we pass the
arguments external_parent_url and external_target to the callback
function? Is there any way to pass those arguments via svn_client_list3()?


Can't you just add the new argument to every call to list_func() that
applies to an external?



There is no separate list_func() call to list external items. We are 
just calling svn_client_list3() recursively for each external, which in 
turn calls list_func() using get_dir_contents(). I am struggling a bit 
hard to carry forward external information via svn_client_list3(). Can 
we add two more arguments external_parent_url and external_target to 
svn_client_list3()?


If you want to look at the patch, here[1] is the latest version. Any 
suggestions are welcome.



[1] http://svn.haxx.se/dev/archive-2012-11/0313.shtml

Thanks  Regards,
Vijayaguru




That would allow removing the initial call, while the callee always has the
information it needs.

Bert