fix hang in git fetch if pointed at a 0 length bundle
PATCH 1/1 fix hang in git fetch if pointed at a 0 length bundle
git-repo if interupted at the exact wrong time will generate zero length bundles- literal empty files. git-repo is wrong here, but git fetch shouldn't effectively spin loop if pointed at a zero length bundle.
--- bundle.c | 2 +- t/t5704-bundle.sh | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletions(-)
bundle.c +1 -1
@@ -31,7 +31,7 @@ static int strbuf_readline_fd(struct strbuf *sb, int fd)while (1 ) {char ch;ssize_t len =xread (fd, &ch,1 ); -if (len <0 ) +if (len <=0 )return -1 ;strbuf_addch (sb, ch);if (ch =='\n' )
t/t5704-bundle.sh +10 -0
@@ -53,4 +53,14 @@ test_expect_failure 'bundle --stdin <rev-list options>' ' ' +test_expect_success 'die if bundle file is empty' ' + + echo -n > empty-bundle + timeout 5 git fetch empty-bundle + ret=$? + [ $ret == 128 ] && return 0 + return $ret + +' + test_done
Signature · 1 line
1.7.8.2
PATCH 1/1 fix hang in git fetch if pointed at a 0 length bundle
git-repo if interupted at the exact wrong time will generate zero length bundles- literal empty files. git-repo is wrong here, but git fetch shouldn't effectively spin loop if pointed at a zero length bundle.
--- bundle.c | 4 ++-- t/t5704-bundle.sh | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-)
bundle.c +2 -2
@@ -31,8 +31,8 @@ static int strbuf_readline_fd(struct strbuf *sb, int fd)while (1 ) {char ch;ssize_t len =xread (fd, &ch,1 ); -if (len <0 ) -return -1 ; +if (len <=0 ) +return len;strbuf_addch (sb, ch);if (ch =='\n' )break ;
t/t5704-bundle.sh +6 -0
@@ -53,4 +53,10 @@ test_expect_failure 'bundle --stdin <rev-list options>' ' ' +test_expect_success 'empty bundle file is rejected' ' + + >empty-bundle && test_must_fail git fetch empty-bundle + +' + test_done
Signature · 1 line
1.7.8.2
Quoted 7 lines · Brian Harring
git-repo if interupted at the exact wrong time will generate zero length bundles- literal empty files. git-repo is wrong here, but git fetch shouldn't effectively spin loop if pointed at a zero length bundle.
Signed-off-by Brian Harring <[email protected]>---
Thanks.
Also thanks to all reviewers.
Am 03.01.2012 02:13, schrieb Brian Harring:
Quoted 4 lines
git-repo if interupted at the exact wrong time will generate zero length bundles- literal empty files. git-repo is wrong here, but git fetch shouldn't effectively spin loop if pointed at a zero length bundle.
Adding a test case is very much appreciated.
Quoted 1 line
+test_expect_success 'die if bundle file is empty' '
How about 'empty bundle file is rejected'?
t/t5704-bundle.sh test_expect_failure 'bundle --stdin <rev-list options>' ' · line 58
Quoted 2 lines
t/t5704-bundle.sh +2
-0+ + echo -n > empty-bundle
'echo -n' is not portable; use simply
>empty-bundle &&
(note the style: no blank after >). Also chain commands using &&.
Quoted 1 line
+ timeout 5 git fetch empty-bundle
Yes, there was an infinite loop. But we do not specifically protect our git invocations in the test suite against this sort of failure. Just write
test_must_fail git fetch empty-bundle
and end the test case here.
t/t5704-bundle.sh test_expect_failure 'bundle --stdin <rev-list options>' ' · line 64
Quoted 5 lines
t/t5704-bundle.sh +5
-0+ ret=$? + [ $ret == 128 ] && return 0 + return $ret + +'
Furthermore, indentation should be one tabstop, not blanks.
-- Hannes
bundle.c static int strbuf_readline_fd(struct strbuf *sb, int fd) · line 37
Quoted 9 lines · Brian Harring
bundle.c +1
-1@@ -31,7 +31,7 @@ static int strbuf_readline_fd(struct strbuf *sb, int fd) while (1) { char ch; ssize_t len = xread(fd, &ch, 1); - if (len < 0) + if (len <= 0) return -1; strbuf_addch(sb, ch); if (ch == '\n')
I think it should return 0 when len == 0 because strictly speaking eof is not a fault. It's not really important though because the two callers in this file work fine either way.
FWIW I went through all xread call sites. All seem to handle return value <= 0 correctly.
Signature · 1 line
Duy
Quoted 13 lines · Nguyen Thai Ngoc Duy
bundle.c static int strbuf_readline_fd(struct strbuf *sb, int fd) · line 37
Quoted 9 lines · Brian Harring
bundle.c +1
-1@@ -31,7 +31,7 @@ static int strbuf_readline_fd(struct strbuf *sb, int fd) while (1) { char ch; ssize_t len = xread(fd, &ch, 1); - if (len < 0) + if (len <= 0) return -1; strbuf_addch(sb, ch); if (ch == '\n')I think it should return 0 when len == 0 because strictly speaking eof is not a fault.
Even if you do not strictly speak, end of file is a perfectly normal thing to see, no? IOW wouldn't the original patch actively _break_ callers that read the whole file from the file descriptor to the end?
Quoted 2 lines
FWIW I went through all xread call sites. All seem to handle return value <= 0 correctly.
Thanks.