diff options
4 files changed, 57 insertions, 25 deletions
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/T0003_BasicTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/T0003_BasicTest.java index ab86bc2e27..aa50697172 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/T0003_BasicTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/T0003_BasicTest.java @@ -368,7 +368,7 @@ public class T0003_BasicTest extends SampleDataRepositoryTestCase { + "[user]\n" + " email = A U Thor <thor@example.com> # Just an example...\n" + " name = \"A Thor \\\\ \\\"\\t \"\n" - + " defaultCheckInComment = \"a many line\\ncomment\\n to test\"\n"; + + " defaultCheckInComment = a many line\\ncomment\\n to test\n"; assertEquals(expectedStr, new String(IO.readFully(cfg), Constants.CHARSET)); } diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java index 44714faa70..fb1ee8cadb 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java @@ -977,14 +977,25 @@ public class ConfigTest { } @Test - public void testEscapeSpecialCharacters() throws ConfigInvalidException { + public void testNoEscapeSpecialCharacters() throws ConfigInvalidException { + assertValueRoundTrip("x\\y", "x\\\\y"); + assertValueRoundTrip("x\"y", "x\\\"y"); + assertValueRoundTrip("x\ny", "x\\ny"); + assertValueRoundTrip("x\ty", "x\\ty"); + assertValueRoundTrip("x\by", "x\\by"); + } + + @Test + public void testParseLiteralBackspace() throws ConfigInvalidException { + // This is round-tripped with an escape sequence by JGit, but C git writes + // it out as a literal backslash. + assertEquals("x\by", parseEscapedValue("x\by")); + } + + @Test + public void testEscapeCommentCharacters() throws ConfigInvalidException { assertValueRoundTrip("x#y", "\"x#y\""); assertValueRoundTrip("x;y", "\"x;y\""); - assertValueRoundTrip("x\\y", "\"x\\\\y\""); - assertValueRoundTrip("x\"y", "\"x\\\"y\""); - assertValueRoundTrip("x\ny", "\"x\\ny\""); - assertValueRoundTrip("x\ty", "\"x\\ty\""); - assertValueRoundTrip("x\by", "\"x\\by\""); } @Test @@ -1014,6 +1025,11 @@ public class ConfigTest { assertEquals("baz", parseEscapedValue("baz # comment")); assertEquals("baz", parseEscapedValue("baz " + WS + " ; comment")); assertEquals("baz", parseEscapedValue("baz " + WS + " # comment")); + + assertEquals("baz ", parseEscapedValue("\"baz \"; comment")); + assertEquals("baz ", parseEscapedValue("\"baz \"# comment")); + assertEquals("baz ", parseEscapedValue("\"baz \" ; comment")); + assertEquals("baz ", parseEscapedValue("\"baz \" # comment")); } @Test @@ -1045,16 +1061,17 @@ public class ConfigTest { public void testParseInvalidSubsections() { assertInvalidSubsection( JGitText.get().newlineInQuotesNotAllowed, "\"x\ny\""); - assertInvalidSubsection( - MessageFormat.format(JGitText.get().badEscape, 'q'), "\"x\\q\""); + } + @Test + public void testDropBackslashFromInvalidEscapeSequenceInSubsectionName() + throws ConfigInvalidException { + assertEquals("x0", parseEscapedSubsection("\"x\\0\"")); + assertEquals("xq", parseEscapedSubsection("\"x\\q\"")); // Unlike for values, \b, \n, and \t are not valid escape sequences. - assertInvalidSubsection( - MessageFormat.format(JGitText.get().badEscape, 'b'), "\"x\\b\""); - assertInvalidSubsection( - MessageFormat.format(JGitText.get().badEscape, 'n'), "\"x\\n\""); - assertInvalidSubsection( - MessageFormat.format(JGitText.get().badEscape, 't'), "\"x\\t\""); + assertEquals("xb", parseEscapedSubsection("\"x\\b\"")); + assertEquals("xn", parseEscapedSubsection("\"x\\n\"")); + assertEquals("xt", parseEscapedSubsection("\"x\\t\"")); } private static void assertValueRoundTrip(String value) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackInserter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackInserter.java index 39cf5bd5da..7ff269694b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackInserter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackInserter.java @@ -99,7 +99,7 @@ import org.eclipse.jgit.util.sha1.SHA1; * Object inserter that inserts one pack per call to {@link #flush()}, and never * inserts loose objects. */ -class PackInserter extends ObjectInserter { +public class PackInserter extends ObjectInserter { /** Always produce version 2 indexes, to get CRC data. */ private static final int INDEX_VERSION = 2; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java index 39161214de..a6313f0cc5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java @@ -155,9 +155,18 @@ public class Config { StringBuilder r = new StringBuilder(x.length()); for (int k = 0; k < x.length(); k++) { char c = x.charAt(k); - boolean thisCharNeedsQuote = true; - - // git-config(1) lists the limited set of supported escape sequences. + // git-config(1) lists the limited set of supported escape sequences, but + // the documentation is otherwise not especially normative. In particular, + // which ones of these produce and/or require escaping and/or quoting + // around them is not documented and was discovered by trial and error. + // In summary: + // + // * Quotes are only required if there is leading/trailing whitespace or a + // comment character. + // * Bytes that have a supported escape sequence are escaped, except for + // \b for some reason which isn't. + // * Needing an escape sequence is not sufficient reason to quote the + // value. switch (c) { case '\0': // Unix command line calling convention cannot pass a '\0' as an @@ -175,25 +184,30 @@ public class Config { break; case '\b': + // Doesn't match `git config foo.bar $'x\by'`, which doesn't escape the + // \x08, but since both escaped and unescaped forms are readable, we'll + // prefer internal consistency here. r.append('\\').append('b'); break; case '\\': + r.append('\\').append('\\'); + break; + case '"': - r.append('\\').append(c); + r.append('\\').append('"'); break; case '#': case ';': + needQuote = true; r.append(c); break; default: - thisCharNeedsQuote = false; r.append(c); break; } - needQuote |= thisCharNeedsQuote; } return needQuote ? '"' + r.toString() + '"' : r.toString(); @@ -1301,9 +1315,10 @@ public class Config { continue; default: - throw new ConfigInvalidException(MessageFormat.format( - JGitText.get().badEscape, - Character.valueOf(((char) c)))); + // C git simply drops backslashes if the escape sequence is not + // recognized. + r.append((char) c); + continue; } } if ('"' == c) { |