Memory overrun in http-push.c

Memory overrun in http-push.c

[Eygene Ryabinkin]

[-- Attachment #1: Type: text/plain, Size: 162 bytes --]

Good day!

Spotted the memory overrun in the http-push.c. Exists at least in
1.5.0.x, not sure about latest development branch. The patch is
attached.
-- 
Eygene

[-- Attachment #2: http-push.c.patch --]
[-- Type: text/plain, Size: 376 bytes --]

--- http-push.c.orig	Wed Feb 28 15:15:01 2007
+++ http-push.c	Wed Feb 28 15:15:21 2007
@@ -1295,7 +1295,7 @@
 	sprintf(url, "%s%s", remote->url, path);
 
 	/* Make sure leading directories exist for the remote ref */
-	ep = strchr(url + strlen(remote->url) + 11, '/');
+	ep = strchr(url + strlen(remote->url) + 1, '/');
 	while (ep) {
 		*ep = 0;
 		slot = get_active_slot();

Re: Memory overrun in http-push.c

[Andy Parkins]

On Wednesday 2007 February 28 15:15, Eygene Ryabinkin wrote:
> --- http-push.c.orig    Wed Feb 28 15:15:01 2007
> +++ http-push.c Wed Feb 28 15:15:21 2007
> @@ -1295,7 +1295,7 @@

A patch for git that wasn't made with git.

Don't take this the wrong way Eygene, but why?



Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

Re: Memory overrun in http-push.c

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 369 bytes --]

Hi,

On Wed, 28 Feb 2007, Andy Parkins wrote:

> On Wednesday 2007 February 28 15:15, Eygene Ryabinkin wrote:
> > --- http-push.c.orig    Wed Feb 28 15:15:01 2007
> > +++ http-push.c Wed Feb 28 15:15:21 2007
> > @@ -1295,7 +1295,7 @@
> 
> A patch for git that wasn't made with git.
> 
> Don't take this the wrong way Eygene, but why?

Old habits die hard?

Ciao,
Dscho

Re: Memory overrun in http-push.c

[Eygene Ryabinkin]

Andy, good day!

Wed, Feb 28, 2007 at 03:41:38PM +0000, Andy Parkins wrote:
> On Wednesday 2007 February 28 15:15, Eygene Ryabinkin wrote:
> > --- http-push.c.orig    Wed Feb 28 15:15:01 2007
> > +++ http-push.c Wed Feb 28 15:15:21 2007
> > @@ -1295,7 +1295,7 @@
> 
> A patch for git that wasn't made with git.
> 
> Don't take this the wrong way Eygene, but why?

Because I am trying to set up Git repository to be used over HTTPS,
but it is failing now: seems like CURL library provokes the SIGSEGV.
The actual patch was made for the FreeBSD ports, so I've used plain
old 'diff -u'.

As Johannes pointed out, old habits are still alive. And when some
tool serves me the right way, I am happy with it. To make the patch
with Git I should create repository, hack there, merge it to the
FreeBSD port, install and so on. A bit boring, do not take me wrong ;))
-- 
Eygene

Re: Memory overrun in http-push.c

[Alex Riesen]

On 3/1/07, Eygene Ryabinkin <rea-git@codelabs.ru> wrote:
> As Johannes pointed out, old habits are still alive. And when some
> tool serves me the right way, I am happy with it. To make the patch
> with Git I should create repository, hack there, merge it to the

hacking on Git you already have a git repo, don't you?
It should have been as simple as:

$ vi http-push.c
...
$ git diff

:)

BTW, git diff works outside of a git repositories (well, very recent
git should).

Re: Memory overrun in http-push.c

[Eygene Ryabinkin]

> >As Johannes pointed out, old habits are still alive. And when some
> >tool serves me the right way, I am happy with it. To make the patch
> >with Git I should create repository, hack there, merge it to the
> 
> hacking on Git you already have a git repo, don't you?

No, I was doing FreeBSD port of Git 1.5.0, so it was just the tarball.
Old habits, you know ;))

> It should have been as simple as:
> 
> $ vi http-push.c
> ...
> $ git diff
> 
> :)
> 
> BTW, git diff works outside of a git repositories (well, very recent
> git should).

Once I will get Git working for me I will get used to it and will
do many things in Git.

OK, I've forgot to ask in my previous letter: will the issue with
'+11' instead of '+1' be fixed (or its already fixed)? Or 11 is 10 + 1,
where 10 it strlen("git") trice plus one just to be sure? ;)))
-- 
Eygene

Re: Memory overrun in http-push.c

[Alex Riesen]

On 3/1/07, Eygene Ryabinkin <rea-git@codelabs.ru> wrote:
> OK, I've forgot to ask in my previous letter: will the issue with
> '+11' instead of '+1' be fixed (or its already fixed)? Or 11 is 10 + 1,
> where 10 it strlen("git") trice plus one just to be sure? ;)))

Your patch is already commited.

http://www.kernel.org/git/?p=git/git.git;a=commit;h=2c46759db757eb742590e8547cb0c63e8bdb1da1

Re: Memory overrun in http-push.c

[Eygene Ryabinkin]

Thu, Mar 01, 2007 at 10:21:47AM +0100, Alex Riesen wrote:
> On 3/1/07, Eygene Ryabinkin <rea-git@codelabs.ru> wrote:
> >OK, I've forgot to ask in my previous letter: will the issue with
> >'+11' instead of '+1' be fixed (or its already fixed)? Or 11 is 10 + 1,
> >where 10 it strlen("git") trice plus one just to be sure? ;)))
> 
> Your patch is already commited.

Thank you!
-- 
Eygene

Re: Memory overrun in http-push.c

[Junio C Hamano]

"Alex Riesen" <raa.lkml@gmail.com> writes:

> On 3/1/07, Eygene Ryabinkin <rea-git@codelabs.ru> wrote:
>> As Johannes pointed out, old habits are still alive. And when some
>> tool serves me the right way, I am happy with it. To make the patch
>> with Git I should create repository, hack there, merge it to the
>
> hacking on Git you already have a git repo, don't you?

You do not have to insist on using git to hack on git itself.  I
was more annoyed by the fact that the patch was an attachment,
than it was generated by diff without leading a/ and b/ paths.

Being able to accept a patch that was made against freshly
extracted tarball distribution is a good thing.  I can take
diffs generated by GNU diff against released tarballs just
fine.

If you make it a requirement to send even an trivially obvious
patch from controlled source, it would make the barrier to
contribute much higher.  Once you make it hard enough to submit
patches, you will end up seeing only whining without patches on
this list.  I do not think we want to go in that direction.

Re: Memory overrun in http-push.c

[Alex Riesen]

On 3/1/07, Junio C Hamano <junkio@cox.net> wrote:
> >> As Johannes pointed out, old habits are still alive. And when some
> >> tool serves me the right way, I am happy with it. To make the patch
> >> with Git I should create repository, hack there, merge it to the
> >
> > hacking on Git you already have a git repo, don't you?
>
> You do not have to insist on using git to hack on git itself.

that wasn't it. It just haven't occurred to me at all, that someone
could hack on git without using it. I feel very uncomfortable
changing anything not under version control (specifically - not
in git repository ;) Just haven't though of tarballs.

Re: Memory overrun in http-push.c

[Andy Parkins]

On Thursday 2007 March 01 09:32, Junio C Hamano wrote:

> If you make it a requirement to send even an trivially obvious

I certainly wasn't suggesting that it should be a requirement.  I just thought 
there was a certain irony.  Perhaps even a lesson to be learned - find out 
why someone has chosen (even with git in front of them and deep enough inside 
to find bugs in it), not to use git.

I think there is a usability lesson to be pulled from this situation.  Why 
didn't Eygene just say:

 $ tar zxvf git-tarball.tgz
 $ cd git
 $ git init
 $ vim http-push.c
 $ git diff

Note, that was not an attack on Eygene - I genuinely am interested to know 
where we should put the documentation that tells a new user what they need to 
know quickly.

Somehow, git has failed this person.  I was simply wondering in what way.

However, Eygene's response made it sound like a situational thing - it was 
inconvenient to actually install git.  Fair enough.


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

Re: Memory overrun in http-push.c

[Eygene Ryabinkin]

Good day!

> was more annoyed by the fact that the patch was an attachment,
> than it was generated by diff without leading a/ and b/ paths.

So, git is unable to use this type of patches directly and I will
need to do
$ patch < patchfile
$ rm file.orig
$ git commit

Just wondering what is bad with the diffs without leading a/ and b/.

> I certainly wasn't suggesting that it should be a requirement.  I just thought
> there was a certain irony.  Perhaps even a lesson to be learned - find out 
> why someone has chosen (even with git in front of them and deep enough inside 
> to find bugs in it), not to use git.

I've just realized that I need git because I need disconnected
commits. That's why I left my beloved SVN and started to hack GIT ;))

By the way, I am missing one thing: the Id keyword in the file. The
problem is that when some user is telling me: there is a bug in the
function a() that is inside the file b.c, then I can ask him to
give me the $Id$ tag of the file and I will have the full information
about the file version. Can I have it with git if user has just the
sources without any signs of the file versions? I've glanced over
the git manual, but was unable to find anything usable.

> I think there is a usability lesson to be pulled from this situation.  Why 
> didn't Eygene just say:
> 
>  $ tar zxvf git-tarball.tgz
>  $ cd git
>  $ git init
>  $ vim http-push.c
>  $ git diff
> 
> Note, that was not an attack on Eygene - I genuinely am interested to know 
> where we should put the documentation that tells a new user what they need to 
> know quickly.
> 
> Somehow, git has failed this person.  I was simply wondering in what way.
> 
> However, Eygene's response made it sound like a situational thing - it was 
> inconvenient to actually install git.  Fair enough.

Not incovinient to install git, but it was just convinient to me to
use FreeBSD port. The other way round ;))
-- 
Eygene

Re: Memory overrun in http-push.c

[Junio C Hamano]

Eygene Ryabinkin <rea-git@codelabs.ru> writes:

> Good day!
>
>> was more annoyed by the fact that the patch was an attachment,
>> than it was generated by diff without leading a/ and b/ paths.
>
> So, git is unable to use this type of patches directly and I will
> need to do
> $ patch < patchfile
> $ rm file.orig
> $ git commit

In case it was unclear, I was saying that "attachment was more
annoying" than "diff output that does not follow the established
convention to be applied with patch -p1".  Both I can handle,
but dealing with the former is more work for me.

Re: Memory overrun in http-push.c

[Eygene Ryabinkin]

> In case it was unclear, I was saying that "attachment was more
> annoying" than "diff output that does not follow the established
> convention to be applied with patch -p1".  Both I can handle,
> but dealing with the former is more work for me.

Ah, sorry, got it.
-- 
Eygene

Re: Memory overrun in http-push.c

[Johannes Schindelin]

Hi,

On Thu, 1 Mar 2007, Eygene Ryabinkin wrote:

> By the way, I am missing one thing: the Id keyword in the file. The 
> problem is that when some user is telling me: there is a bug in the 
> function a() that is inside the file b.c, then I can ask him to give me 
> the $Id$ tag of the file and I will have the full information about the 
> file version.

The problem is: Git does not track files, but rather trees. So, there is 
no sane way to add an Id.

But all is not lost! If your colleague sends you the offending file, you 
can calculate the object name from it by

	$ git-hash-object -t blob <filename>

If the file is too big, your colleague can run it herself and send you the 
object name (it is a 40 character hex string, but the first 8 or so should 
really be sufficient).

Then you can run

	$ git log -p HEAD <filename>

in your repository, and search for "index <object name>". You will find 
the commit which changed the file _from_ the state your colleague is 
seeing.

The idea is that the diff shows not only the names, but also a line "index 
<abbrev1>..<abbrev2>", where the abbrev's are the abbreviated object names 
of the old and new versions of that file.

BTW we had a similar discussion a while ago, which made me even write a 
patch for git-name-rev to name objects, too, but it was rejected as the 
workflow I outlined above is already sufficient for your use case.

Hth,
Dscho

Re: Memory overrun in http-push.c

[Andy Parkins]

On Thursday 2007, March 01, Johannes Schindelin wrote:

> The problem is: Git does not track files, but rather trees. So, there
> is no sane way to add an Id.

> If the file is too big, your colleague can run it herself and send
> you the object name (it is a 40 character hex string, but the first 8
> or so should really be sufficient).

So when you say "there is no sane way to add an Id", you meant "here is 
a perfect string that would act as an Id"?  ;-)



Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

Re: Memory overrun in http-push.c

[Johannes Schindelin]

Hi,

On Thu, 1 Mar 2007, Andy Parkins wrote:

> On Thursday 2007, March 01, Johannes Schindelin wrote:
> 
> > The problem is: Git does not track files, but rather trees. So, there
> > is no sane way to add an Id.
> 
> > If the file is too big, your colleague can run it herself and send
> > you the object name (it is a 40 character hex string, but the first 8
> > or so should really be sufficient).
> 
> So when you say "there is no sane way to add an Id", you meant "here is 
> a perfect string that would act as an Id"?  ;-)

Got it.

So, in a very real sense, we have file ids. Even tree ids. And commit ids.

Ciao,
Dscho

Re: Memory overrun in http-push.c

[Andy Parkins]

On Thursday 2007, March 01, Johannes Schindelin wrote:

> Got it.
>
> So, in a very real sense, we have file ids. Even tree ids. And commit
> ids.

Absolutely.  Moreso than CVS/SVN.  Putting $Id$ $Rev$ in a git managed 
file would have far more meaning that it does in a CVS managed file.

Actually, $Id$ covers the file hash; $Rev$ covers the commit hash.  I 
wonder if the tree id would be useful to finish the set?

The reason it's so hard to do is because putting the file id in the file 
would change the file, and hence its Id.  Ouch.  However, I reckon the 
upcoming gitattributes stuff is going to have no trouble keeping the 
file contents stable from git's point of view and yet let you 
mangle/unmangle the contents with this keyword stuff.

Actually once gitattributes exists we almost have to have a $Id$, the 
hash of the working directory file isn't stable because it might have 
had it's line endings mangled.  On the other hand we could argue that 
git-hash-object should have unmangling options anyway.



Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

Re: Memory overrun in http-push.c

[Johannes Schindelin]

Hi,

On Thu, 1 Mar 2007, Andy Parkins wrote:

> Putting $Id$ $Rev$ in a git managed file would have far more meaning 
> that it does in a CVS managed file.

No. My point was that you do not even have to have an id. The hash of the 
object is the id.

This is obviously much better than the mess of CVS/SVN's file ids. There 
is an option, even, to switch off key expansion, so you can have erroneous 
ids. That just cannot happen with hashes.

Of course, it does not give you any hint about when this file was current. 
But there is no way to tell in distributed development _anyway_. You have 
to look it up, when, and who, changed the file to the current state.

Ciao,
Dscho

Re: Memory overrun in http-push.c

[Andy Parkins]

On Thursday 2007 March 01 20:43, Johannes Schindelin wrote:

> > Putting $Id$ $Rev$ in a git managed file would have far more meaning
> > that it does in a CVS managed file.
>
> No. My point was that you do not even have to have an id. The hash of the
> object is the id.

I think you're thinking of these tags in too strict terms.  You're right they 
aren't /needed/ you can query the version control system for the information.  
However, if you sent them out in a tarball to someone who didn't have or want 
to have your version control system, then it is much easier for them to be 
able to tell you the $Id$ field from the file itself than to generate the 
hash or send you a copy of the file.

They're purely for information.  As I've mentioned before, personally the only 
place I use them is in inkscape SVG files (thank the lord SVG's are ASCII), 
where I add a text field that appears on the diagram itself and I write $Id$ 
as the text.  Then whenever I print that diagram out (which makes it much 
harder to hash), I will know which version of the file I'm looking at.  This 
is the /only/ thing I miss from subversion.

> This is obviously much better than the mess of CVS/SVN's file ids. There
> is an option, even, to switch off key expansion, so you can have erroneous
> ids. That just cannot happen with hashes.

As I said, I don't think anyone ever imagines that these Id fields are 
absolute guarantees of correctness - nor does any version control 
system /ever/ read them and use them.  They are there only as a convenience 
for the user.  Obviously, I could just edit the $Id$ anyway if I wanted, so 
it obviously isn't absolute.

> Of course, it does not give you any hint about when this file was current.
> But there is no way to tell in distributed development _anyway_. You have
> to look it up, when, and who, changed the file to the current state.

What about this though:
 * Tag a release
 * Export the working tree into a fresh directory
 * Edit each source file to put the hash of the tagged revision into
   every file.
 * Edit the makefile to include the tag hash in the released version
 * tar it up
 * Release
It's such a mundane, useless waste of time to edit the hash in by hand - why 
can't the version control system do it?

It's already nearly there in the form of git-describe.  It's only purpose is 
to supply strings that describe the current revision.  Well, so would $Id$, 
but for files instead.

I don't see that keywords are the evil they're made out to be; they're for 
convenience, not for absolute truth.  When some random printout of the source 
turns up, at least the $Id$ might give you a clue as to which version it is.  
If it doesn't, well you're no worse off.

Here's another similar idea: generating copyright lines.  Let's say we want to 
copyright every source file - that means writing "(C) Junio, (C) Johannes, 
etc" at the top of every file.  Wouldn't it be nicer if we could put 
$Copyright$ in the file, then have some git-blame-like machinery fill in the 
copyrights automatically based on who's made contributions?

e.g.

 git-blame refs.c | sort -f 2 | uniq -c -s 10 -w 10 | sort -nr



Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

Re: Memory overrun in http-push.c

[Jakub Narebski]

Andy Parkins wrote:


> What about this though:
>  * Tag a release
>  * Export the working tree into a fresh directory
>  * Edit each source file to put the hash of the tagged revision into
>    every file.
>  * Edit the makefile to include the tag hash in the released version
>  * tar it up
>  * Release
> It's such a mundane, useless waste of time to edit the hash in by hand - why 
> can't the version control system do it?

What about this (that is roughtly what git and Linux kernel use):
 * Tag a release
 * Issue "make dist" which would automatically replace @@STH_VERSION@@
   (or ++STH_VERSION++ for example in Perl files) with result of
   "git describe" (although I think that plain old "make" also does this)
   _and_ create proper STH_VERSION file
 * tar it up using "git archive --format=tar" which would add version
   as a tar comment
 * Release
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: Memory overrun in http-push.c

[Andy Parkins]

On Friday 2007 March 02 14:46, Jakub Narebski wrote:

> What about this (that is roughtly what git and Linux kernel use):
>  * Tag a release
>  * Issue "make dist" which would automatically replace @@STH_VERSION@@
>    (or ++STH_VERSION++ for example in Perl files) with result of
>    "git describe" (although I think that plain old "make" also does this)
>    _and_ create proper STH_VERSION file
>  * tar it up using "git archive --format=tar" which would add version
>    as a tar comment
>  * Release

Yep; you've improved the workflow for my example.  However, it's not quite the 
same because in git the tokens in the ".perl" files are substituted into 
files without the suffix.  This means that those files aren't the real source 
files.  You can't put that token in the source files themselves because that 
would change their content and would then show up to git as changes.

Therefore git itself has to be involved with the keyword substitution, because 
you want it to be blind to that substitution when it's checking for changes.

Of course that's all predicated on you actually wanting keywords.  For people 
that don't want them, I can see why the point is moot.


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

Re: Memory overrun in http-push.c

[Johannes Schindelin]

Hi,

On Fri, 2 Mar 2007, Andy Parkins wrote:

> Here's another similar idea: generating copyright lines.  Let's say we 
> want to copyright every source file - that means writing "(C) Junio, (C) 
> Johannes, etc" at the top of every file.  Wouldn't it be nicer if we 
> could put $Copyright$ in the file, then have some git-blame-like 
> machinery fill in the copyrights automatically based on who's made 
> contributions?

That's a horrible idea. A typo fix is not copyrightable. You'd add these. 
An obious fix is not copyrightable. You'd add these cases, too.

Besides, IMHO adding copyright should be a _concious_ act.

Ciao,
Dscho

Re: Memory overrun in http-push.c

[Andy Parkins]

On Friday 2007, March 02, Johannes Schindelin wrote:

> On Fri, 2 Mar 2007, Andy Parkins wrote:
> > Here's another similar idea: generating copyright lines.  Let's say
> > we want to copyright every source file - that means writing "(C)
> > Junio, (C) Johannes, etc" at the top of every file.  Wouldn't it be
> > nicer if we could put $Copyright$ in the file, then have some
> > git-blame-like machinery fill in the copyrights automatically based
> > on who's made contributions?
>
> That's a horrible idea. A typo fix is not copyrightable. You'd add
> these. An obious fix is not copyrightable. You'd add these cases,
> too.

Well, I wasn't actually suggesting that as a final solution, it might be 
that it's done by more than just line count, and that some clever bit 
of machinery measures real contribution.  That wasn't really the point.

> Besides, IMHO adding copyright should be a _concious_ act.

Well, IMHO, the concious act was the commital of a change.

A lot of your objection to this seems to be "IMHO".  Fine: you don't 
like keywords, you wouldn't be /forced/ to use them.  I can see a use 
for them.  I'm having a lot of trouble understanding why they are seen 
as inherently evil.  Difficult to implement, I would accept, 
impractical to make work in git would be fine; but not this hand wavery 
that there is no conceivable use for them.


Andy

-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

Re: Memory overrun in http-push.c

[Daniel Barkalow]

On Fri, 2 Mar 2007, Andy Parkins wrote:

> What about this though:
>  * Tag a release
>  * Export the working tree into a fresh directory
>  * Edit each source file to put the hash of the tagged revision into
>    every file.
>  * Edit the makefile to include the tag hash in the released version
>  * tar it up
>  * Release
> It's such a mundane, useless waste of time to edit the hash in by hand - why 
> can't the version control system do it?

Actually, how about doing keyword expansion just like the new CRLF
handling? Have the text filter replace $Id...$ with $Id$ going into the 
database, and $Id$ with $Id <info>$ coming out of the database. The info 
could even include the commit (although that might be confusing for files 
that you haven't changed since you checked them out, and when you checked 
them out, they weren't changed in that commit; the commit would be a sort 
of arbitrary choice among commits that have that file).

It seems to me like keywords fit the pattern we now have (with 
CRLF-conversion) of data which isn't tracked, but is present in working 
directory files. In the CRLF case, it's a constant character (CR), but 
there's no reason we couldn't have the non-canonical form depend on more 
information. (And we'd beat CVS/SVN for accuracy: if keyword substitution 
isn't on, nothing appears in $Id$ tags, rather than being left with 
something inaccurate).

	-Daniel
*This .sig left intentionally blank*

Re: Memory overrun in http-push.c

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> Actually, how about doing keyword expansion just like the new CRLF
> handling?

That part is true, but practically speaking the only thing you
can honestly replace is $Id$ with blob object name, and nothing
else.  Trying to put anything that is available via commit would
at best telling a lie and at worst ending up denying the index
(and denying the index is denying git as somebody said).  You
could fake the values for $Author$ and $Date$ from HEAD commit,
but your index may or may not match HEAD commit when you check
out path from it.

Re: Memory overrun in http-push.c

[Daniel Barkalow]

On Sun, 4 Mar 2007, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > Actually, how about doing keyword expansion just like the new CRLF
> > handling?
> 
> That part is true, but practically speaking the only thing you
> can honestly replace is $Id$ with blob object name, and nothing
> else.  Trying to put anything that is available via commit would
> at best telling a lie and at worst ending up denying the index
> (and denying the index is denying git as somebody said).  You
> could fake the values for $Author$ and $Date$ from HEAD commit,
> but your index may or may not match HEAD commit when you check
> out path from it.

I was somehow expecting the index to be on the "working directory" side of 
conversions, but that obviously doesn't work very well (since you'd get a 
ton of unreachable blobs all the time that way). Even so, I think the 
useful thing would the the "git-describe" for HEAD, if the index entry is 
actually from HEAD (otherwise, it's a state that nobody else has seen, 
right? No point in trying to specify further...). I don't think Author and 
Date from a commit really works, because they wouldn't actually reflect 
the last change to anything near the tag, necessarily. The commit hash, on 
the other hand, tends you what to run blame on.

But, in any case, $Id <blob hash>$ would put into the working directory 
file (and suitable installed derivates thereof) something that would lead 
people with the repository to the actual blob, with at least a possible 
context. $Id <commit hash>:<path>$, for <commit hash>=HEAD, if HEAD:<path> 
is <blob hash>, should be just right.

Is it common to check out files from the index which don't match HEAD? The 
only case I can think of is resetting to a state stored in the index with 
update-index. I think my personal workflow is always HEAD->index->working 
directory->index->new HEAD.

	-Daniel
*This .sig left intentionally blank*

Re: Memory overrun in http-push.c

[Alex Riesen]

On 3/1/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> > By the way, I am missing one thing: the Id keyword in the file. The
> > problem is that when some user is telling me: there is a bug in the
> > function a() that is inside the file b.c, then I can ask him to give me
> > the $Id$ tag of the file and I will have the full information about the
> > file version.
>
> The problem is: Git does not track files, but rather trees. So, there is
> no sane way to add an Id.
>
> But all is not lost! If your colleague sends you the offending file, you
> can calculate the object name from it by
>
>         $ git-hash-object -t blob <filename>
>

Wont work. The ids in question are really strings embedded into object.
Something like:

  $Id 1.2 user filename.c$

They can be found, extracted from the binary and sent to developer
with a bug report. It's more of a build signature.
IOW, the colleague in question has to have the source, which,
I suspect, he wont.

Of course, it's very simple to embed the HEAD into build and
tell the tester to send the SHA-1 in case of bug report.

Re: Memory overrun in http-push.c

[Shawn O. Pearce]

Alex Riesen <raa.lkml@gmail.com> wrote:
> Wont work. The ids in question are really strings embedded into object.
> Something like:
> 
>  $Id 1.2 user filename.c$
> 
> They can be found, extracted from the binary and sent to developer
> with a bug report. It's more of a build signature.
> IOW, the colleague in question has to have the source, which,
> I suspect, he wont.
> 
> Of course, it's very simple to embed the HEAD into build and
> tell the tester to send the SHA-1 in case of bug report.

Yes.  Either the commit SHA-1 or the output of git-describe does
the trick.  Even better when your program prints this value on
demand, like say `git version`.  ;-)

Before programmers got smart and wrote switches to dump the
version (aka -v/-V/--version) users were expected to use
`strings frob | grep '$Id' | mail baddev@example.com` to
send in bug reports.  What a world we live in!  ;-)

-- 
Shawn.

Re: Memory overrun in http-push.c

[Uwe Kleine-König]

Hello Eygene,

> By the way, I am missing one thing: the Id keyword in the file. The
> problem is that when some user is telling me: there is a bug in the
> function a() that is inside the file b.c, then I can ask him to
> give me the $Id$ tag of the file and I will have the full information
> about the file version. Can I have it with git if user has just the
> sources without any signs of the file versions? I've glanced over
> the git manual, but was unable to find anything usable.
Git and the linux kernel (can) have the commit hash in their version.

For git you can see it with

	$ git version
	git version 1.5.0.2.260.g2eb065

for the linux kernel you have to enable CONFIG_LOCALVERSION_AUTO, then
you get:

	$ uname -r
	2.6.20-g509cb37e

Knowing the commit you can determine the corresponding "version" of b.c.

There are some other possiblities to communicate that hash.  E.g.
a tar archive created with
	
	$ git-archive --format=tar ...

allows you do to

	$ git-get-tar-commit-id < $tarfile
	2eb06531e3d53c2604f20c32e5cb791d5044ff02

Best regards
Uwe

-- 
Uwe Kleine-König

exit vi, lesson IV:
Z Z

NB: may write current file

Re: Memory overrun in http-push.c

[Jakub Narebski]

Eygene Ryabinkin wrote:

> By the way, I am missing one thing: the Id keyword in the file. The
> problem is that when some user is telling me: there is a bug in the
> function a() that is inside the file b.c, then I can ask him to
> give me the $Id$ tag of the file and I will have the full information
> about the file version. Can I have it with git if user has just the
> sources without any signs of the file versions? I've glanced over
> the git manual, but was unable to find anything usable.
 
First, keyword expansion on checkout (CVS-like) is an abomination;
the proper way is to do it on _build_ time, like git and Linux kernel
do, so "git version" (or "git --version") and "uname -a" gives us
the version which can be used with git to go to it.

Second, there were some work on git mailing list to create git command
which given a blob (or sha1 of a blob object) would list all commits
(in date order) which have this exact version of a file (of a blob).

Third, tar files generated by git-acrhive have version embedded as
a comment in them.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: Memory overrun in http-push.c

[Johannes Schindelin]

Hi,

On Fri, 2 Mar 2007, Jakub Narebski wrote:

> Second, there were some work on git mailing list to create git command 
> which given a blob (or sha1 of a blob object) would list all commits (in 
> date order) which have this exact version of a file (of a blob).

It was shot down, and rightly so.

Ciao,
Dscho

identifying blobs (was Re: Memory overrun in http-push.c)

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Fri, 2 Mar 2007, Jakub Narebski wrote:
>
>> Second, there were some work on git mailing list to create git command 
>> which given a blob (or sha1 of a blob object) would list all commits (in 
>> date order) which have this exact version of a file (of a blob).
>
> It was shot down, and rightly so.

I think this "rightly" needs a clarification (otherwise Jakub
wouldn't have said the above).

It wasn't that identifying the commit that contained the version
of the file was not needed.  It is just there already is a way
to do so, and "to create git command" turns out to be
unnecessary.  Something like:

	$ hash=`git hash-object --stdin <$file_in_question`
	$ git log -z --raw -r --abbrev=40 |
          grep -z '^:[0-7][0-7]* [0-7][0-7]* [0-9a-f][0-9a-f]* '"$hash"'

Re: identifying blobs (was Re: Memory overrun in http-push.c)

[Linus Torvalds]

On Fri, 2 Mar 2007, Junio C Hamano wrote:
> 
> It wasn't that identifying the commit that contained the version
> of the file was not needed.  It is just there already is a way
> to do so, and "to create git command" turns out to be
> unnecessary.  Something like:
> 
> 	$ hash=`git hash-object --stdin <$file_in_question`
> 	$ git log -z --raw -r --abbrev=40 |
>           grep -z '^:[0-7][0-7]* [0-7][0-7]* [0-9a-f][0-9a-f]* '"$hash"'

That's not very nice, because if the blob is already in the *top* commit, 
and isn't changed in a long long time, you'll not see it until it changes 
(which is potentially at the root commit when it was introduced).

So it's better to do something like

 - get the hash

	hash=`git hash-object --stdin <$file_in_question`

 - see if it is in the current top of the tree

	git ls-tree -r HEAD | cut -d3- | grep ^$hash

 - after that, start doing the backwards searching for things where it 
   went away if it was there before (roughly your "git log -z ...")

which should be fairly simple and straightforward.

For extra bonus points, the script could be trivially written so that it 
allows the user to specify a path limiter, so that you say "can you tell 
me which version this blob existed in, but I only want to know about 
drivers/scsi/ and include/scsi/"

		Linus

Re: Memory overrun in http-push.c

[Andy Parkins]

On Friday 2007 March 02 14:38, Jakub Narebski wrote:

> First, keyword expansion on checkout (CVS-like) is an abomination;
> the proper way is to do it on _build_ time, like git and Linux kernel

What about my SVG example?  There is no build time, the file is the file.


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

Re: Memory overrun in http-push.c

[Matthieu Moy]

Andy Parkins <andyparkins@gmail.com> writes:

> On Friday 2007 March 02 14:38, Jakub Narebski wrote:
>
>> First, keyword expansion on checkout (CVS-like) is an abomination;
>> the proper way is to do it on _build_ time, like git and Linux kernel
>
> What about my SVG example?  There is no build time, the file is the file.

Same with manually written HTML, or even just plain text files, or ...

A typical other example is when you're working with people having some
kind of alergy to revision control systems. Then, you have to send the
source, and they send you back the modified source. That's very hard
to manage without some kind of timestamp within the file.

However, I have to admit that workflows requiring keyword expansions
still appear dirty to me.

A (dirty too) solution is to have a pre-commit hook that would update
the keywords for each modified file. Then, the keyword is in the
source itself.

-- 
Matthieu

Re: Memory overrun in http-push.c

[Andy Parkins]

On Friday 2007 March 02 15:30, Matthieu Moy wrote:

> A (dirty too) solution is to have a pre-commit hook that would update
> the keywords for each modified file. Then, the keyword is in the
> source itself.

That would be bad for merges though.  I think it's got to be filtered out in a 
pre-commit or equivalent script.


Andy

-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

Re: Memory overrun in http-push.c

[Florian Weimer]

* Eygene Ryabinkin:

> Spotted the memory overrun in the http-push.c. Exists at least in
> 1.5.0.x, not sure about latest development branch. The patch is
> attached.

Is this issue security-relevant?  After all, the misplaced pointer is
dereferenced and written to.

Re: Memory overrun in http-push.c

[Eygene Ryabinkin]

Florian, good day!

> > Spotted the memory overrun in the http-push.c. Exists at least in
> > 1.5.0.x, not sure about latest development branch. The patch is
> > attached.
> 
> Is this issue security-relevant?  After all, the misplaced pointer is
> dereferenced and written to.

It can be relevant: basically, it is the heap overflow, because
'url' is allocated by xmalloc. Did not tried to exploit it, but
old sudo exploit proved that even one byte off memory dereference
can be exploited. But this particular exploit will work only for
the URLs where the 'path' is shorter than 10 bytes. And I doubt
that many people are running http-push in the set-uid mode, so
the exploit target will be the person running git-push over HTTP,
not the root user. But it does not mean that this is not an issue.

Sorry for the long letter.
-- 
Eygene