Skip to content

Commit

Permalink
read-cache: optionally disallow HFS+ .git variants
Browse files Browse the repository at this point in the history
The point of disallowing ".git" in the index is that we
would never want to accidentally overwrite files in the
repository directory. But this means we need to respect the
filesystem's idea of when two paths are equal. The prior
commit added a helper to make such a comparison for HFS+;
let's use it in verify_path.

We make this check optional for two reasons:

  1. It restricts the set of allowable filenames, which is
     unnecessary for people who are not on HFS+. In practice
     this probably doesn't matter, though, as the restricted
     names are rather obscure and almost certainly would
     never come up in practice.

  2. It has a minor performance penalty for every path we
     insert into the index.

This patch ties the check to the core.protectHFS config
option. Though this is expected to be most useful on OS X,
we allow it to be set everywhere, as HFS+ may be mounted on
other platforms. The variable does default to on for OS X,
though.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Dec 17, 2014
1 parent 6162a1d commit a42643a
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 5 deletions.
5 changes: 5 additions & 0 deletions Documentation/config.txt
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,11 @@ core.precomposeunicode::
When false, file names are handled fully transparent by Git,
which is backward compatible with older versions of Git.

core.protectHFS::
If set to true, do not allow checkout of paths that would
be considered equivalent to `.git` on an HFS+ filesystem.
Defaults to `true` on Mac OS, and `false` elsewhere.

core.trustctime::
If false, the ctime differences between the index and the
working tree are ignored; useful when the inode change time
Expand Down
1 change: 1 addition & 0 deletions cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,7 @@ extern int fsync_object_files;
extern int core_preload_index;
extern int core_apply_sparse_checkout;
extern int precomposed_unicode;
extern int protect_hfs;

/*
* The character that begins a commented line in user-editable file
Expand Down
5 changes: 5 additions & 0 deletions config.c
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,11 @@ static int git_default_core_config(const char *var, const char *value)
return 0;
}

if (!strcmp(var, "core.protecthfs")) {
protect_hfs = git_config_bool(var, value);
return 0;
}

/* Add other config variables here and to Documentation/config.txt. */
return 0;
}
Expand Down
1 change: 1 addition & 0 deletions config.mak.uname
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ ifeq ($(uname_S),Darwin)
HAVE_DEV_TTY = YesPlease
COMPAT_OBJS += compat/precompose_utf8.o
BASIC_CFLAGS += -DPRECOMPOSE_UNICODE
BASIC_CFLAGS += -DPROTECT_HFS_DEFAULT=1
endif
ifeq ($(uname_S),SunOS)
NEEDS_SOCKET = YesPlease
Expand Down
5 changes: 5 additions & 0 deletions environment.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
struct startup_info *startup_info;
unsigned long pack_size_limit_cfg;

#ifndef PROTECT_HFS_DEFAULT
#define PROTECT_HFS_DEFAULT 0
#endif
int protect_hfs = PROTECT_HFS_DEFAULT;

/*
* The character that begins a commented line in user-editable file
* that is subject to stripspace.
Expand Down
3 changes: 3 additions & 0 deletions read-cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "resolve-undo.h"
#include "strbuf.h"
#include "varint.h"
#include "utf8.h"

static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);

Expand Down Expand Up @@ -786,6 +787,8 @@ int verify_path(const char *path)
return 1;
if (is_dir_sep(c)) {
inside:
if (protect_hfs && is_hfs_dotgit(path))
return 0;
c = *path++;
if ((c == '.' && !verify_dotfile(path)) ||
is_dir_sep(c) || c == '\0')
Expand Down
24 changes: 20 additions & 4 deletions t/t1014-read-tree-confusing.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,39 @@ test_expect_success 'create base tree' '
tree=$(git rev-parse HEAD^{tree})
'

while read path; do
test_expect_success "reject $path at end of path" '
test_expect_success 'enable core.protectHFS for rejection tests' '
git config core.protectHFS true
'

while read path pretty; do
: ${pretty:=$path}
test_expect_success "reject $pretty at end of path" '
printf "100644 blob %s\t%s" "$blob" "$path" >tree &&
bogus=$(git mktree <tree) &&
test_must_fail git read-tree $bogus
'

test_expect_success "reject $path as subtree" '
test_expect_success "reject $pretty as subtree" '
printf "040000 tree %s\t%s" "$tree" "$path" >tree &&
bogus=$(git mktree <tree) &&
test_must_fail git read-tree $bogus
'
done <<-\EOF
done <<-EOF
.
..
.git
.GIT
${u200c}.Git {u200c}.Git
.gI${u200c}T .gI{u200c}T
.GiT${u200c} .GiT{u200c}
EOF

test_expect_success 'utf-8 paths allowed with core.protectHFS off' '
test_when_finished "git read-tree HEAD" &&
test_config core.protectHFS false &&
printf "100644 blob %s\t%s" "$blob" ".gi${u200c}t" >tree &&
ok=$(git mktree <tree) &&
git read-tree $ok
'

test_done
6 changes: 5 additions & 1 deletion t/test-lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,11 @@ _z40=0000000000000000000000000000000000000000
LF='
'

export _x05 _x40 _z40 LF
# UTF-8 ZERO WIDTH NON-JOINER, which HFS+ ignores
# when case-folding filenames
u200c=$(printf '\342\200\214')

export _x05 _x40 _z40 LF u200c

# Each test should start with something like this, after copyright notices:
#
Expand Down

0 comments on commit a42643a

Please sign in to comment.