From e3468735111f26e4cdb4c249c257da52f4c3cffb Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 26 Sep 2016 16:41:19 -0400 Subject: [PATCH] TreeFormatter: disallow empty filenames in trees Git barfs on these (and they don't make any sense), so we certainly shouldn't write them. Change-Id: I3faf8554a05f0fd147be2e63fbe55987d3f88099 Signed-off-by: David Turner Signed-off-by: David Pursehouse --- .../storage/file/T0003_BasicTest.java | 15 ++++++++++ .../DirCacheCheckoutMaliciousPathTest.java | 4 ++- .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../org/eclipse/jgit/lib/TreeFormatter.java | 29 +++++++++++++++++++ 5 files changed, 49 insertions(+), 1 deletion(-) 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 b6ad22bbe..ae1e531b8 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 @@ -82,9 +82,13 @@ import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.storage.file.FileRepositoryBuilder; import org.eclipse.jgit.test.resources.SampleDataRepositoryTestCase; import org.eclipse.jgit.util.FileUtils; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; public class T0003_BasicTest extends SampleDataRepositoryTestCase { + @Rule + public ExpectedException expectedException = ExpectedException.none(); @Test public void test001_Initalize() { @@ -325,6 +329,17 @@ public class T0003_BasicTest extends SampleDataRepositoryTestCase { assertFalse("Exists " + o, o.isFile()); } + @Test + public void test002_CreateBadTree() throws Exception { + // We won't create a tree entry with an empty filename + // + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage(JGitText.get().invalidTreeZeroLengthName); + final TreeFormatter formatter = new TreeFormatter(); + formatter.append("", FileMode.TREE, + ObjectId.fromString("4b825dc642cb6eb9a060e54bf8d69288fbee4904")); + } + @Test public void test006_ReadUglyConfig() throws IOException, ConfigInvalidException { diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutMaliciousPathTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutMaliciousPathTest.java index 59a4699bc..32a1ec96a 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutMaliciousPathTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/DirCacheCheckoutMaliciousPathTest.java @@ -366,7 +366,9 @@ public class DirCacheCheckoutMaliciousPathTest extends RepositoryTestCase { insertId = blobId; for (int i = path.length - 1; i >= 0; --i) { TreeFormatter treeFormatter = new TreeFormatter(); - treeFormatter.append(path[i], mode, insertId); + treeFormatter.append(path[i].getBytes(), 0, + path[i].getBytes().length, + mode, insertId, true); insertId = newObjectInserter.insert(treeFormatter); mode = FileMode.TREE; } diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 2c721eabb..5f420ab18 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -367,6 +367,7 @@ invalidTagOption=Invalid tag option: {0} invalidTimeout=Invalid timeout: {0} invalidTimeUnitValue2=Invalid time unit value: {0}.{1}={2} invalidTimeUnitValue3=Invalid time unit value: {0}.{1}.{2}={3} +invalidTreeZeroLengthName=Cannot append a tree entry with zero-length name invalidURL=Invalid URL {0} invalidWildcards=Invalid wildcards {0} invalidRefSpec=Invalid refspec {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 956171b12..e920554a4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -425,6 +425,7 @@ public class JGitText extends TranslationBundle { /***/ public String invalidTimeout; /***/ public String invalidTimeUnitValue2; /***/ public String invalidTimeUnitValue3; + /***/ public String invalidTreeZeroLengthName; /***/ public String invalidURL; /***/ public String invalidWildcards; /***/ public String invalidRefSpec; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/TreeFormatter.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/TreeFormatter.java index 065b8f46b..777ce94aa 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/TreeFormatter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/TreeFormatter.java @@ -53,6 +53,7 @@ import static org.eclipse.jgit.lib.FileMode.TREE; import java.io.IOException; import org.eclipse.jgit.errors.CorruptObjectException; +import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.revwalk.RevBlob; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevTree; @@ -193,6 +194,34 @@ public class TreeFormatter { */ public void append(byte[] nameBuf, int namePos, int nameLen, FileMode mode, AnyObjectId id) { + append(nameBuf, namePos, nameLen, mode, id, false); + } + + /** + * Append any entry to the tree. + * + * @param nameBuf + * buffer holding the name of the entry. The name should be UTF-8 + * encoded, but file name encoding is not a well defined concept + * in Git. + * @param namePos + * first position within {@code nameBuf} of the name data. + * @param nameLen + * number of bytes from {@code nameBuf} to use as the name. + * @param mode + * mode describing the treatment of {@code id}. + * @param id + * the ObjectId to store in this entry. + * @param allowEmptyName + * allow an empty filename (creating a corrupt tree) + * @since 4.6 + */ + public void append(byte[] nameBuf, int namePos, int nameLen, FileMode mode, + AnyObjectId id, boolean allowEmptyName) { + if (nameLen == 0 && !allowEmptyName) { + throw new IllegalArgumentException( + JGitText.get().invalidTreeZeroLengthName); + } if (fmtBuf(nameBuf, namePos, nameLen, mode)) { id.copyRawTo(buf, ptr); ptr += OBJECT_ID_LENGTH;