Re: [PATCH] use refnames instead of left/right in dirdiffs
Hi John.
On Wed, 2013-03-27 at 23:07 +, John Keeping wrote:
That's not going to work well on Windows, is it?
Uhm Winwhat? No seriously... doesn't dir-diff fail ther anway? The mkdir
right now also uses mkpath with /... and I could read in it's
documentation that it would automatically translate this, does it?
Anything with two dots
in is already forbidden so we don't need to worry about that
Sure about this? I mean they're forbidden as refnames, but is this
really checked within git-difftool before?
; I'm not
sure we need to remove forward slashes at all
Mhh could be... seems that the cleanup happens via completely removing
the $tmpdir path.
And for the actual diff it shouldn't matter when there's a partentdir
more.
until we consider the
commit containing syntax ':/fix nasty bug' or 'master^{/fix bug}'.
Phew... don't ask me... I'm not that into the git source code... from
the filename side, these shouldn't bother, only / an NUL is forbidden
(in POSIX,... again I haven't checked win/mac which might not adhere to
the standards).
I'm more concerned with specifiers containing '^', '@', '{', ':' - see
'SPECIFYING REVISIONS' in git-rev-parse(1) for the full details of
what's acceptable.
Mhh there are likely more problems... I just noted that $ldir/$rdir are
used in a call to system() so... that needs to be shell escaped to
prevent any bad stuff
And if perl (me not a perl guy) interprets any such characters specially
in strings, it must be handled either.
At some point I think it may be better to fall back
to the SHA1 of the relevant commit.
Think that would be quite a drawback...
Cheers,
Chris.
smime.p7s
Description: S/MIME cryptographic signature
[PATCH] use refnames instead of left/right in dirdiffs
Currently, when a dir-diff is made with git-difftool the two revisions are
stored in two temporary directories .../left and .../right.
Many difftools show these pathnames in ther UI and therefore it would be helpful
for users, if actual reference names specified as progam arguments was used
instead.
Reference names might contain slash / characters which are not allowed to be
part of a file name. These must therefore be encoded.
Also, reference names that would could possibly break out of the temporary
directory (e.g. /foo, foo/../bar or foo/././bar) must be sanitised.
* Added a subroutine escape_reference_to_single_directory_name() which encodes a
reference name to a valid single directory name.
Any occurance of a slash / is replaced by two backslashes \\.
Having a backslash \ in a reference name should be forbidden, but just to be
save from collisions, any occurance of a backslash \ is replaced by a
backslash followed by an underscore \_ at first.
* Use this new function to construct the pathnames of the temporary directories
for the two revisions in dir-diffs.
Signed-off-by: Christoph Anton Mitterer m...@christoph.anton.mitterer.name
diff --git a/git-difftool.perl b/git-difftool.perl
index 12231fb..53e756d 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -83,6 +83,28 @@ sub exit_cleanup
exit($status | ($status 8));
}
+sub escape_reference_to_single_directory_name
+{
+ # Git allows reference names (see git-check-ref-format(1)) which cannot
+ # be directly mapped to a single directory name.
+ #
+ # This subroutines replaces any occurance of a slash / by two
+ # backslashes \\.
+ # Thereby, break-out attempts like /foo, foo/../bar or foo/././bar
+ # are prevented, too.
+ #
+ # Having a backslash \ in a reference name should be forbidden, but just
+ # to be save from collisions, any occurance of a backslash \ is replaced
+ # by a backslash followed by an underscore \_ at first.
+
+ my ($commit_name) = @_;
+
+ $commit_name =~ s/\\/\\_/g;
+ $commit_name =~ s/\///g;
+
+ return $commit_name;
+}
+
sub setup_dir_diff
{
my ($repo, $workdir, $symlinks) = @_;
@@ -169,8 +191,13 @@ EOF
# Setup temp directories
my $tmpdir = tempdir('git-difftool.X', CLEANUP = 0, TMPDIR = 1);
- my $ldir = $tmpdir/left;
- my $rdir = $tmpdir/right;
+ my $ldir = $tmpdir/ .
escape_reference_to_single_directory_name($ARGV[0]);
+ my $rdir = $tmpdir/;
+ if (@ARGV 2) {
+ $rdir .= 'HEAD';
+ } else {
+ $rdir .= escape_reference_to_single_directory_name($ARGV[1]);
+ }
mkpath($ldir) or exit_cleanup($tmpdir, 1);
mkpath($rdir) or exit_cleanup($tmpdir, 1);
smime.p7s
Description: S/MIME cryptographic signature
Re: [PATCH] use refnames instead of left/right in dirdiffs
On Wed, Mar 27, 2013 at 11:13:17PM +0100, Christoph Anton Mitterer wrote:
Currently, when a dir-diff is made with git-difftool the two revisions are
stored in two temporary directories .../left and .../right.
Many difftools show these pathnames in ther UI and therefore it would be
helpful
for users, if actual reference names specified as progam arguments was used
instead.
Reference names might contain slash / characters which are not allowed to be
part of a file name. These must therefore be encoded.
Also, reference names that would could possibly break out of the temporary
directory (e.g. /foo, foo/../bar or foo/././bar) must be sanitised.
* Added a subroutine escape_reference_to_single_directory_name() which
encodes a
reference name to a valid single directory name.
Any occurance of a slash / is replaced by two backslashes \\.
Having a backslash \ in a reference name should be forbidden, but just to be
save from collisions, any occurance of a backslash \ is replaced by a
backslash followed by an underscore \_ at first.
* Use this new function to construct the pathnames of the temporary
directories
for the two revisions in dir-diffs.
Signed-off-by: Christoph Anton Mitterer m...@christoph.anton.mitterer.name
diff --git a/git-difftool.perl b/git-difftool.perl
index 12231fb..53e756d 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -83,6 +83,28 @@ sub exit_cleanup
exit($status | ($status 8));
}
+sub escape_reference_to_single_directory_name
+{
+ # Git allows reference names (see git-check-ref-format(1)) which
cannot
+ # be directly mapped to a single directory name.
+ #
+ # This subroutines replaces any occurance of a slash / by two
+ # backslashes \\.
+ # Thereby, break-out attempts like /foo, foo/../bar or
foo/././bar
+ # are prevented, too.
That's not going to work well on Windows, is it? Anything with two dots
in is already forbidden so we don't need to worry about that; I'm not
sure we need to remove forward slashes at all, until we consider the
commit containing syntax ':/fix nasty bug' or 'master^{/fix bug}'.
I'm more concerned with specifiers containing '^', '@', '{', ':' - see
'SPECIFYING REVISIONS' in git-rev-parse(1) for the full details of
what's acceptable. At some point I think it may be better to fall back
to the SHA1 of the relevant commit.
+ #
+ # Having a backslash \ in a reference name should be forbidden, but
just
+ # to be save from collisions, any occurance of a backslash \ is
replaced
+ # by a backslash followed by an underscore \_ at first.
+
+ my ($commit_name) = @_;
+
+ $commit_name =~ s/\\/\\_/g;
+ $commit_name =~ s/\///g;
+
+ return $commit_name;
+}
+
sub setup_dir_diff
{
my ($repo, $workdir, $symlinks) = @_;
@@ -169,8 +191,13 @@ EOF
# Setup temp directories
my $tmpdir = tempdir('git-difftool.X', CLEANUP = 0, TMPDIR = 1);
- my $ldir = $tmpdir/left;
- my $rdir = $tmpdir/right;
+ my $ldir = $tmpdir/ .
escape_reference_to_single_directory_name($ARGV[0]);
+ my $rdir = $tmpdir/;
+ if (@ARGV 2) {
+ $rdir .= 'HEAD';
+ } else {
+ $rdir .= escape_reference_to_single_directory_name($ARGV[1]);
+ }
I don't think this approach is general enough, since git-difftool
accepts the same range of arguments that git-diff does.
We need to walk over @ARGV here, ignore anything starting with '-', stop
when we reach '--' and handle the arguments found in there including the
three acceptable range forms:
commit commit
commit..commit
commit...commit
replacing missing commits with HEAD.
mkpath($ldir) or exit_cleanup($tmpdir, 1);
mkpath($rdir) or exit_cleanup($tmpdir, 1);
--
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
