From: Dipesh Patel <dipthegeezer.opensource@gmail.com>
Date: Wed, 1 Sep 2010 17:58:21 +0000 (+0100)
Subject: Do proper checking of git directories.
X-Git-Tag: 0.002007~9
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FGitalist.git;a=commitdiff_plain;h=afbb1a52b828221eba9a1c0c74dfa850992e3399

Do proper checking of git directories.

Tidy up and use callback to resolve the repo path for a recursive directory.
---

diff --git a/lib/Gitalist/Git/CollectionOfRepositories/FromDirectoryRecursive.pm b/lib/Gitalist/Git/CollectionOfRepositories/FromDirectoryRecursive.pm
index e654543..38c18b8 100644
--- a/lib/Gitalist/Git/CollectionOfRepositories/FromDirectoryRecursive.pm
+++ b/lib/Gitalist/Git/CollectionOfRepositories/FromDirectoryRecursive.pm
@@ -19,8 +19,17 @@ class Gitalist::Git::CollectionOfRepositories::FromDirectoryRecursive
     }
 
     method _get_path_for_repository_name (NonEmptySimpleStr $name) {
-      my $path = Path::Class::Dir->new( $name )->resolve;
-      die "Directory traversal prohibited: $path"
+      my $path;
+      $self->repo_dir->recurse( 
+        callback => sub {
+          my ( $thing ) = @_;
+          return unless ( $thing->is_dir );
+          $path = $thing if ( $thing->dir_list(-1) eq $name 
+                                  && $self->_is_git_repo( $thing ) );
+        }
+      );
+      $path->resolve if $path;
+      die "Directory traversal prohibited: ".( $path || 'path undefined' )
           unless $self->repo_dir->contains($path);
       return $path;
     }
@@ -35,11 +44,17 @@ class Gitalist::Git::CollectionOfRepositories::FromDirectoryRecursive
             for my $repo ( @ret ) {
               # no need to go further if parent is git dir
               # never have a git repo in a git repo?
-              return if ( $repo->path->contains( $dir ) );
+              my $check_dir = $repo->path;
+              # go up one and ignore all in that path
+              # if in hidden .git directory
+              $check_dir = $check_dir->parent 
+                  if ( -f $check_dir->parent->file('.git', 'HEAD') );
+              return if ( $check_dir->contains( $dir ) );
             }
             eval {
-              # pass directory as string
-              my $p = $self->get_repository("$dir");
+              # pass directory name as string
+              my @list = $dir->dir_list();
+              my $p = $self->get_repository($list[$#list]);
               push @ret, $p;
             };
           }
diff --git a/lib/Gitalist/Model/CollectionOfRepos.pm b/lib/Gitalist/Model/CollectionOfRepos.pm
index 2820b53..45c9e35 100644
--- a/lib/Gitalist/Model/CollectionOfRepos.pm
+++ b/lib/Gitalist/Model/CollectionOfRepos.pm
@@ -1,7 +1,7 @@
 package Gitalist::Model::CollectionOfRepos;
 
 use Moose;
-use Gitalist::Git::CollectionOfRepositories::FromDirectory;
+use Gitalist::Git::CollectionOfRepositories::FromDirectoryRecursive;
 use Gitalist::Git::CollectionOfRepositories::FromListOfDirectories;
 use MooseX::Types::Moose qw/Maybe ArrayRef/;
 use MooseX::Types::Common::String qw/NonEmptySimpleStr/;
@@ -71,7 +71,7 @@ sub build_per_context_instance {
         Gitalist::Git::CollectionOfRepositories::FromListOfDirectories->new(repos => $self->repos);
     }
     else {
-        Gitalist::Git::CollectionOfRepositories::FromDirectory->new(repo_dir => $self->repo_dir);
+        Gitalist::Git::CollectionOfRepositories::FromDirectoryRecursive->new(repo_dir => $self->repo_dir);
     }
 }
 
diff --git a/t/02git_CollectionOfRepositories_FromDirectoryRecursive.t b/t/02git_CollectionOfRepositories_FromDirectoryRecursive.t
index e407aea..9e33d2e 100644
--- a/t/02git_CollectionOfRepositories_FromDirectoryRecursive.t
+++ b/t/02git_CollectionOfRepositories_FromDirectoryRecursive.t
@@ -30,7 +30,7 @@ my @sorted_names = sort map { $_->{name} } @{$repository_list};
 is_deeply( \@sorted_names, [ qw( bare.git barerecursive.git nodescription repo1 scratch.git) ], 'Repositories are correctly loaded' );
 
 dies_ok {
-  my $repository = $repo->get_repository("$repo_dir/NoSuchRepository");
+  my $repository = $repo->get_repository("NoSuchRepository");
 } 'throws exception for invalid repository';
 
 dies_ok {
@@ -41,7 +41,7 @@ dies_ok {
   my $repository = $repo->get_repository('../../../');
 } 'Relative directory not contained within repo_dir';
 
-my $repository = $repo->get_repository( "$repo_dir/repo1" );
+my $repository = $repo->get_repository( "repo1" );
 isa_ok($repository, 'Gitalist::Git::Repository');
 
 # check for bug where get_repository blew up if repo_dir
@@ -49,5 +49,7 @@ isa_ok($repository, 'Gitalist::Git::Repository');
 lives_ok {
   my $repo2_dir = "$Bin/lib/../lib/repositories";
   my $repo2 = Gitalist::Git::CollectionOfRepositories::FromDirectoryRecursive->new( repo_dir => $repo2_dir );
-  my $repo2_proj = $repo2->get_repository("$repo2_dir/repo1");
+  my $repo2_proj = $repo2->get_repository("repo1");
 } 'relative repo_dir properly handled';
+
+# TODO Write a recursive test ignoring directories