threads / bug / 3711

Bug encountered while comitting

Subject: Bug encountered while comitting

## tl;dr

7 messages between Mar 24, 2006 and Mar 25, 2006.

replies: 6people: 3as markdown or json

Matthias Kestenholz· Mar 24, 2006, 18:39 UTC · lore
Hello list,

I don't know if this is the right place to report a bug, but I'll just try and see what comes back.

I am trying to build a Wiki [1] using PHP, a hacked version of Markdown, and git for content tracking. I use the git core plumbing to do the history work.

The PHP script created directories under .git/objects which were only writable by www-data. There were other directories which were owned by user mk and group www-data, and they were group writable.

So, I had write access to only a part of the .git directory.

When I tried to commit, I got a message saying "Unable to write sha1 filename".

The result was, that only part of the commit was recorded and that I experienced repository corruption. refs/heads/master pointed to a non-existant object.

The expected behavior would have been an error message telling me I had insufficient write privileges and surely no repository corruption.

Thanks, Matthias

[1]: http://spinlock.ch/cgi-bin/gitweb.pl?p=swisdk2/bugs.git;a=tree (See Wiki_ctrl.php for source)

-- 
:wq
Andreas Ericsson· Mar 24, 2006, 19:34 UTC · re: Matthias Kestenholz · lore

Re: Bug encountered while comitting

Matthias Kestenholz wrote:
Show 10 quoted lines
> Hello list,
> 
> I don't know if this is the right place to report a bug, but I'll
> just try and see what comes back.
> 
> I am trying to build a Wiki [1] using PHP, a hacked version of Markdown,
> and git for content tracking. I use the git core plumbing to do the
> history work.
> 
> The PHP script created directories under .git/objects

Ouch... You're not really supposed to do that. The proper thing to do is to do things in the working tree and commit them to git later.

Show 6 quoted lines
> which were
> only writable by www-data. There were other directories which were
> owned by user mk and group www-data, and they were group writable.
> 
> So, I had write access to only a part of the .git directory.
>

Unless you're using the git tools (or things hooking in to the git core C functions somehow), don't touch the .git directory.

(this merits an exclamation marks, so brace yourselves) !
> When I tried to commit, I got a message saying "Unable to write sha1
> filename".
> 
What file were you trying to write?
> The result was, that only part of the commit was recorded and that I
> experienced repository corruption. refs/heads/master pointed to a
> non-existant object.
> 
Did you use git tools to update .git/refs/heads/master ?
> The expected behavior would have been an error message telling me I
> had insufficient write privileges and surely no repository
> corruption.
>

Didn't you get the strerror(3) message from that? If so, I'd consider it a bug.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Matthias Kestenholz· Mar 25, 2006, 01:15 UTC · re: Andreas Ericsson · lore

Re: Bug encountered while comitting

* Andreas Ericsson (ae@op5.se) wrote:
Show 8 quoted lines
> Matthias Kestenholz wrote:
> >
> >The PHP script created directories under .git/objects
> 
> 
> Ouch... You're not really supposed to do that. The proper thing to do is 
> to do things in the working tree and commit them to git later.
>

I think I did not express myself clearly. The PHP Script executes git commands which in turn create or modify the mentioned directories. I do not create them myself by hand.

Show 6 quoted lines
> >When I tried to commit, I got a message saying "Unable to write sha1
> >filename".
> >
> 
> What file were you trying to write?
>

Some file I was updating (in this case, a file holding some wiki content)

Show 7 quoted lines
> >The result was, that only part of the commit was recorded and that I
> >experienced repository corruption. refs/heads/master pointed to a
> >non-existant object.
> >
> 
> Did you use git tools to update .git/refs/heads/master ?
> 
Yes.
Show 8 quoted lines
> 
> >The expected behavior would have been an error message telling me I
> >had insufficient write privileges and surely no repository
> >corruption.
> >
> 
> Didn't you get the strerror(3) message from that? If so, I'd consider it 
> a bug.

As I (only now) wrote in the other email, I got the "Permission denied" message, and that was a sufficient hint what was wrong.

-- 
:wq
Junio C Hamano· Mar 24, 2006, 22:55 UTC · re: Matthias Kestenholz · lore

Re: Bug encountered while comitting

Matthias Kestenholz <lists@irregular.ch> writes:
Show 5 quoted lines
> The PHP script created directories under .git/objects which were
> only writable by www-data. There were other directories which were
> owned by user mk and group www-data, and they were group writable.
>
> So, I had write access to only a part of the .git directory.
core.sharedrepository perhaps?

While it probably is not a good idea to have you in www-data, it appears that is essentially you will end up doing, because PHP scripts that may _create_ new directories better not to have privilege to give newly created directories away to you (busting your quota), so they will be owned by www-data.www-data and for you to be able to write into it you either need to be www-data user or in www-data group, with core.sharedrepostiory set.

Matthias Kestenholz· Mar 25, 2006, 01:15 UTC · re: Junio C Hamano · lore

Re: Bug encountered while comitting

* Junio C Hamano (junkio@cox.net) wrote:
Show 18 quoted lines
> Matthias Kestenholz <lists@irregular.ch> writes:
> 
> > The PHP script created directories under .git/objects which were
> > only writable by www-data. There were other directories which were
> > owned by user mk and group www-data, and they were group writable.
> >
> > So, I had write access to only a part of the .git directory.
> 
> core.sharedrepository perhaps?
> 
> While it probably is not a good idea to have you in www-data, it
> appears that is essentially you will end up doing, because PHP
> scripts that may _create_ new directories better not to have
> privilege to give newly created directories away to you (busting
> your quota), so they will be owned by www-data.www-data and for
> you to be able to write into it you either need to be www-data
> user or in www-data group, with core.sharedrepostiory set.
>

Thanks for your answer; I did not know about this option (I should probably re-read all the docs).

Anyway, I think git should never corrupt a repository, even if it does not have the write permissions it needs.

The following commands were sufficient to create a corrupt repository with git (v1.2.4-1, debian package):

$ git-init-db $ echo test > file $ git-update-index --add file ; git commit -m 'message'

repeat (f.e. 10 times): $ echo test >> file $ git-update-index file ; git commit -m 'message'

$ sudo chown root .git/objects/*

repeat the modification and commit commands until you get a message similar to the following:

unable to write sha1 filename .git/objects/90/b33..: Permission denied fatal: 90b33... is not a valid 'tree' object unable to write sha1 filename .git/objects/ba/fe4..: Permission denied error: file: failed to insert into database fatal: Unable to process file file etc...

The result of this all is: refs/heads/master might now point to a non-existant commit object. Every git command now errors out with:

fatal: bad tree object HEAD

and git-log shows no output (probably since it does not find a commit to begin with)

git-commit should abort as soon as it encounters an error and not update HEAD.

Thanks, Matthias

(Note: To find the last valid commit object, I could just scan the objects directory for the recently modified files and write the sha1 value to refs/heads/master, so I had no data loss.)

-- 
:wq
Junio C Hamano· Mar 25, 2006, 06:17 UTC · re: Matthias Kestenholz · lore

Re: Bug encountered while comitting

Matthias Kestenholz <lists@irregular.ch> writes:
Show 19 quoted lines
> $ sudo chown root .git/objects/*
>
> repeat the modification and commit commands until you get a message
> similar to the following:
>
> unable to write sha1 filename .git/objects/90/b33..: Permission denied
> fatal: 90b33... is not a valid 'tree' object
> unable to write sha1 filename .git/objects/ba/fe4..: Permission denied
> error: file: failed to insert into database
> fatal: Unable to process file file
> etc...
>
> The result of this all is: refs/heads/master might now point to a
> non-existant commit object. Every git command now errors out with:
>
> fatal: bad tree object HEAD
>
> and git-log shows no output (probably since it does not find a
> commit to begin with)

You are right. commit-tree does not seem to check if it successfully wrote the commit object. How about this?

-- >8 --
diff --git a/commit-tree.c b/commit-tree.c
index 88871b0..16c1787 100644
--- a/commit-tree.c
+++ b/commit-tree.c
@@ -125,7 +125,10 @@ int main(int argc, char **argv)
 	while (fgets(comment, sizeof(comment), stdin) != NULL)
 		add_buffer(&buffer, &size, "%s", comment);
 
-	write_sha1_file(buffer, size, "commit", commit_sha1);
-	printf("%s\n", sha1_to_hex(commit_sha1));
-	return 0;
+	if (!write_sha1_file(buffer, size, "commit", commit_sha1)) {
+		printf("%s\n", sha1_to_hex(commit_sha1));
+		return 0;
+	}
+	else
+		return 1;
 }
Matthias Kestenholz· Mar 25, 2006, 10:23 UTC · re: Junio C Hamano · lore

Re: Bug encountered while comitting

* Junio C Hamano (junkio@cox.net) wrote:
Show 22 quoted lines
> You are right.  commit-tree does not seem to check if it
> successfully wrote the commit object.  How about this?
> 
> -- >8 --
> diff --git a/commit-tree.c b/commit-tree.c
> index 88871b0..16c1787 100644
> --- a/commit-tree.c
> +++ b/commit-tree.c
> @@ -125,7 +125,10 @@ int main(int argc, char **argv)
>  	while (fgets(comment, sizeof(comment), stdin) != NULL)
>  		add_buffer(&buffer, &size, "%s", comment);
>  
> -	write_sha1_file(buffer, size, "commit", commit_sha1);
> -	printf("%s\n", sha1_to_hex(commit_sha1));
> -	return 0;
> +	if (!write_sha1_file(buffer, size, "commit", commit_sha1)) {
> +		printf("%s\n", sha1_to_hex(commit_sha1));
> +		return 0;
> +	}
> +	else
> +		return 1;
>  }

This patch fixes the problem I was encountering (git-update-ref is not executed anymore).

Thank you all for this really great tool!
-- 
:wq

← back to recent threads