Do not take mode bits from index after type change.

When we do not trust executable bit from lstat(2), we copied
existing ce_mode bits without checking if the filesystem object
is a regular file (which is the only thing we apply the "trust
executable bit" business) nor if the blob in the index is a
regular file (otherwise, we should do the same as registering a
new regular file, which is to default non-executable).

Noticed by Johannes Sixt.

Signed-off-by: Junio C Hamano <junkio@cox.net>

diff --git a/builtin-apply.c b/builtin-apply.c
index 3fefdacd94526b62ac72707d6d80b039f104c06d..abe35387156cf2774c8bc619d74b93c5e83285a8 100644 (file)
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1988,7 +1988,7 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)
                        return error("%s: %s", old_name, strerror(errno));
 
                if (!cached)
                        return error("%s: %s", old_name, strerror(errno));
 
                if (!cached)

-                       st_mode = ntohl(create_ce_mode(st.st_mode));
+                       st_mode = ntohl(ce_mode_from_stat(ce, st.st_mode));

 
                if (patch->is_new < 0)
                        patch->is_new = 0;
 
                if (patch->is_new < 0)
                        patch->is_new = 0;

diff --git a/builtin-update-index.c b/builtin-update-index.c
index 1ac613a78869e5b0a2ac45ca6ab72adc18a55575..772aaba7bbfded782cd9c0adbc4199fdac0de642 100644 (file)
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -109,16 +109,17 @@ static int add_file_to_cache(const char *path)
        ce->ce_flags = htons(namelen);
        fill_stat_cache_info(ce, &st);
 
        ce->ce_flags = htons(namelen);
        fill_stat_cache_info(ce, &st);
 

-       ce->ce_mode = create_ce_mode(st.st_mode);
-       if (!trust_executable_bit) {
+       if (trust_executable_bit)
+               ce->ce_mode = create_ce_mode(st.st_mode);
+       else {

                /* If there is an existing entry, pick the mode bits
                 * from it, otherwise assume unexecutable.
                 */
                /* If there is an existing entry, pick the mode bits
                 * from it, otherwise assume unexecutable.
                 */

+               struct cache_entry *ent;

                int pos = cache_name_pos(path, namelen);
                int pos = cache_name_pos(path, namelen);

-               if (0 <= pos)
-                       ce->ce_mode = active_cache[pos]->ce_mode;
-               else if (S_ISREG(st.st_mode))
-                       ce->ce_mode = create_ce_mode(S_IFREG | 0666);
+
+               ent = (0 <= pos) ? active_cache[pos] : NULL;
+               ce->ce_mode = ce_mode_from_stat(ent, st.st_mode);

        }
 
        if (index_path(ce->sha1, path, &st, !info_only))
        }
 
        if (index_path(ce->sha1, path, &st, !info_only))

diff --git a/cache.h b/cache.h
index c62b0b090d2b3e628764557127f1629d26459fbf..04f8e63baf6c23d9a27618b642b98506c9db688a 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -106,6 +106,16 @@ static inline unsigned int create_ce_mode(unsigned int mode)
                return htonl(S_IFLNK);
        return htonl(S_IFREG | ce_permissions(mode));
 }
                return htonl(S_IFLNK);
        return htonl(S_IFREG | ce_permissions(mode));
 }

+static inline unsigned int ce_mode_from_stat(struct cache_entry *ce, unsigned int mode)
+{
+       extern int trust_executable_bit;
+       if (!trust_executable_bit && S_ISREG(mode)) {
+               if (ce && S_ISREG(ntohl(ce->ce_mode)))
+                       return ce->ce_mode;
+               return create_ce_mode(0666);
+       }
+       return create_ce_mode(mode);
+}

 #define canon_mode(mode) \
        (S_ISREG(mode) ? (S_IFREG | ce_permissions(mode)) : \
        S_ISLNK(mode) ? S_IFLNK : S_IFDIR)
 #define canon_mode(mode) \
        (S_ISREG(mode) ? (S_IFREG | ce_permissions(mode)) : \
        S_ISLNK(mode) ? S_IFLNK : S_IFDIR)

diff --git a/diff-lib.c b/diff-lib.c
index 91cd87742f9efd2c0fcf878c179bc6ed989cb50a..556d5345bfc74c720d35097d24322436ffdebe57 100644 (file)
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -170,9 +170,7 @@ static int get_stat_data(struct cache_entry *ce,
                }
                changed = ce_match_stat(ce, &st, 0);
                if (changed) {
                }
                changed = ce_match_stat(ce, &st, 0);
                if (changed) {

-                       mode = create_ce_mode(st.st_mode);
-                       if (!trust_executable_bit && S_ISREG(st.st_mode))
-                               mode = ce->ce_mode;
+                       mode = ce_mode_from_stat(ce, st.st_mode);

                        sha1 = no_sha1;
                }
        }
                        sha1 = no_sha1;
                }
        }

diff --git a/read-cache.c b/read-cache.c
index c54a61187711087b98138b9598db6353457e4df3..605b35239674c72a272e1c0fc1fb886c4b75be46 100644 (file)
--- a/read-cache.c
+++ b/read-cache.c
@@ -344,16 +344,17 @@ int add_file_to_index(const char *path, int verbose)
        ce->ce_flags = htons(namelen);
        fill_stat_cache_info(ce, &st);
 
        ce->ce_flags = htons(namelen);
        fill_stat_cache_info(ce, &st);
 

-       ce->ce_mode = create_ce_mode(st.st_mode);
-       if (!trust_executable_bit) {
+       if (trust_executable_bit)
+               ce->ce_mode = create_ce_mode(st.st_mode);
+       else {

                /* If there is an existing entry, pick the mode bits
                 * from it, otherwise assume unexecutable.
                 */
                /* If there is an existing entry, pick the mode bits
                 * from it, otherwise assume unexecutable.
                 */

+               struct cache_entry *ent;

                int pos = cache_name_pos(path, namelen);
                int pos = cache_name_pos(path, namelen);

-               if (pos >= 0)
-                       ce->ce_mode = active_cache[pos]->ce_mode;
-               else if (S_ISREG(st.st_mode))
-                       ce->ce_mode = create_ce_mode(S_IFREG | 0666);
+
+               ent = (0 <= pos) ? active_cache[pos] : NULL;
+               ce->ce_mode = ce_mode_from_stat(ent, st.st_mode);

        }
 
        if (index_path(ce->sha1, path, &st, 1))
        }
 
        if (index_path(ce->sha1, path, &st, 1))

diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index caaab26c2f3f2d04581a6c6918ab36f97a51f975..08e035220cfd5c2da0798eba84c779981f185b4a 100755 (executable)
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -30,6 +30,16 @@ test_expect_success \
         *) echo fail; git-ls-files --stage xfoo1; (exit 1);;
         esac'
 
         *) echo fail; git-ls-files --stage xfoo1; (exit 1);;
         esac'
 

+test_expect_success 'git-add: filemode=0 should not get confused by symlink' '
+       rm -f xfoo1 &&
+       ln -s foo xfoo1 &&
+       git-add xfoo1 &&
+       case "`git-ls-files --stage xfoo1`" in
+       120000" "*xfoo1) echo ok;;
+       *) echo fail; git-ls-files --stage xfoo1; (exit 1);;
+       esac
+'
+

 test_expect_success \
        'git-update-index --add: Test that executable bit is not used...' \
        'git config core.filemode 0 &&
 test_expect_success \
        'git-update-index --add: Test that executable bit is not used...' \
        'git config core.filemode 0 &&


@@ -41,6 +51,16 @@ test_expect_success \
         *) echo fail; git-ls-files --stage xfoo2; (exit 1);;
         esac'
 
         *) echo fail; git-ls-files --stage xfoo2; (exit 1);;
         esac'
 

+test_expect_success 'git-add: filemode=0 should not get confused by symlink' '
+       rm -f xfoo2 &&
+       ln -s foo xfoo2 &&
+       git update-index --add xfoo2 &&
+       case "`git-ls-files --stage xfoo2`" in
+       120000" "*xfoo2) echo ok;;
+       *) echo fail; git-ls-files --stage xfoo2; (exit 1);;
+       esac
+'
+

 test_expect_success \
        'git-update-index --add: Test that executable bit is not used...' \
        'git config core.filemode 0 &&
 test_expect_success \
        'git-update-index --add: Test that executable bit is not used...' \
        'git config core.filemode 0 &&