Our NTLM support has been known to be broken in various situations for a
while, specifically on Windows. The core team is unable to troubleshoot
these problems, and nobody has stepped up to maintain the NTLM support.
In addition, NTLM uses cryptography and security techniques that are
known to be insecure, such as the algorithms DES, MD4, and MD5, as well
as simple, unsalted hashes of passwords.
Since we now support Kerberos, most users should be able to replace
their use of NTLM with Kerberos instead. Users have reported this
working on Windows and it is known to work well on at least Debian as
well. Drop support for NTLM and remove it from the codebase.
Since we're uploading and downloading a large amount of data, we may
prefer to use transport compression. Currently, Go will do this for us
automatically unless we ask it not to. Let's detect this case and add a
trace output so people can find it and we can add a test that it
continues to work.
Note that it's not possible to detect this using the headers since Go
adds the headers on the request and strips the headers on the response
as part of making the request, so it's not readily apparent to users
without this trace output.
Currently, our default branch in tests is "master". This is the Git
default, but the Git default will likely change in the future, so it
makes sense to update our testsuite to be explicit about the branch
name. We'll ensure this continues by building against older versions of
Git as well as newer versions.
We use "main" for the new branch name, since that's the proposed value
upstream.
This commit was made entirely by automated means using the following
command:
git grep -l master t | xargs sed -i -e 's/master/main/g'
When running our test suite, including the Go tests, we want to ensure
that our locally compiled "git-lfs" binary is the one which is used
whenever "git-lfs" is invoked, for instance as a Git filter command
or as the transfer queue's default custom adapter. Otherwise we may
see unexpected test failures or successes, if another "git-lfs" binary
installed elsewhere on the system is used instead.
Therefore we define an init() function in the common test utilities
package which prepends to the PATH environment variable an absolute path
to the working tree's "bin/" directory, to ensure our "git-lfs" binary
is always found in preference to any other installed versions.
It's important for us to flag this binary as a test tool so that Go
doesn't complain about us having multiple instances of the main function
in different files. Not doing this causes failures when the Debian
package is built in the Docker containers, since the Debian build system
wants to build all packages, including the test packages, at once.
On Actions, our Windows build root has a path with a short name. This
causes significant grief, since our tests expect one version of the path
and we output another, causing our tests for equality to fail. Since
Windows lacks a realpath(1) utility, provide a small wrapper around the
Go primitives to provide a limited version of this utility for use only
in our tests.
The credential helper can fail in some cases when it gets data that
doesn't contain a colon. In such a case, make it print a useful error
instead of panicking.
When we make a request to the test server, we need an appropriate Accept
header, and when we make a POST request, we need an appropriate
Content-Type header as well. To ensure we have the right behavior in all
code paths, have the test server check for the Accept header and return
a 406 if it's not correct. Similarly, check all POST requests for an
appropriate Content-Type header, and fail the request if it's wrong.
This ensures that all of our code paths have the expected behavior.
The upstream of go-ntlm has archived its repository and is no longer
doing releases. Because this dependency is required for Git LFS, we've
created our own fork to ensure that the upstream repo doesn't disappear
on us. Use our own copy of go-ntlm within Git LFS.
When there's a problem with the test server, it's helpful to see the log
output to help troubleshoot the issue. However, when the Stderr member
of a command struct is not set, the standard error gets redirected to
/dev/null, which isn't really what we want.
Since we already have a log set up for logging standard output, send
standard error there as well.
If we're attempting to resume a download but the file we have on disk is
the expected size or too large, we'll send an invalid Range header where
the lower bound is greater than the upper bound. If this situation
occurs, simply retry the download from the beginning, since we clearly
don't have the expected data (or we wouldn't be resuming) and it isn't
clear how else to recover.
Honor the repository permssions set with the umask or
core.sharedRepository when creating directories by using tools.MkdirAll.
Pass an appropriate configuration or filesystem object through as needed
to ensure that we can query the proper permissions. Add a test that
these code paths work by cloning a new repository and then performing
some operations on it.
Note that in the test, we match patterns with either the setgid bit
clear or set, since it may be set or not depending on whether the system
has SysV or BSD group semantics.
For tq requests, use the URL being transferred to for the access
mode instead of the LFS API endpoint. Since these endpoints are
different, this will ensure credentials aren't passed along
needlessly if, for example, the API endpoint requires
authentication and the endpoint being transferred to does not.
Add a new Access parameter to DoWithAuth() to allow callers to
specify the URL Access Mode. This removes the assumption that
DoWithAuth() will always be used for an LFS API request. This
commit also adds DoAPIRequestWithAuth(), which provides the
functionality of DoWithAuth() while explicitly using the LFS API
endpoint for the access mode, allowing parity with the previous
DoWithAuth() functionality.
In the future, we'll want to run integration tests that use an encrypted
private key. To make this as easy as possible, create an encrypted
private key as part of the test setup. Set up credentials for this key
using the credential helper.
Note that in order to get Git to use the credential helper for the
encrypted private key instead of prompting using OpenSSL, we need to
set GIT_SSL_CERT_PASSWORD_PROTECTED, which makes Git think that the
certificate is password-protected, too. Therefore, specify credentials
for both the certificate (which isn't really encrypted) and the
encrypted private key.
The lfstest credential helper provides credentials found by reading a
file on the system. Currently, it expects that there is a colon in the
URL followed by one or more slashes. If there is not (say, because
we're using a file:/// or cert:/// URL), it fails to concatenate the
path properly, and instead of looking in the credentials directory,
looks for a different file one directory higher. This occurs because
filepath.Join strips off a trailing slash if the second argument passed
to it is empty.
This, of course, makes the credential helper fail to work and the tests
hang waiting for input. Insert an additional slash in the environment
variable and handle the empty string case properly so that credentials
for local paths can be looked up properly.
There are current some tests which randomly fail on CI systems that
don't appear to fail on normal development systems. To help
troubleshoot these, always log when the lfstest credential helper (which
is used in our tests) receives a command, even if we ignore it.
When a custom transfer adapter is specified in the configuration, it is
not possible to specify multiple arguments to the process, because only
the last value of the lfs.customtransfer.*.args option is read and the
value is not split. To make things easier and more flexible,
concatenate the path (after quoting) and the arguments and pass them to
the shell. Update the documentation to reflect this change.
Update the test custom transfer adapter to parse its arguments (which
are otherwise ignored) and mention them to standard error. Make the
test check that the arguments are parsed as the shell would expect them
to be.
The custom adapter test binary wraps the HTTP API client and returns
various responses from the HTTP response to the custom transfer adapter.
This code assumes that if an error occurs, we will always have a valid
HTTP response object, but this is not the case. If we lack such an
object, we will get a nil pointer dereference, a panic, and a segfault
in the client, causing the test to fail.
While it's currently unknown how to reproduce this issue, it has been
seen on the CI servers. In order to improve the reliability of the
test, check to see if the response object is non-nil before
dereferencing it and produce a different but distinct error code if it
is nil.
On the AppVeyor CI Windows environment, there exist issues that prevent
the lfstest-count-tests program from acquiring the test_count.lock file
correctly.
Instead of debugging these environment-specific issues further, note
that because GIT_LFS_NO_TEST_COUNT=1, there is only ever one running
instance of lfstest-count-tests (at the beginning and end of the
integration suite).
So, disable the lock acquisition step entirely, and thus skip acquiring
the test_count.lock file in the first place.
In both the 'increment' and 'decrement' countFn, we observer the
following pattern:
if /* some computation with n */ {
// ...
return n +- 1, nil
}
// ...
return /* 0 or 1 */, nil
But in the below case of the increment implementation, we violate this
example. Let's fix that be adhering to the above.
By default, the prove(1) program looks for tests to run in the 't'
directory at the repository root, so let's put existing tests there in
order to remove a non-default argument from prove.
