Merge pull request #643 from thockin/master

V4: Allow quoted keys for --git-config

cmd/git-sync/main.go

@@ -133,7 +133,7 @@ var flAskPassURL = pflag.String("askpass-url", envMultiString([]string{"GIT_SYNC
 var flGitCmd = pflag.String("git", envString("GIT_SYNC_GIT", "git"),
 	"the git command to run (subject to PATH search, mostly for testing)")
 var flGitConfig = pflag.String("git-config", envString("GIT_SYNC_GIT_CONFIG", ""),
-	"additional git config options in 'key1:val1,key2:val2' format")
+	"additional git config options in 'section.var1:val1,\"section.sub.var2\":\"val2\"' format")
 var flGitGC = pflag.String("git-gc", envString("GIT_SYNC_GIT_GC", "auto"),
 	"git garbage collection behavior: one of 'auto', 'always', 'aggressive', or 'off'")
 
@@ -1588,8 +1588,9 @@ func (git *repoSync) SetupCookieFile(ctx context.Context) error {
 //
 // The expected URL callback output is below,
 // see https://git-scm.com/docs/gitcredentials for more examples:
-//   username=xxx@example.com
+//
-//   password=xxxyyyzzz
+//	username=xxx@example.com
+//	password=xxxyyyzzz
 func (git *repoSync) CallAskPassURL(ctx context.Context) error {
 	git.log.V(2).Info("calling auth URL to get credentials")
 
@@ -1714,9 +1715,19 @@ func parseGitConfigs(configsFlag string) ([]keyVal, error) {
 		if r, ok := <-ch; !ok {
 			break
 		} else {
-			cur.key, err = parseGitConfigKey(r, ch)
+			// This can accumulate things that git doesn't allow, but we'll
-			if err != nil {
+			// just let git handle it, rather than try to pre-validate to their
-				return nil, err
+			// spec.
+			if r == '"' {
+				cur.key, err = parseGitConfigQKey(ch)
+				if err != nil {
+					return nil, err
+				}
+			} else {
+				cur.key, err = parseGitConfigKey(r, ch)
+				if err != nil {
+					return nil, err
+				}
 			}
 		}
 
@@ -1743,6 +1754,23 @@ func parseGitConfigs(configsFlag string) ([]keyVal, error) {
 	return result, nil
 }
 
+func parseGitConfigQKey(ch <-chan rune) (string, error) {
+	str, err := parseQString(ch)
+	if err != nil {
+		return "", err
+	}
+
+	// The next character must be a colon.
+	r, ok := <-ch
+	if !ok {
+		return "", fmt.Errorf("unexpected end of key: %q", str)
+	}
+	if r != ':' {
+		return "", fmt.Errorf("unexpected character after quoted key: %q%c", str, r)
+	}
+	return str, nil
+}
+
 func parseGitConfigKey(r rune, ch <-chan rune) (string, error) {
 	buf := make([]rune, 0, 64)
 	buf = append(buf, r)
@@ -1752,9 +1780,6 @@ func parseGitConfigKey(r rune, ch <-chan rune) (string, error) {
 		case r == ':':
 			return string(buf), nil
 		default:
-			// This can accumulate things that git doesn't allow, but we'll
-			// just let git handle it, rather than try to pre-validate to their
-			// spec.
 			buf = append(buf, r)
 		}
 	}
@@ -1762,30 +1787,17 @@ func parseGitConfigKey(r rune, ch <-chan rune) (string, error) {
 }
 
 func parseGitConfigQVal(ch <-chan rune) (string, error) {
-	buf := make([]rune, 0, 64)
+	str, err := parseQString(ch)
-
+	if err != nil {
-	for r := range ch {
+		return "", err
-		switch r {
-		case '\\':
-			if e, err := unescape(ch); err != nil {
-				return "", err
-			} else {
-				buf = append(buf, e)
-			}
-		case '"':
-			// Once we have a closing quote, the next must be either a comma or
-			// end-of-string.  This helps reset the state for the next key, if
-			// there is one.
-			r, ok := <-ch
-			if ok && r != ',' {
-				return "", fmt.Errorf("unexpected trailing character '%c'", r)
-			}
-			return string(buf), nil
-		default:
-			buf = append(buf, r)
-		}
 	}
-	return "", fmt.Errorf("unexpected end of value: %q", string(buf))
+
+	// If there is a next character, it must be a comma.
+	r, ok := <-ch
+	if ok && r != ',' {
+		return "", fmt.Errorf("unexpected character after quoted value %q%c", str, r)
+	}
+	return str, nil
 }
 
 func parseGitConfigVal(r rune, ch <-chan rune) (string, error) {
@@ -1810,6 +1822,26 @@ func parseGitConfigVal(r rune, ch <-chan rune) (string, error) {
 	return string(buf), nil
 }
 
+func parseQString(ch <-chan rune) (string, error) {
+	buf := make([]rune, 0, 64)
+
+	for r := range ch {
+		switch r {
+		case '\\':
+			if e, err := unescape(ch); err != nil {
+				return "", err
+			} else {
+				buf = append(buf, e)
+			}
+		case '"':
+			return string(buf), nil
+		default:
+			buf = append(buf, r)
+		}
+	}
+	return "", fmt.Errorf("unexpected end of quoted string: %q", string(buf))
+}
+
 // unescape processes most of the documented escapes that git config supports.
 func unescape(ch <-chan rune) (rune, error) {
 	r, ok := <-ch
@@ -1919,14 +1951,22 @@ OPTIONS
             testing).  This defaults to "git".
 
     --git-config <string>, $GIT_SYNC_GIT_CONFIG
-            Additional git config options in 'key1:val1,key2:val2' format.  The
+            Additional git config options in a comma-separated 'key:val'
-            key parts are passed to 'git config' and must be valid syntax for
+            format.  The parsed keys and values are passed to 'git config' and
-            that command.  The val parts can be either quoted or unquoted
+            must be valid syntax for that command.
-            values.  For all values the following escape sequences are
+
-            supported: '\n' => [newline], '\t' => [tab], '\"' => '"', '\,' =>
+            Both keys and values can be either quoted or unquoted strings.
-            ',', '\\' => '\'.  Within unquoted values, commas MUST be escaped.
+            Within quoted keys and all values (quoted or not), the following
-            Within quoted values, commas MAY be escaped, but are not required
+            escape sequences are supported:
-            to be.  Any other escape sequence is an error.
+                '\n' => [newline]
+                '\t' => [tab]
+                '\"' => '"'
+                '\,' => ','
+                '\\' => '\'
+            To include a colon within a key (e.g. a URL) the key must be
+            quoted.  Within unquoted values commas must be escaped.  Within
+            quoted values commas may be escaped, but are not required to be.
+            Any other escape sequence is an error.
 
     --git-gc <string>, $GIT_SYNC_GIT_GC
             The git garbage collection behavior: one of "auto", "always",

cmd/git-sync/main_test.go

@@ -194,26 +194,66 @@ func TestParseGitConfigs(t *testing.T) {
 		name:   "one-pair",
 		input:  `k:v`,
 		expect: []keyVal{keyVal{"k", "v"}},
+	}, {
+		name:   "one-pair-qkey",
+		input:  `"k":v`,
+		expect: []keyVal{keyVal{"k", "v"}},
 	}, {
 		name:   "one-pair-qval",
 		input:  `k:"v"`,
 		expect: []keyVal{keyVal{"k", "v"}},
+	}, {
+		name:   "one-pair-qkey-qval",
+		input:  `"k":"v"`,
+		expect: []keyVal{keyVal{"k", "v"}},
+	}, {
+		name:   "multi-pair",
+		input:  `k1:v1,"k2":v2,k3:"v3","k4":"v4"`,
+		expect: []keyVal{{"k1", "v1"}, {"k2", "v2"}, {"k3", "v3"}, {"k4", "v4"}},
 	}, {
 		name:  "garbage",
 		input: `abc123`,
 		fail:  true,
 	}, {
-		name:  "invalid-val",
+		name:   "key-section-var",
-		input: `k:v\xv`,
+		input:  `sect.var:v`,
-		fail:  true,
+		expect: []keyVal{keyVal{"sect.var", "v"}},
 	}, {
-		name:  "invalid-qval",
+		name:   "key-section-subsection-var",
-		input: `k:"v\xv"`,
+		input:  `sect.sub.var:v`,
-		fail:  true,
+		expect: []keyVal{keyVal{"sect.sub.var", "v"}},
 	}, {
-		name:   "two-pair",
+		name:   "key-subsection-with-space",
-		input:  `k1:v1,k2:v2`,
+		input:  `k.sect.sub section:v`,
-		expect: []keyVal{{"k1", "v1"}, {"k2", "v2"}},
+		expect: []keyVal{keyVal{"k.sect.sub section", "v"}},
+	}, {
+		name:   "key-subsection-with-escape",
+		input:  `k.sect.sub\tsection:v`,
+		expect: []keyVal{keyVal{"k.sect.sub\\tsection", "v"}},
+	}, {
+		name:   "key-subsection-with-comma",
+		input:  `k.sect.sub,section:v`,
+		expect: []keyVal{keyVal{"k.sect.sub,section", "v"}},
+	}, {
+		name:   "qkey-subsection-with-space",
+		input:  `"k.sect.sub section":v`,
+		expect: []keyVal{keyVal{"k.sect.sub section", "v"}},
+	}, {
+		name:   "qkey-subsection-with-escapes",
+		input:  `"k.sect.sub\t\n\\section":v`,
+		expect: []keyVal{keyVal{"k.sect.sub\t\n\\section", "v"}},
+	}, {
+		name:   "qkey-subsection-with-comma",
+		input:  `"k.sect.sub,section":v`,
+		expect: []keyVal{keyVal{"k.sect.sub,section", "v"}},
+	}, {
+		name:   "qkey-subsection-with-colon",
+		input:  `"k.sect.sub:section":v`,
+		expect: []keyVal{keyVal{"k.sect.sub:section", "v"}},
+	}, {
+		name:  "invalid-qkey",
+		input: `"k\xk":v"`,
+		fail:  true,
 	}, {
 		name:   "val-spaces",
 		input:  `k1:v 1,k2:v 2`,
@@ -235,17 +275,21 @@ func TestParseGitConfigs(t *testing.T) {
 		input:  `k1:"v1",k2:"v2",`,
 		expect: []keyVal{{"k1", "v1"}, {"k2", "v2"}},
 	}, {
-		name:   "val-escapes",
+		name:   "val-with-escapes",
 		input:  `k1:v\n\t\\\"\,1`,
 		expect: []keyVal{{"k1", "v\n\t\\\",1"}},
 	}, {
-		name:   "qval-escapes",
+		name:   "qval-with-escapes",
 		input:  `k1:"v\n\t\\\"\,1"`,
 		expect: []keyVal{{"k1", "v\n\t\\\",1"}},
 	}, {
-		name:   "qval-comma",
+		name:   "qval-with-comma",
 		input:  `k1:"v,1"`,
 		expect: []keyVal{{"k1", "v,1"}},
+	}, {
+		name:  "qkey-missing-close",
+		input: `"k1:v1`,
+		fail:  true,
 	}, {
 		name:  "qval-missing-close",
 		input: `k1:"v1`,
@@ -262,7 +306,7 @@ func TestParseGitConfigs(t *testing.T) {
 				t.Errorf("unexpected success")
 			}
 			if !reflect.DeepEqual(kvs, tc.expect) {
-				t.Errorf("bad result: expected %v, got %v", tc.expect, kvs)
+				t.Errorf("bad result:\n\texpected: %#v\n\t     got: %#v", tc.expect, kvs)
 			}
 		})
 	}