(Sorry for not being to review your patch before you committed, Erik. I saw
your IRC message, but was wrapped up in other things today.)
On 06/06/2011 05:27 PM, ehu_at_apache.org wrote:
> Author: ehu
> Date: Mon Jun 6 21:27:21 2011
> New Revision: 1132782
>
> URL: http://svn.apache.org/viewvc?rev=1132782&view=rev
> Log:
> Check node kind of resolved special nodes to be a directory, in order
> to include symlinks pointing to directories.
This summary doesn't do a particularly good job of setting the context of
the change. Could you wordsmith it a bit to at least let us know that we're
talking about mod_dav_svn's service of directory listings via GET or
something? Thanks!
> * subversion/mod_dav_svn/repos.c
> (deliver): Extend 'is directory' check for inclusion
> in parent path listing to include symlinks-to-directory.
>
> Modified:
> subversion/trunk/subversion/mod_dav_svn/repos.c
>
> Modified: subversion/trunk/subversion/mod_dav_svn/repos.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/repos.c?rev=1132782&r1=1132781&r2=1132782&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/mod_dav_svn/repos.c (original)
> +++ subversion/trunk/subversion/mod_dav_svn/repos.c Mon Jun 6 21:27:21 2011
> @@ -3247,7 +3247,23 @@ deliver(const dav_resource *resource, ap
> apr_hash_this(hi, &key, NULL, &val);
> dirent = val;
>
> - if (dirent->kind != svn_node_dir)
> + if (dirent->kind == svn_node_file && dirent->special)
> + {
> + svn_node_kind_t resolved_kind;
> + const char *name = key;
> +
> + serr = svn_io_check_resolved_path(name, &resolved_kind,
> + resource->pool);
> + if (serr != NULL)
> + return dav_svn__convert_err(serr,
> + HTTP_INTERNAL_SERVER_ERROR,
> + "couldn't fetch dirents "
> + "of SVNParentPath",
> + resource->pool);
This error message doesn't really reflect the failed condition, does it?
But while we're here, is an error the right thing? Should a single busted
symlink in the parent path cause the parent path listing to fail altogether?
I'm wondering if simply skipping over the unresolved link (like we would
skip over non-directories) would be a kinder approach.
Did you have a strong opinion on this, Erik?
> + if (resolved_kind != svn_node_dir)
> + continue;
> + }
> + else if (dirent->kind != svn_node_dir)
> continue;
>
> ent->name = key;
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on 2011-06-07 02:41:28 CEST