[PATCH v2] git svn: info: correctly handle absolute path args

2014-09-07 Thread Eric Wong
Calling git svn info $(pwd) would hit:
  Reading from filehandle failed at ...
errors due to improper prefixing and canonicalization.

Strip the toplevel path from absolute filesystem paths to ensure
downstream canonicalization routines are only exposed to paths
tracked in git (or SVN).

Thanks to Andrej Manduch amand...@gmail.com for originally
noticing the issue and fixing my original version of this to handle
more corner cases such as /path/to/top/../top and
/path/to/top/../top/file as shown in the new test cases.

Signed-off-by: Andrej Manduch amand...@gmail.com
Signed-off-by: Eric Wong normalper...@yhbt.net
---
Eric Wong normalper...@yhbt.net wrote:
 Thanks Andrej.  I'll queue that on top of mine.
 Can you turn that into a proper commit message with Subject?
 Thanks.

I just squashed in your change and gave you credit.
Sorry, I forgot about this completely.  Will test a bit more
and ask Junio to pull.

 git-svn.perl| 22 --
 t/t9119-git-svn-info.sh | 30 ++
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 1f41ee1..47cd6ea 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1477,10 +1477,20 @@ sub cmd_commit_diff {
}
 }
 
-
 sub cmd_info {
-   my $path = canonicalize_path(defined($_[0]) ? $_[0] : .);
-   my $fullpath = canonicalize_path($cmd_dir_prefix . $path);
+   my $path_arg = defined($_[0]) ? $_[0] : '.';
+   my $path = $path_arg;
+   if ($path =~ m!\A/!) {
+   my $toplevel = eval {
+   my @cmd = qw/rev-parse --show-toplevel/;
+   command_oneline(\@cmd, STDERR = 0);
+   };
+   $path = canonicalize_path($path);
+   $path =~ s!\A\Q$toplevel\E/?!!;
+   $path = canonicalize_path($path);
+   } else {
+   $path = canonicalize_path($cmd_dir_prefix . $path);
+   }
if (exists $_[1]) {
die Too many arguments specified\n;
}
@@ -1501,14 +1511,14 @@ sub cmd_info {
# canonicalize_path() will return  to make libsvn 1.5.x happy,
$path = . if $path eq ;
 
-   my $full_url = canonicalize_url( add_path_to_url( $url, $fullpath ) );
+   my $full_url = canonicalize_url( add_path_to_url( $url, $path ) );
 
if ($_url) {
print $full_url\n;
return;
}
 
-   my $result = Path: $path\n;
+   my $result = Path: $path_arg\n;
$result .= Name:  . basename($path) . \n if $file_type ne dir;
$result .= URL: $full_url\n;
 
@@ -1539,7 +1549,7 @@ sub cmd_info {
}
 
my ($lc_author, $lc_rev, $lc_date_utc);
-   my @args = Git::SVN::Log::git_svn_log_cmd($rev, $rev, --, $fullpath);
+   my @args = Git::SVN::Log::git_svn_log_cmd($rev, $rev, --, $path);
my $log = command_output_pipe(@args);
my $esc_color = qr/(?:\033\[(?:(?:\d+;)*\d*)?m)*/;
while ($log) {
diff --git a/t/t9119-git-svn-info.sh b/t/t9119-git-svn-info.sh
index ff19695..f16f323 100755
--- a/t/t9119-git-svn-info.sh
+++ b/t/t9119-git-svn-info.sh
@@ -74,6 +74,36 @@ test_expect_success 'info .' 
test_cmp_info expected.info-dot actual.info-dot

 
+test_expect_success 'info $(pwd)' '
+   (cd svnwc; svn info $(pwd)) expected.info-pwd 
+   (cd gitwc; git svn info $(pwd)) actual.info-pwd 
+   grep -v ^Path: expected.info-pwd expected.info-np 
+   grep -v ^Path: actual.info-pwd actual.info-np 
+   test_cmp_info expected.info-np actual.info-np 
+   test $(sed -ne \/^Path:/ s!/svnwc!!\ expected.info-pwd) = \
+$(sed -ne \/^Path:/ s!/gitwc!!\ actual.info-pwd)
+   '
+
+test_expect_success 'info $(pwd)/../___wc' '
+   (cd svnwc; svn info $(pwd)/../svnwc) expected.info-pwd 
+   (cd gitwc; git svn info $(pwd)/../gitwc) actual.info-pwd 
+   grep -v ^Path: expected.info-pwd expected.info-np 
+   grep -v ^Path: actual.info-pwd actual.info-np 
+   test_cmp_info expected.info-np actual.info-np 
+   test $(sed -ne \/^Path:/ s!/svnwc!!\ expected.info-pwd) = \
+$(sed -ne \/^Path:/ s!/gitwc!!\ actual.info-pwd)
+   '
+
+test_expect_success 'info $(pwd)/../___wc//file' '
+   (cd svnwc; svn info $(pwd)/../svnwc//file) expected.info-pwd 
+   (cd gitwc; git svn info $(pwd)/../gitwc//file) actual.info-pwd 
+   grep -v ^Path: expected.info-pwd expected.info-np 
+   grep -v ^Path: actual.info-pwd actual.info-np 
+   test_cmp_info expected.info-np actual.info-np 
+   test $(sed -ne \/^Path:/ s!/svnwc!!\ expected.info-pwd) = \
+$(sed -ne \/^Path:/ s!/gitwc!!\ actual.info-pwd)
+   '
+
 test_expect_success 'info --url .' '
test $(cd gitwc; git svn info --url .) = $quoted_svnrepo
'
-- 
EW
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info 

Re: [PATCH v2] git svn: info: correctly handle absolute path args

2014-09-07 Thread Johannes Sixt
Am 07.09.2014 10:06, schrieb Eric Wong:
 diff --git a/git-svn.perl b/git-svn.perl
 index 1f41ee1..47cd6ea 100755
 --- a/git-svn.perl
 +++ b/git-svn.perl
 @@ -1477,10 +1477,20 @@ sub cmd_commit_diff {
   }
  }
  
 -
  sub cmd_info {
 - my $path = canonicalize_path(defined($_[0]) ? $_[0] : .);
 - my $fullpath = canonicalize_path($cmd_dir_prefix . $path);
 + my $path_arg = defined($_[0]) ? $_[0] : '.';
 + my $path = $path_arg;
 + if ($path =~ m!\A/!) {

There must be a more portable way to check for an absolute path. Think
of DOS-style paths...

 + my $toplevel = eval {
 + my @cmd = qw/rev-parse --show-toplevel/;
 + command_oneline(\@cmd, STDERR = 0);
 + };
 + $path = canonicalize_path($path);
 + $path =~ s!\A\Q$toplevel\E/?!!;
 + $path = canonicalize_path($path);
 + } else {
 + $path = canonicalize_path($cmd_dir_prefix . $path);
 + }
   if (exists $_[1]) {
   die Too many arguments specified\n;
   }

-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] git svn: info: correctly handle absolute path args

2014-09-07 Thread brian m. carlson
On Sun, Sep 07, 2014 at 10:57:43AM +0200, Johannes Sixt wrote:
 Am 07.09.2014 10:06, schrieb Eric Wong:
   sub cmd_info {
  -   my $path = canonicalize_path(defined($_[0]) ? $_[0] : .);
  -   my $fullpath = canonicalize_path($cmd_dir_prefix . $path);
  +   my $path_arg = defined($_[0]) ? $_[0] : '.';
  +   my $path = $path_arg;
  +   if ($path =~ m!\A/!) {
 
 There must be a more portable way to check for an absolute path. Think
 of DOS-style paths...

You probably want File::Spec-file_name_is_absolute($path).  Either that
or always turn the path into absolute or relative form with
File::Spec-abs2rel or File::Spec-rel2abs, and then go from there.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature