Karl Fogel <kfogel_at_red-bean.com> writes:
> "David Glasser" <glasser_at_davidglasser.net> writes:
>>> + for (i = 0; i < mergeinfo->nelts; i++)
>>
>> Uh, isn't mergeinfo an apr_hash_t **? This is not how you iterate
>> over a hash...
>
> WTF? It *compiled*, sheesh. (Haven't tested at all yet; I just
> wanted your review on the general approach, though am very glad to
> have you spot things like this too :-) ).
I'm embarrassed.
It turns out I compiled the wrong tree.
Here's a new patch:
[[[
On the reintegrate branch:
* subversion/libsvn_repos/fs-wrap.c: Include svn_hash.h.
(svn_repos_fs_get_mergeinfo): Do authz checking on the paths
returned in mergeinfo that haven't already been checked.
* reintegrate-branch-TODO: Remove the reminder about this.
]]]
Index: subversion/libsvn_repos/fs-wrap.c
===================================================================
--- subversion/libsvn_repos/fs-wrap.c (revision 28840)
+++ subversion/libsvn_repos/fs-wrap.c (working copy)
@@ -23,6 +23,7 @@
#include "svn_error.h"
#include "svn_fs.h"
#include "svn_path.h"
+#include "svn_hash.h"
#include "svn_props.h"
#include "svn_repos.h"
#include "repos.h"
@@ -595,10 +596,73 @@
in *MERGEINFO, avoiding massive authz overhead which would allow
us to protect the name of where a change was merged from, but not
the change itself. */
- /* ### TODO(reint): ... but how about descendant merged-to paths? */
- if (readable_paths->nelts > 0)
- SVN_ERR(svn_fs_get_mergeinfo(mergeinfo, root, readable_paths, inherit,
- include_descendants, pool));
+ if (readable_paths->nelts > 0)
+ {
+ SVN_ERR(svn_fs_get_mergeinfo(mergeinfo, root, readable_paths, inherit,
+ include_descendants, pool));
+
+ if (include_descendants)
+ {
+ /* The mergeinfo might have paths that never got authz-checked;
+ if so, check them now. But only check the ones where we
+ can't know by examining 'paths' and 'readable_paths',
+ since an authz check is (relatively) expensive. */
+
+ apr_hash_index_t *hi;
+ apr_hash_t *original_paths_hash, *readable_paths_hash;
+
+ /* Make lookups easy by converting to hash. */
+ SVN_ERR(svn_hash_from_cstring_keys(&original_paths_hash,
+ paths, pool));
+ if (readable_paths == paths)
+ {
+ readable_paths_hash = original_paths_hash;
+ }
+ else
+ {
+ SVN_ERR(svn_hash_from_cstring_keys(&readable_paths_hash,
+ readable_paths, pool));
+ }
+
+ for (hi = apr_hash_first(pool, *mergeinfo); hi;
+ hi = apr_hash_next(hi))
+ {
+ const void *key;
+ const char *path;
+
+ svn_pool_clear(iterpool);
+ apr_hash_this(hi, &key, NULL, NULL);
+ path = key;
+ if (! apr_hash_get(readable_paths_hash, path,
+ APR_HASH_KEY_STRING))
+ {
+ /* It's not in readable_paths, so if it *is* in
+ the original paths, that means it was already
+ authz-checked and found unreadable -- in which
+ case remove it from the mergeinfo. */
+ if (readable_paths_hash != original_paths_hash
+ && apr_hash_get(original_paths_hash, path,
+ APR_HASH_KEY_STRING))
+ {
+ apr_hash_set(*mergeinfo, path, APR_HASH_KEY_STRING,
+ NULL);
+ }
+ else
+ {
+ /* We have to actually do the authz check,
+ because we have no other information about
+ the path's readability. */
+ svn_boolean_t readable;
+ SVN_ERR(authz_read_func(&readable, root, path,
+ authz_read_baton, iterpool));
+ if (! readable)
+ apr_hash_set(*mergeinfo, path, APR_HASH_KEY_STRING,
+ NULL);
+ }
+ }
+ }
+ }
+ }
else
*mergeinfo = NULL;
Index: reintegrate-branch-TODO
===================================================================
--- reintegrate-branch-TODO (revision 28840)
+++ reintegrate-branch-TODO (working copy)
@@ -20,8 +20,6 @@
* "Is this the same line?" direct ancestor check (glasser)
- * authz check (glasser)
-
* remember there may be some TODO(reint) comments in .py tests; they
won't show up in a tags-search.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-01-10 06:07:44 CET