When the "clean" filter processes a pre-existing LFS pointer
blob, the copyToTemp() function returns a CleanPointerError;
it does this when DecodeFrom() successfully reads and parses
the pointer blob and when the entirety of the blob's contents
have been read (i.e., there is no additional blob data to be
read).
This latter check was originally introduced in commit
e09e5e1000bdc890ff8b27f6f330bbdb980f232b in PR #271, when
the relevant code was in the Clean() method in the
pointer/clean.go file; it used the same 512 byte maximum
to determine if all the blob content had been read. This
was aligned with the size of the byte array used in
DecodeFrom() in the pointer/pointer.go file. The 512-byte
buffer created in DecodeFrom() was adjusted and returned to
Clean(), which then checked its length to see if it had been
populated to the 512-byte maximum, meaning there might still
be additional data to be read.
In commit f58db7f7935fe612f455e2939bbf4617dda9e615 in
PR #684 the size of the read buffer in DecodeFrom() was
changed from 512 bytes to the value of blobSizeCutoff, which
was 1024 (and remains so since). However, the check in
Clean()'s copyToTemp() function was not changed at the same
time. (Note that Clean() had been refactored and the check
was in copyToTemp(), where it remains.)
In commit 6f54232890174a119f7dbfd562d5c1ab57174d0b in
PR #1796 the DecodeFrom() method was changed to return an
io.Reader instead of a byte array, and copyToTemp() would
then read from that into a byte array using ioutil.ReadAll().
That introduced a bug when handling large malformed pointers,
fixed in commit dcc05817c350fc7f629271c7bed5fa4f4e36bde9 in
PR #1852 by reading into a byte array in copyToTemp() with
blobSizeCutoff length. However, the check for a resultant
data length of less than 512 bytes still remained in place.
In order to keep this blob size check in sync with the
allocated byte array in copyToTemp(), we therefore change
copyToTemp() so it checks if a pointer was successfully
read and the blob size is below blobSizeCutoff. This
implies there is no more data to be read and we can
return a CleanPointerError (to signal a clean pointer was
found) containing the byte array, which will then be written
out, leaving the blob's contents unchanged.
When we use git hash-object --stdin with the --path option, Git applies
filters to the object, so Git LFS is invoked. However, if the object
provided is less than 1024 bytes in size, we would hang. This occurred
because of our packet reader didn't quite implement the io.Reader
interface completely: if it returned a non-zero value and io.EOF, the
next call to Read would not return 0 and io.EOF. Instead, it would try
to read from stdin, which would not be sending us more data until we
provided a response, so we would hang.
To solve this, keep track of the EOF and always return it on subsequent
Read calls. In addition, don't process the callback to write the file
in this case, since we don't actually want to write into the working
tree.
When writing files to the LFS file storage, we create a temporary file
and rename it into its correct place. Use the function that was
recently introduced to create a temporary file that honors the umask.
This should make all uses of Git LFS honor the umask, since Git handles
writing the working tree files for us.
We compute the proper permissions value on demand. In a future commit,
we'll need to read the configuration file, and on clone, we'll want to
wait to read the configuration until we have a repository.
Add a test for this and skip it on Windows, since we cannot be
guaranteed to have POSIX permission support there.
