github-todos can break git push on very large commits

[stephan-nordnes-eriksen]

I recently tried to push a very large commit (thousands of lines changed). This resulted in github-todos printing so much to the stdout that the git push failed (exceeded buffer).

A very large css file (9000+lines) was printed to stdout. I suspect this was the cause of the issue, but I am not sure if it was the only problem.

If anyone else gets the same problem, you can do git push --no-verify to omit the githook.

[naholyr]

I guess it was an inlined css file ? I'll try to reproduce, best if you can give me any more info like OS, maybe line feeds, even a screenshot or extract of the result.

Thanks for the tip btw :D

[stephan-nordnes-eriksen]

I was working on osx. I totally forgot to include the error message. I actually removed github-todos from the repo. I will try to add it back and reproduce.

FYI: I was adding/removing zurb foundation css, js and other related things in one commit (and of course I did this the silly way of removing it line-by-line in stead of git rm).

[naholyr]

Thanks i'll check out fundation code but my guess is a huge single line file with a todo and bim ;) You can ignore paths in configuration, and i'll add a limit to todo messages

[BarelyAPrincess]

This is an issue on Debian Jessie for me while trying to run the hook for my massive number of previous project commits. Note: I'm simply working with files with a possible large number of changes per each commit.

I believe your bug is found in the child_process.exec function, according to documentation the maximum stdout output by default is only 200/kb, ref: Stack Overflow Question Out of curiosity I ran "git diff -u d23f926..685053c | wc -c", which reported the output was somewhere close to 1.8/mb, which explains why the buffer of 200/kb was being exceeded. So I went into file lib/git.js and added {maxBuffer: 10240000} as the second argument to the exec method on line 29. This fixed the problem except then I was getting TypeError: Cannot read property 'content' of undefined from file lib/parse-diff.js. Since I didn't really feel like finding the source of the problem at that point, I ignored it for now by adding file.lines[0] != null && to the if case on line 19. To my surprise this hasn't seem to cause any issues with the function of the program.