Merge pull request #3584 from bk2204/url-case-config

Properly handle config options for URLs with upper case letters

config/config.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 	"sync"
 	"time"
+	"unicode"
 
 	"github.com/git-lfs/git-lfs/fs"
 	"github.com/git-lfs/git-lfs/git"
@@ -155,6 +156,15 @@ func NewFrom(v Values) *Configuration {
 			}
 
 			for key, values := range v.Git {
+				parts := strings.Split(key, ".")
+				isCaseSensitive := len(parts) >= 3
+				hasUpper := strings.IndexFunc(key, unicode.IsUpper) > -1
+
+				// This branch should only ever trigger in
+				// tests, and only if they'd be broken.
+				if !isCaseSensitive && hasUpper {
+					panic(fmt.Sprintf("key %q has uppercase, shouldn't", key))
+				}
 				for _, value := range values {
 					fmt.Printf("Config: %s=%s\n", key, value)
 					source.Lines = append(source.Lines, fmt.Sprintf("%s=%s", key, value))

config/config_test.go

@@ -13,7 +13,7 @@ func TestRemoteDefault(t *testing.T) {
 	cfg := NewFrom(Values{
 		Git: map[string][]string{
 			"branch.unused.remote":     []string{"a"},
-			"branch.unused.pushRemote": []string{"b"},
+			"branch.unused.pushremote": []string{"b"},
 		},
 	})
 	assert.Equal(t, "origin", cfg.Remote())
@@ -24,7 +24,7 @@ func TestRemoteBranchConfig(t *testing.T) {
 	cfg := NewFrom(Values{
 		Git: map[string][]string{
 			"branch.master.remote":    []string{"a"},
-			"branch.other.pushRemote": []string{"b"},
+			"branch.other.pushremote": []string{"b"},
 		},
 	})
 	cfg.ref = &git.Ref{Name: "master"}
@@ -37,8 +37,8 @@ func TestRemotePushDefault(t *testing.T) {
 	cfg := NewFrom(Values{
 		Git: map[string][]string{
 			"branch.master.remote":    []string{"a"},
-			"remote.pushDefault":      []string{"b"},
-			"branch.other.pushRemote": []string{"c"},
+			"remote.pushdefault":      []string{"b"},
+			"branch.other.pushremote": []string{"c"},
 		},
 	})
 	cfg.ref = &git.Ref{Name: "master"}
@@ -51,8 +51,8 @@ func TestRemoteBranchPushDefault(t *testing.T) {
 	cfg := NewFrom(Values{
 		Git: map[string][]string{
 			"branch.master.remote":     []string{"a"},
-			"remote.pushDefault":       []string{"b"},
-			"branch.master.pushRemote": []string{"c"},
+			"remote.pushdefault":       []string{"b"},
+			"branch.master.pushremote": []string{"c"},
 		},
 	})
 	cfg.ref = &git.Ref{Name: "master"}

config/git_fetcher.go

@@ -32,7 +32,10 @@ func readGitConfig(configs ...*git.ConfigurationSource) (gf *GitFetcher, extensi
 			}
 
 			allowed := !gc.OnlySafeKeys
-			key, val := strings.ToLower(pieces[0]), pieces[1]
+
+			// We don't need to change the case of the key here,
+			// since Git will already have canonicalized it for us.
+			key, val := pieces[0], pieces[1]
 
 			if origKey, ok := uniqKeys[key]; ok {
 				if ShowConfigWarnings && len(vals[key]) > 0 && vals[key][len(vals[key])-1] != val && strings.HasPrefix(key, gitConfigWarningPrefix) {
@@ -114,7 +117,8 @@ func readGitConfig(configs ...*git.ConfigurationSource) (gf *GitFetcher, extensi
 // empty string and false will be returned, signaling that the value was
 // absent.
 //
-// Map lookup by key is case-insensitive, as per the .gitconfig specification.
+// Map lookup by key is case-insensitive, except for the middle part of a
+// three-part key, as per the .gitconfig specification.
 //
 // Get is safe to call across multiple goroutines.
 func (g *GitFetcher) Get(key string) (val string, ok bool) {
@@ -130,7 +134,7 @@ func (g *GitFetcher) GetAll(key string) []string {
 	g.vmu.RLock()
 	defer g.vmu.RUnlock()
 
-	return g.vals[strings.ToLower(key)]
+	return g.vals[g.caseFoldKey(key)]
 }
 
 func (g *GitFetcher) All() map[string][]string {
@@ -148,6 +152,27 @@ func (g *GitFetcher) All() map[string][]string {
 	return newmap
 }
 
+func (g *GitFetcher) caseFoldKey(key string) string {
+	parts := strings.Split(key, ".")
+	last := len(parts) - 1
+
+	// We check for 3 or more parts here because if the middle part is a
+	// URL, it may have dots in it. We'll downcase the part before the first
+	// dot and after the last dot, but preserve the piece in the middle,
+	// which may be a branch name, remote, or URL, all of which are
+	// case-sensitive. This is the algorithm Git uses to canonicalize its
+	// keys.
+	if len(parts) < 3 {
+		return strings.ToLower(key)
+	}
+
+	return strings.Join([]string{
+		strings.ToLower(parts[0]),
+		strings.Join(parts[1:last], "."),
+		strings.ToLower(parts[last]),
+	}, ".")
+}
+
 func keyIsUnsafe(key string) bool {
 	for _, safe := range safeKeys {
 		if safe == key {

config/git_fetcher_test.go

@@ -0,0 +1,25 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetCanonicalization(t *testing.T) {
+	vals := map[string][]string{
+		"user.name":                   []string{"Pat Doe"},
+		"branch.MixedCase.pushremote": []string{"Somewhere"},
+		"http.https://example.com/BIG-TEXT.git.extraheader": []string{"X-Foo: Bar"},
+	}
+
+	fetcher := GitFetcher{vals: vals}
+	assert.Equal(t, []string{"Somewhere"}, fetcher.GetAll("bRanch.MixedCase.pushRemote"))
+	assert.Equal(t, []string{"Somewhere"}, fetcher.GetAll("branch.MixedCase.pushremote"))
+	assert.Equal(t, []string(nil), fetcher.GetAll("branch.mixedcase.pushremote"))
+	assert.Equal(t, []string{"Pat Doe"}, fetcher.GetAll("user.name"))
+	assert.Equal(t, []string{"Pat Doe"}, fetcher.GetAll("User.Name"))
+	assert.Equal(t, []string{"X-Foo: Bar"}, fetcher.GetAll("http.https://example.com/BIG-TEXT.git.extraheader"))
+	assert.Equal(t, []string{"X-Foo: Bar"}, fetcher.GetAll("http.https://example.com/BIG-TEXT.git.extraHeader"))
+	assert.Equal(t, []string(nil), fetcher.GetAll("http.https://example.com/big-text.git.extraHeader"))
+}

t/t-extra-header.sh

@@ -96,3 +96,35 @@ begin_test "http.<url>.extraHeader with authorization (casing)"
   [ "0" -eq "$(grep -c "creds: git credential reject" curl.log)" ]
 )
 end_test
+
+begin_test "http.<url>.extraHeader with mixed-case URLs"
+(
+  set -e
+
+  reponame="Mixed-Case-Headers"
+  setup_remote_repo "$reponame"
+  clone_repo "$reponame" "$reponame"
+
+  # These config options check for several things.
+  #
+  # First, we check for mixed-case URLs being read properly and not forced to
+  # lowercase. Second, we check that the user can specify a config option for
+  # the Git URL and have that apply to the LFS URL, which exercises the
+  # URLConfig lookup code. Finally, we also write "ExtraHeader" in mixed-case as
+  # well to test that we lower-case the rightmost portion of the config key
+  # during lookup.
+  url="$(git config remote.origin.url).git"
+  git config --add "http.$url.ExtraHeader" "X-Foo: bar"
+  git config --add "http.$url.ExtraHeader" "X-Foo: baz"
+
+  git lfs track "*.dat"
+  printf "contents" > a.dat
+  git add .gitattributes a.dat
+  git commit -m "initial commit"
+
+  GIT_CURL_VERBOSE=1 GIT_TRACE=1 git push origin master 2>&1 | tee curl.log
+
+  grep "> X-Foo: bar" curl.log
+  grep "> X-Foo: baz" curl.log
+)
+end_test