[PATCH] opening files in remote.c should ensure it is opening a file

HP-UX allows directories to be opened with fopen (path, "r"), which
will cause some translations that expect to read files, read dirs
instead. This patch makes sure the two fopen () calls in remote.c
only open the file if it is a file.

Signed-off-by: H.Merijn Brand <redacted>
---

diff -pur git-1.5.4a/remote.c git-1.5.4b/remote.c

--- git-1.5.4a/remote.c    2008-01-27 09:04:18 +0100
+++ git-1.5.4/remote.c     2008-02-08 17:38:43 +0100

@@ -121,9 +121,18 @@ static struct branch *make_branch(const
        return branches[empty];
 }

+/* Helper function to ensure that we are opening a file and not a directory */
+static FILE *open_file(char *full_path)
+{
+       struct stat st_buf;
+       if (stat(full_path, &st_buf) || !S_ISREG(st_buf.st_mode))
+               return NULL;
+       return (fopen(full_path, "r"));
+}
+
 static void read_remotes_file(struct remote *remote)
 {
-       FILE *f = fopen(git_path("remotes/%s", remote->name), "r");
+       FILE *f = open_file(git_path("remotes/%s", remote->name));

        if (!f)
                return;

@@ -173,7 +182,7 @@ static void read_branches_file(struct re
        char *frag;
        char *branch;
        int n = slash ? slash - remote->name : 1000;
-       FILE *f = fopen(git_path("branches/%.*s", n, remote->name), "r");
+       FILE *f = open_file(git_path("branches/%.*s", n, remote->name));
        char *s, *p;
        int len;

--

git-1.5.4

-- 
H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)
using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,
& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org
http://mirrors.develooper.com/hpux/            http://www.test-smoke.org
                        http://www.goldmark.org/jeff/stupid-disclaimers/

On Feb 8, 2008 4:46 PM, H.Merijn Brand [off-list ref] wrote:

HP-UX allows directories to be opened with fopen (path, "r"), which
will cause some translations that expect to read files, read dirs
instead. This patch makes sure the two fopen () calls in remote.c
only open the file if it is a file.

Signed-off-by: H.Merijn Brand <redacted>

Many thanks, this is also required for AIX. I had got some way to
tracking it down, but I thought it was an issue with strbuf. So...

Tested-by: Mike Ralphson <redacted>

Your other fix there [- if (!strbuf_avail(sb)) / + if
(strbuf_avail(sb) < 64) ] is, guess what, also required on AIX.

Thanks again.

On Fri, 8 Feb 2008 17:25:52 +0000, "Mike Ralphson" [off-list ref]
wrote:

On Feb 8, 2008 4:46 PM, H.Merijn Brand [off-list ref] wrote:

quoted

HP-UX allows directories to be opened with fopen (path, "r"), which
will cause some translations that expect to read files, read dirs
instead. This patch makes sure the two fopen () calls in remote.c
only open the file if it is a file.

Signed-off-by: H.Merijn Brand <redacted>

Many thanks, this is also required for AIX. I had got some way to
tracking it down, but I thought it was an issue with strbuf. So...

Tested-by: Mike Ralphson <redacted>

Your other fix there [- if (!strbuf_avail(sb)) / + if
(strbuf_avail(sb) < 64) ] is, guess what, also required on AIX.

Thanks again.

Not there yet ...

$ cat do-tests
#!/bin/sh

export TAR=ntar
rm -f *.err
for t in t[0-9]*.sh ; do
    echo $t
    sh $t > test.err 2>&1 || mv test.err $t.err
    rm -f test.err
    done
$

197509 -rw-rw-rw- 1 merijn softwr 1633 Feb  8 18:03 t5302-pack-index.sh.err
196846 -rw-rw-rw- 1 merijn softwr  943 Feb  8 18:04 t5500-fetch-pack.sh.err
203431 -rw-rw-rw- 1 merijn softwr  344 Feb  8 18:05 t5600-clone-fail-cleanup.sh.err
202602 -rw-rw-rw- 1 merijn softwr  458 Feb  8 18:05 t5701-clone-local.sh.err
202761 -rw-rw-rw- 1 merijn softwr 3039 Feb  8 18:06 t6002-rev-list-bisect.sh.err
202641 -rw-rw-rw- 1 merijn softwr 3980 Feb  8 18:06 t6003-rev-list-topo-order.sh.err
202731 -rw-rw-rw- 1 merijn softwr  899 Feb  8 18:06 t6022-merge-rename.sh.err
197510 -rw-rw-rw- 1 merijn softwr 1340 Feb  8 18:08 t7201-co.sh.err
202705 -rw-rw-rw- 1 merijn softwr  149 Feb  8 18:09 t9300-fast-import.sh.err
197051 -rw-rw-rw- 1 merijn softwr 1651 Feb  8 18:09 t9301-fast-export.sh.err

http://www.xs4all.nl/~procura/git-1.5.3-1123ipf.tar

Tips welcome :)

-- 
H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)
using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,
& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org
http://mirrors.develooper.com/hpux/            http://www.test-smoke.org
                        http://www.goldmark.org/jeff/stupid-disclaimers/

+/* Helper function to ensure that we are opening a file and not a directory */
+static FILE *open_file(char *full_path)
+{
+       struct stat st_buf;
+       if (stat(full_path, &st_buf) || !S_ISREG(st_buf.st_mode))
+               return NULL;
+       return (fopen(full_path, "r"));
+}

That looks wrong.  stat+fopen has a pointless race condition that
open+fstat+fdopen would not have.

Morten

On Fri, 8 Feb 2008, Mike Ralphson wrote:

On Feb 8, 2008 4:46 PM, H.Merijn Brand [off-list ref] wrote:

quoted

HP-UX allows directories to be opened with fopen (path, "r"), which
will cause some translations that expect to read files, read dirs
instead. This patch makes sure the two fopen () calls in remote.c
only open the file if it is a file.

Signed-off-by: H.Merijn Brand <redacted>

Many thanks, this is also required for AIX. I had got some way to
tracking it down, but I thought it was an issue with strbuf. So...

Does the following help? We really ought to know that ".." must be a path 
literal (and there obviously should be more limitations on nicknames for 
remotes, but I haven't figured out what they should be yet).

	-Daniel
*This .sig left intentionally blank*

diff --git a/remote.c b/remote.c
index 0e00680..83a3d9d 100644
--- a/remote.c
+++ b/remote.c

@@ -348,7 +348,7 @@ struct remote *remote_get(const char *name)
        if (!name)
                name = default_remote_name;
        ret = make_remote(name, 0);
-       if (name[0] != '/') {
+       if (name[0] != '/' && strcmp(name, "..")) {
                if (!ret->url)
                        read_remotes_file(ret);
                if (!ret->url)

Hi,

On Fri, 8 Feb 2008, Daniel Barkalow wrote:

You'll need to check for ".", too: "git pull . <branch>" was originally 
the only way to merge a local branch, and it is still valid.

Ciao,
Dscho

On Fri, 8 Feb 2008 21:04:47 +0100, "H.Merijn Brand" [off-list ref]
wrote:

$ cat do-tests
#!/bin/sh

export TAR=ntar
rm -f *.err
for t in t[0-9]*.sh ; do
    echo $t
    sh $t > test.err 2>&1 || mv test.err $t.err
    rm -f test.err
    done
$

197509 -rw-rw-rw- 1 merijn softwr 1633 Feb  8 18:03 t5302-pack-index.sh.err
196846 -rw-rw-rw- 1 merijn softwr  943 Feb  8 18:04 t5500-fetch-pack.sh.err
203431 -rw-rw-rw- 1 merijn softwr  344 Feb  8 18:05 t5600-clone-fail-cleanup.sh.err
202602 -rw-rw-rw- 1 merijn softwr  458 Feb  8 18:05 t5701-clone-local.sh.err
202761 -rw-rw-rw- 1 merijn softwr 3039 Feb  8 18:06 t6002-rev-list-bisect.sh.err
202641 -rw-rw-rw- 1 merijn softwr 3980 Feb  8 18:06 t6003-rev-list-topo-order.sh.err
202731 -rw-rw-rw- 1 merijn softwr  899 Feb  8 18:06 t6022-merge-rename.sh.err
197510 -rw-rw-rw- 1 merijn softwr 1340 Feb  8 18:08 t7201-co.sh.err
202705 -rw-rw-rw- 1 merijn softwr  149 Feb  8 18:09 t9300-fast-import.sh.err
197051 -rw-rw-rw- 1 merijn softwr 1651 Feb  8 18:09 t9301-fast-export.sh.err

http://www.xs4all.nl/~procura/git-1.5.3-1123ipf.tar

Tips welcome :)

Most bizarre workaround found for clone (the first 4 failures):
--8<---
diff -pur /a5/pro/3gl/LINUX/git-1.5.4/git-clone.sh git-clone.sh

--- a/git-1.5.4/git-clone.sh  2008-02-02 05:09:01 +0100
+++ b/git-1.5.4/git-clone.sh  2008-02-18 10:03:26 +0100

@@ -368,7 +368,8 @@ yes)
                '') git-fetch-pack --all -k $quiet $depth $no_progress "$repo";;
                *) git-fetch-pack --all -k $quiet "$upload_pack" $depth $no_progress "$repo" ;;
                esac >"$GIT_DIR/CLONE_HEAD" ||
-                       die "fetch-pack from '$repo' failed."
+                       exit 1
+                       # die "fetch-pack from '$repo' failed."
                ;;
        esac
        ;;
-->8---

4 down, 6 to go
197225 -rw-rw-rw- 1 merijn softwr  7246 Feb 18 09:58 t6002-rev-list-bisect.sh.err
197111 -rw-rw-rw- 1 merijn softwr 10763 Feb 18 09:58 t6003-rev-list-topo-order.sh.err
197190 -rw-rw-rw- 1 merijn softwr 17903 Feb 18 09:58 t6022-merge-rename.sh.err
196841 -rw-rw-rw- 1 merijn softwr  9299 Feb 18 10:00 t7201-co.sh.err
196928 -rw-rw-rw- 1 merijn softwr   484 Feb 18 10:01 t9300-fast-import.sh.err
196683 -rw-rw-rw- 1 merijn softwr  5035 Feb 18 10:01 t9301-fast-export.sh.err

-- 
H.Merijn Brand         Amsterdam Perl Mongers (http://amsterdam.pm.org/)
using & porting perl 5.6.2, 5.8.x, 5.10.x  on HP-UX 10.20, 11.00, 11.11,
& 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin.       http://qa.perl.org
http://mirrors.develooper.com/hpux/            http://www.test-smoke.org
                        http://www.goldmark.org/jeff/stupid-disclaimers/