Git LFS behind a proxy about git-lfs HOT 3 OPEN

Hey, I'm sorry you're having trouble, and thanks for the detailed report.

I think I've been able to reproduce this issue with a fairly simple case. Assuming there's a repository on GitHub like https://github.com/chrisd8088/test:

$ export HTTPS_PROXY=https://google.com
$ git config --global http.https://github.com.proxy ''
$ git config --global http.https://github.com.version 'HTTP/1.1'

$ GIT_TRACE=1 GIT_TRANSFER_TRACE=1 GIT_CURL_VERBOSE=1 git clone https://github.com/chrisd8088/test
...
> POST /chrisd8088/test.git/info/lfs/objects/batch HTTP/1.1
...
20:23:44.355924 trace git-lfs: api error: Post "https://github.com/chrisd8088/test.git/info/lfs/objects/batch": Not Found

The issue, I believe, stems from a differing interpretation of an empty http.proxy/http.<url>.proxy value. In Git, an empty value sets the CURLOPT_PROXY option to an empty string, and according to the cURL library, that explicitly disables proxying, so that any HTTP_PROXY or similar environment variables are ignored. In Git LFS, though, we only consider the http.proxy/http.<url>.proxy value found in the Git configuration if it is not empty, and otherwise we ignore it, so the environment variables take effect.

I'm therefore going to mark this as a bug, since we aim to follow Git's implementation in these sorts of matters.

from git-lfs.

Thanks for the detailed investigation, and I think this is exactly the problem I encountered. Now by setting NO_PROXY I temporarily bypass this bug.

from git-lfs.

A few extra notes from some investigation into ways to address this issue:

There's a bit of a confusing history to the relevant code. The original implementation dates from PR #1358, where an empty Git configuration setting was ignored and treated as an unset value, so any environment variables would then take effect.

There was a change in PR #3062 to a later version of this code with the stated intention of fixing the bug reported in #3060, where environment variables supposedly took precedence over Git configuration settings. However, from what I can tell, that change (which persists in the current version of the code) had no real effect, which suggests that the bug report was either incorrect or reflected some particular condition which was not documented at the time.

Confusingly, in commit 5db9370 of PR #3062 the TestHttpsProxyFromGitConfig test was revised to have the opposite meaning from what is desired, and from what the stated intent of the PR was, making the test check that environment variables were preferred over Git configuration settings. This apparently then caused CI failures (since, as far as I can tell, the PR did not alter the behaviour of the code, which both before and after the PR would have preferred non-empty Git config proxy settings over the corresponding environment variables), and the test was returned to its original, correct expectations in PR #3138.

So far as I can tell, though, at no point were empty Git configuration settings (or empty but not unset environment variables) considered in any of these changes.

Meanwhile, ever since the first implementation in PR #1358 we've honoured the HTTP_PROXY environment variable (note the uppercase name), preferring its value to that of any http_proxy variable. However, neither Git nor cURL read the uppercase version of that variable. The curl(1) command is explicitly documented to only accept the lowercase variant:

The environment variables can be specified in lower case or upper case. The lower case version has precedence. http_proxy is an exception as it is only available in lower case.

On that note, we also incorrectly prefer the uppercase version of HTTPS_PROXY to the lowercase version. This should work, but doesn't, because while Git clones successfully, Git LFS tries to contact the wrong host:

$ export HTTPS_PROXY=https://google.com
$ export https_proxy=
$ GIT_TRACE=1 GIT_TRANSFER_TRACE=1 GIT_CURL_VERBOSE=1 git clone https://github.com/chrisd8088/test
...
> POST /chrisd8088/test.git/info/lfs/objects/batch HTTP/1.1
...
00:06:13.128663 trace git-lfs: api error: Post "https://github.com/chrisd8088/test.git/info/lfs/objects/batch": malformed HTTP response

The behaviour above appears to stem from the Go language's support for these environment variables, which reads both HTTP_PROXY and http_proxy, unlike either the Git or cURL implementations.

However, the Go implementation in the x/net/http/httpproxy package has been changed from the version on which our code was based in one particular respect; namely, it now only applies the value of any HTTP_PROXY or http_proxy environment variable to HTTP connections, never HTTPS connections. Previously, it used those as a fallback for HTTPS connection proxies if one was not defined in HTTPS_PROXY or https_proxy, but this was altered in commit golang/net@7b1cca2 to address the issues raised in golang/go#40909.

But we still treat any proxy defined for HTTP as a fallback for HTTPS connections, reflecting how the Go language implementation used to behave.

As well, there are some oddities in our getProxyServers() function left over from the change in PR #3062. For instance, the function returns immediately if osEnv is nil. This used to make sense because the function first checked for any http.proxy/http.<url>.proxy settings, and then could return early if osEnv was not valid as there was no point in using it to look for environment variables. Now, though, we never reach the checks for Git configuration settings if osEnv is nil, although it would still be possible to do so if we skipped over the environment variable lookups.

We also perform some checks, like the check for an empty httpsProxy string, at points where it's not possible they could be anything other than an empty string because they are defined as return variables and haven't been set yet. Again, these checks used to make sense because the Git configuration values were read first and used to set the httpsProxy and httpProxy variables, but with the change in PR #3062, they are now redundant.

Whether we can fix all of these concerns in the v3.x release series I'm not immediately certain. We should be able to address most of the issues, but removing support for HTTP_PROXY in particular might be considered a change which could break some existing user configurations (even though Git ignores that environment variable) and have to wait for v4.0.0.

from git-lfs.