ref: 2204634275cbb3337cfc7cc994b57f9aa4017d0d
parent: fad1b3f7f77f08764f0e32f8ce18bee16da3aa69
author: Ori Bernstein <ori@eigenstate.org>
date: Fri Jul 16 20:10:44 EDT 2021
git/fetch: ensure we clean packfiles on failure When pulling into a git repository that is group writable as a non-owner, the pack file is left in place because we do not have permission to remove it. We also leave it behind if we bail out early due to an error, or due to only listing the changes. This pushes down the creation of the file, and cleans it up on error. thanks to Anthony Martin for spotting the bug. git/fetch: ensure we clean packfiles on failure When pulling into a git repository that is group writable as a non-owner, the pack file is left in place because we do not have permission to remove it. We also leave it behind if we bail out early due to an error, or due to only listing the changes. This pushes down the creation of the file, and cleans it up on error. Also, while we're here, clean up index caching, and ensure we close the fd in all cases. thanks to Anthony Martin for spotting the bug.
--- a/sys/src/cmd/git/fetch.c
+++ b/sys/src/cmd/git/fetch.c
@@ -5,7 +5,6 @@
char *fetchbranch;
char *upstream = "origin";
-char *packtmp = ".git/objects/pack/fetch.tmp";
int listonly;
int
@@ -111,7 +110,7 @@
for(p=strchr(s+1, '/'); p; p=strchr(p+1, '/')){
*p = 0;
if(access(s, AEXIST) != 0){
- fd = create(s, OREAD, DMDIR | 0755);
+ fd = create(s, OREAD, DMDIR | 0775);
if(fd == -1)
return -1;
close(fd);
@@ -162,13 +161,30 @@
}
}
+void
+fail(char *pack, char *idx, char *msg, ...)
+{
+ char buf[ERRMAX];
+ va_list ap;
+
+ va_start(ap, msg);
+ snprint(buf, sizeof(buf), msg, ap);
+ va_end(ap);
+
+ remove(pack);
+ remove(idx);
+ fprint(2, "%s", buf);
+ exits(buf);
+}
+
int
-fetchpack(Conn *c, int pfd, char *packtmp)
+fetchpack(Conn *c)
{
- char buf[Pktmax], idxtmp[256], *sp[3];
+ char buf[Pktmax], *sp[3];
+ char *packtmp, *idxtmp;
Hash h, *have, *want;
int nref, refsz, first;
- int i, n, req;
+ int i, n, req, pfd;
vlong packsz;
Object *o;
@@ -249,6 +265,15 @@
sysfatal("read: %r");
buf[n] = 0;
+ if((packtmp = smprint(".git/objects/pack/fetch.%d.pack", getpid())) == nil)
+ sysfatal("smprint: %r");
+ if((idxtmp = smprint(".git/objects/idx/fetch.%d.idx", getpid())) == nil)
+ sysfatal("smprint: %r");
+ if(mkoutpath(packtmp) == -1)
+ sysfatal("could not create %s: %r", packtmp);
+ if((pfd = create(packtmp, ORDWR, 0664)) == -1)
+ sysfatal("could not create %s: %r", packtmp);
+
fprint(2, "fetching...\n");
packsz = 0;
while(1){
@@ -259,19 +284,17 @@
sysfatal("fetch packfile: %r");
packsz += n;
}
+
closeconn(c);
if(seek(pfd, 0, 0) == -1)
- sysfatal("packfile seek: %r");
+ fail(packtmp, idxtmp, "packfile seek: %r");
if(checkhash(pfd, packsz, &h) == -1)
- sysfatal("corrupt packfile: %r");
+ fail(packtmp, idxtmp, "corrupt packfile: %r");
close(pfd);
- n = strlen(packtmp) - strlen(".tmp");
- memcpy(idxtmp, packtmp, n);
- memcpy(idxtmp + n, ".idx", strlen(".idx") + 1);
if(indexpack(packtmp, idxtmp, h) == -1)
- sysfatal("could not index fetched pack: %r");
+ fail(packtmp, idxtmp, "could not index fetched pack: %r");
if(rename(packtmp, idxtmp, h) == -1)
- sysfatal("could not rename indexed pack: %r");
+ fail(packtmp, idxtmp, "could not rename indexed pack: %r");
return 0;
}
@@ -287,7 +310,6 @@
void
main(int argc, char **argv)
{
- int pfd;
Conn c;
ARGBEGIN{
@@ -302,14 +324,9 @@
if(argc != 1)
usage();
- if(mkoutpath(packtmp) == -1)
- sysfatal("could not create %s: %r", packtmp);
- if((pfd = create(packtmp, ORDWR, 0644)) == -1)
- sysfatal("could not create %s: %r", packtmp);
-
if(gitconnect(&c, argv[0], "upload") == -1)
sysfatal("could not dial %s: %r", argv[0]);
- if(fetchpack(&c, pfd, packtmp) == -1)
+ if(fetchpack(&c) == -1)
sysfatal("fetch failed: %r");
closeconn(&c);
exits(nil);
--- a/sys/src/cmd/git/pack.c
+++ b/sys/src/cmd/git/pack.c
@@ -197,23 +197,22 @@
}
}
if((ifd = open(buf, OREAD)) == -1)
- goto error;
- if((d = dirfstat(ifd)) == nil)
- goto error;
+ return -1;
+ if((d = dirfstat(ifd)) == nil){
+ close(ifd);
+ return -1;
+ }
pf->nidx = d->length;
pf->idx = emalloc(pf->nidx);
if(readn(ifd, pf->idx, pf->nidx) != pf->nidx){
+ close(ifd);
free(pf->idx);
free(d);
- goto error;
+ return -1;
}
+ close(ifd);
free(d);
return 0;
-
-error:
- if(ifd != -1)
- close(ifd);
- return -1;
}
static void