git-fast-export is returning streams with source code inside

Hello,

I'm not sure if this is the right place to post bug reports - if not,
please can you let me know.

I've found (what appears to be) a problem in git-fast-export. I've
filed a bug in Launchpad, but I'm not sure if anyone from the Git
community actively monitors it so I'm reporting it here to see if it
can be fixed.

The bug can be viewed online at:
https://bugs.launchpad.net/ubuntu/+source/git/+bug/815828
and I've done my best to describe the bug in some detail. If you need
me to elaborate on the problem further, don't hesitate to let me know.

Thanks,
James

-- 

James Gregory
Senior Developer
epiGenesys - a University of Sheffield company
Enterprise Zone, Portobello, Sheffield, S1 4DP, UK
T: +44 (0)114 22 21 884 ~ F: +44 (0)114 22 24 042
www.epigenesys.co.uk ~ @epigenesys
--
This email and its attachments should be considered confidential and
are intended solely for the individual to whom the email is addressed.
If you are not the intended recipient you should take no action based
upon them, nor should you copy or show them to anyone. Any views or
opinions expressed are solely those of the author and might not
represent those of Epi Genesys Limited.
Epi Genesys Limited is registered in England and Wales with company
number 06388971 and has its registered office at 40 Leavygreave Road,
Sheffield, S3 7RD.

On Mon, Aug 01, 2011 at 12:57:58PM +0100, James Gregory wrote:

I'm not sure if this is the right place to post bug reports - if not,
please can you let me know.

This is the right place.

I've found (what appears to be) a problem in git-fast-export. I've
filed a bug in Launchpad, but I'm not sure if anyone from the Git
community actively monitors it so I'm reporting it here to see if it
can be fixed.

I don't think anybody monitors Launchpad explicitly; thanks for the
pointer.

The bug can be viewed online at:
https://bugs.launchpad.net/ubuntu/+source/git/+bug/815828
and I've done my best to describe the bug in some detail. If you need
me to elaborate on the problem further, don't hesitate to let me know.

Definitely looks like a bug. Is it possible for you to make the
offending repo available either publicly or to me in private? There's
not enough information in your report for me to reproduce the problem
here.

-Peff

Hi Peff,

Thanks for your reply. I'm afraid I won't be able to allow you access
to repo as it is for a business project and covered by NDAs, etc. If I
can do anything to help with diagnosing the problem (debug logs, more
excerpts from the stream, etc.) then that shouldn't be a problem.

James

On 4 August 2011 08:05, Jeff King [off-list ref] wrote:

On Mon, Aug 01, 2011 at 12:57:58PM +0100, James Gregory wrote:

quoted

I'm not sure if this is the right place to post bug reports - if not,
please can you let me know.

This is the right place.

quoted

I've found (what appears to be) a problem in git-fast-export. I've
filed a bug in Launchpad, but I'm not sure if anyone from the Git
community actively monitors it so I'm reporting it here to see if it
can be fixed.

I don't think anybody monitors Launchpad explicitly; thanks for the
pointer.

quoted

The bug can be viewed online at:
https://bugs.launchpad.net/ubuntu/+source/git/+bug/815828
and I've done my best to describe the bug in some detail. If you need
me to elaborate on the problem further, don't hesitate to let me know.

Definitely looks like a bug. Is it possible for you to make the
offending repo available either publicly or to me in private? There's
not enough information in your report for me to reproduce the problem
here.

-Peff

-- 

James Gregory
Senior Developer
epiGenesys - a University of Sheffield company
Enterprise Zone, Portobello, Sheffield, S1 4DP, UK
T: +44 (0)114 22 21 884 ~ F: +44 (0)114 22 24 042
www.epigenesys.co.uk ~ @epigenesys
--
This email and its attachments should be considered confidential and
are intended solely for the individual to whom the email is addressed.
If you are not the intended recipient you should take no action based
upon them, nor should you copy or show them to anyone. Any views or
opinions expressed are solely those of the author and might not
represent those of Epi Genesys Limited.
Epi Genesys Limited is registered in England and Wales with company
number 06388971 and has its registered office at 40 Leavygreave Road,
Sheffield, S3 7RD.

On Thu, Aug 04, 2011 at 10:08:43AM +0100, James Gregory wrote:

Thanks for your reply. I'm afraid I won't be able to allow you access
to repo as it is for a business project and covered by NDAs, etc. If I
can do anything to help with diagnosing the problem (debug logs, more
excerpts from the stream, etc.) then that shouldn't be a problem.

Since it seems like blob data is making it into the stream in an odd
place, my first guess would be a memory problem. Can you try installing
valgrind (since you are on Ubuntu, there should be a package), and then
running:

  valgrind git fast-export --all >/dev/null 2>valgrind.out

and then sharing the results of valgrind.out?

-Peff

Hi Peff,

I've attached the valgrind.out file. If I'm reading the output
properly, it does look like it is suffering from a memory leak.

James

On 4 August 2011 19:32, Jeff King [off-list ref] wrote:

On Thu, Aug 04, 2011 at 10:08:43AM +0100, James Gregory wrote:

quoted

Thanks for your reply. I'm afraid I won't be able to allow you access
to repo as it is for a business project and covered by NDAs, etc. If I
can do anything to help with diagnosing the problem (debug logs, more
excerpts from the stream, etc.) then that shouldn't be a problem.

Since it seems like blob data is making it into the stream in an odd
place, my first guess would be a memory problem. Can you try installing
valgrind (since you are on Ubuntu, there should be a package), and then
running:

 valgrind git fast-export --all >/dev/null 2>valgrind.out

and then sharing the results of valgrind.out?

-Peff

-- 

James Gregory
Senior Developer
epiGenesys - a University of Sheffield company
Enterprise Zone, Portobello, Sheffield, S1 4DP, UK
T: +44 (0)114 22 21884 ~ F: +44 (0)114 22 24042
www.epigenesys.co.uk ~ @epigenesys
--
This email and its attachments should be considered confidential and
are intended solely for the individual to whom the email is addressed.
If you are not the intended recipient you should take no action based
upon them, nor should you copy or show them to anyone. Any views or
opinions expressed are solely those of the author and might not
represent those of Epi Genesys Limited.
Epi Genesys Limited is registered in England and Wales with company
number 06388971 and has its registered office at 40 Leavygreave Road,
Sheffield, S3 7RD.

On Fri, Aug 05, 2011 at 10:24:07AM +0100, James Gregory wrote:

I've attached the valgrind.out file. If I'm reading the output
properly, it does look like it is suffering from a memory leak.

It looks pretty innocuous:

==3267== Conditional jump or move depends on uninitialised value(s)
==3267==    at 0x4E39510: inflateReset2 (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
==3267==    by 0x4E39605: inflateInit2_ (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
==3267==    by 0x4D5B02: ??? (in /usr/bin/git)
==3267==    by 0x4BB059: ??? (in /usr/bin/git)
==3267==    by 0x4BC464: ??? (in /usr/bin/git)
==3267==    by 0x4BCA29: ??? (in /usr/bin/git)
==3267==    by 0x4BCAFD: ??? (in /usr/bin/git)
==3267==    by 0x4BD161: ??? (in /usr/bin/git)
==3267==    by 0x49AEAC: ??? (in /usr/bin/git)
==3267==    by 0x4B38AA: ??? (in /usr/bin/git)
==3267==    by 0x4B422A: ??? (in /usr/bin/git)
==3267==    by 0x4A9CD2: ??? (in /usr/bin/git)

This is a well-known false positive caused by zlib, and is nothing to
worry about.

==3267== LEAK SUMMARY:
==3267==    definitely lost: 48,016 bytes in 3 blocks
==3267==    indirectly lost: 30,226 bytes in 1,889 blocks

These are actual leaks, but minor.

==3267==      possibly lost: 0 bytes in 0 blocks
==3267==    still reachable: 2,743,134 bytes in 4,334 blocks

These are not really leaks, but rather things we don't bother cleaning
up since we're about to exit and let the OS reclaim memory (e.g., all of
the commit objects).

So yes, there's leaking, but it's not much. And more importantly, I was
looking not for leaks, but for memory access errors (of which there are
none, except for the zlib false positive).

So I'm not sure where to go from here. I can't reproduce the problem
locally. Is there anything else you can tell us about the problem? Does
it always happen on the same commit? If you export just that commit,
does the problem happen? Is there anything noteworthy in the contents of
that commit?

-Peff

Hi Peff,

Thanks for the feedback. I've just looked at the commit on gitweb, and
this could be the problem!

---
fixed dodgy filename
spec/blueprints/sjt_blueprint.rb	[moved from
spec/blueprints/sjt_blueprint.rb\n lead_in\n scenario\n answers {
Sham.answers_object }\n justification_selected\n
justification_unselected\n mark_scheme {
Sham.single_mark_scheme_object }\nend\nmcq_blueprint.rb with 100%
similarity]
---

I'm guessing that is where the problem lies... somehow the git
transaction has got corrupt(?)

James

On 5 August 2011 10:36, Jeff King [off-list ref] wrote:

On Fri, Aug 05, 2011 at 10:24:07AM +0100, James Gregory wrote:

quoted

I've attached the valgrind.out file. If I'm reading the output
properly, it does look like it is suffering from a memory leak.

It looks pretty innocuous:

quoted

==3267== Conditional jump or move depends on uninitialised value(s)
==3267==    at 0x4E39510: inflateReset2 (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
==3267==    by 0x4E39605: inflateInit2_ (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
==3267==    by 0x4D5B02: ??? (in /usr/bin/git)
==3267==    by 0x4BB059: ??? (in /usr/bin/git)
==3267==    by 0x4BC464: ??? (in /usr/bin/git)
==3267==    by 0x4BCA29: ??? (in /usr/bin/git)
==3267==    by 0x4BCAFD: ??? (in /usr/bin/git)
==3267==    by 0x4BD161: ??? (in /usr/bin/git)
==3267==    by 0x49AEAC: ??? (in /usr/bin/git)
==3267==    by 0x4B38AA: ??? (in /usr/bin/git)
==3267==    by 0x4B422A: ??? (in /usr/bin/git)
==3267==    by 0x4A9CD2: ??? (in /usr/bin/git)

This is a well-known false positive caused by zlib, and is nothing to
worry about.

quoted

==3267== LEAK SUMMARY:
==3267==    definitely lost: 48,016 bytes in 3 blocks
==3267==    indirectly lost: 30,226 bytes in 1,889 blocks

These are actual leaks, but minor.

quoted

==3267==      possibly lost: 0 bytes in 0 blocks
==3267==    still reachable: 2,743,134 bytes in 4,334 blocks

These are not really leaks, but rather things we don't bother cleaning
up since we're about to exit and let the OS reclaim memory (e.g., all of
the commit objects).

So yes, there's leaking, but it's not much. And more importantly, I was
looking not for leaks, but for memory access errors (of which there are
none, except for the zlib false positive).

So I'm not sure where to go from here. I can't reproduce the problem
locally. Is there anything else you can tell us about the problem? Does
it always happen on the same commit? If you export just that commit,
does the problem happen? Is there anything noteworthy in the contents of
that commit?

-Peff

-- 

James Gregory
Senior Developer
epiGenesys - a University of Sheffield company
Enterprise Zone, Portobello, Sheffield, S1 4DP, UK
T: +44 (0)114 22 21884 ~ F: +44 (0)114 22 24042
www.epigenesys.co.uk ~ @epigenesys
--
This email and its attachments should be considered confidential and
are intended solely for the individual to whom the email is addressed.
If you are not the intended recipient you should take no action based
upon them, nor should you copy or show them to anyone. Any views or
opinions expressed are solely those of the author and might not
represent those of Epi Genesys Limited.
Epi Genesys Limited is registered in England and Wales with company
number 06388971 and has its registered office at 40 Leavygreave Road,
Sheffield, S3 7RD.

On Fri, Aug 05, 2011 at 10:54:29AM +0100, James Gregory wrote:

Thanks for the feedback. I've just looked at the commit on gitweb, and
this could be the problem!

---
fixed dodgy filename
spec/blueprints/sjt_blueprint.rb	[moved from
spec/blueprints/sjt_blueprint.rb\n lead_in\n scenario\n answers {
Sham.answers_object }\n justification_selected\n
justification_unselected\n mark_scheme {
Sham.single_mark_scheme_object }\nend\nmcq_blueprint.rb with 100%
similarity]
---

I'm guessing that is where the problem lies... somehow the git
transaction has got corrupt(?)

Ah, OK. That makes sense. I can replicate your problem easily with:

  $ touch 'file with
    newline'
  $ git init
  $ git add .
  $ git commit -m foo
  $ git fast-export HEAD | git fast-import
  fatal: Unsupported command: newline

According to the fast-import manpage, fast-export should be quoting the
embedded line-feed. It looks like it isn't doing any quoting at all of
pathnames right now, which is just wrong.

Does the patch below fix your issue?

---

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 9247871..bd27f08 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c

@@ -16,6 +16,7 @@
 #include "string-list.h"
 #include "utf8.h"
 #include "parse-options.h"
+#include "quote.h"
 
 static const char *fast_export_usage[] = {
 	"git fast-export [rev-list-opts]",

@@ -167,6 +168,15 @@ static int depth_first(const void *a_, const void *b_)
 	return (a->status == 'R') - (b->status == 'R');
 }
 
+static void print_path(const char *path)
+{
+	int need_quote = quote_c_style(path, NULL, NULL, 0);
+	if (need_quote)
+		quote_c_style(path, NULL, stdout, 0);
+	else
+		printf("%s", path);
+}
+
 static void show_filemodify(struct diff_queue_struct *q,
 			    struct diff_options *options, void *data)
 {

@@ -184,13 +194,18 @@ static void show_filemodify(struct diff_queue_struct *q,
 
 		switch (q->queue[i]->status) {
 		case DIFF_STATUS_DELETED:
-			printf("D %s\n", spec->path);
+			printf("D ");
+			print_path(spec->path);
+			putchar('\n');
 			break;
 
 		case DIFF_STATUS_COPIED:
 		case DIFF_STATUS_RENAMED:
-			printf("%c \"%s\" \"%s\"\n", q->queue[i]->status,
-			       ospec->path, spec->path);
+			printf("%c ", q->queue[i]->status);
+			print_path(ospec->path);
+			putchar(' ');
+			print_path(spec->path);
+			putchar('\n');
 
 			if (!hashcmp(ospec->sha1, spec->sha1) &&
 			    ospec->mode == spec->mode)

@@ -205,13 +220,15 @@ static void show_filemodify(struct diff_queue_struct *q,
 			 * output the SHA-1 verbatim.
 			 */
 			if (no_data || S_ISGITLINK(spec->mode))
-				printf("M %06o %s %s\n", spec->mode,
-				       sha1_to_hex(spec->sha1), spec->path);
+				printf("M %06o %s ", spec->mode,
+				       sha1_to_hex(spec->sha1));
 			else {
 				struct object *object = lookup_object(spec->sha1);
-				printf("M %06o :%d %s\n", spec->mode,
-				       get_object_mark(object), spec->path);
+				printf("M %06o :%d ", spec->mode,
+				       get_object_mark(object));
 			}
+			print_path(spec->path);
+			putchar('\n');
 			break;
 
 		default: