From cc2197ed9c3be35c875c8316f17a2d6e8d338c88 Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Tue, 24 May 2011 00:45:21 +0200 Subject: Fix CloneCommand not to fetch into remote tracking branches when bare When cloning into a bare repository we should not create remote tracking branches (e.g refs/remotes/origin/testX). Branches of the remote repository should but fetched into into branches of the same name (e.g refs/heads/testX). Also add the noCheckout option which would prevent checkout after fetch. Change-Id: I5d4cc0389f3f30c53aa0065f38119af2a1430909 Signed-off-by: Christian Halstrick --- .../tst/org/eclipse/jgit/api/CloneCommandTest.java | 70 +++++++++++++++++++++- .../org/eclipse/jgit/api/GitConstructionTest.java | 4 +- 2 files changed, 70 insertions(+), 4 deletions(-) (limited to 'org.eclipse.jgit.test/tst/org') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java index f21dc4a0be..9d01291b7a 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java @@ -49,12 +49,14 @@ import static org.junit.Assert.fail; import java.io.File; import java.io.IOException; import java.util.Collections; +import java.util.List; import org.eclipse.jgit.api.ListBranchCommand.ListMode; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.RepositoryTestCase; @@ -136,6 +138,42 @@ public class CloneCommandTest extends RepositoryTestCase { assertNotNull(git2); assertEquals(git2.getRepository().getFullBranch(), "refs/heads/master"); + assertEquals( + "refs/heads/master, refs/remotes/origin/master, refs/remotes/origin/test", + allRefNames(git2.branchList().setListMode(ListMode.ALL) + .call())); + + // Same thing, but now without checkout + directory = createTempDirectory("testCloneRepositoryWithBranch_bare"); + command = Git.cloneRepository(); + command.setBranch("refs/heads/master"); + command.setDirectory(directory); + command.setURI("file://" + + git.getRepository().getWorkTree().getPath()); + command.setNoCheckout(true); + git2 = command.call(); + assertNotNull(git2); + assertEquals(git2.getRepository().getFullBranch(), + "refs/heads/master"); + assertEquals( + "refs/remotes/origin/master, refs/remotes/origin/test", + allRefNames(git2.branchList().setListMode(ListMode.ALL) + .call())); + + // Same thing, but now test with bare repo + directory = createTempDirectory("testCloneRepositoryWithBranch_bare"); + command = Git.cloneRepository(); + command.setBranch("refs/heads/master"); + command.setDirectory(directory); + command.setURI("file://" + + git.getRepository().getWorkTree().getPath()); + command.setBare(true); + git2 = command.call(); + assertNotNull(git2); + assertEquals(git2.getRepository().getFullBranch(), + "refs/heads/master"); + assertEquals("refs/heads/master, refs/heads/test", allRefNames(git2 + .branchList().setListMode(ListMode.ALL).call())); } catch (Exception e) { fail(e.getMessage()); } @@ -156,13 +194,41 @@ public class CloneCommandTest extends RepositoryTestCase { assertNotNull(git2); assertEquals(git2.getRepository().getFullBranch(), "refs/heads/master"); - assertEquals(1, git2.branchList().setListMode(ListMode.REMOTE) - .call().size()); + assertEquals("refs/remotes/origin/master", + allRefNames(git2.branchList() + .setListMode(ListMode.REMOTE).call())); + + // Same thing, but now test with bare repo + directory = createTempDirectory("testCloneRepositoryWithBranch_bare"); + command = Git.cloneRepository(); + command.setBranch("refs/heads/master"); + command.setBranchesToClone(Collections + .singletonList("refs/heads/master")); + command.setDirectory(directory); + command.setURI("file://" + + git.getRepository().getWorkTree().getPath()); + command.setBare(true); + git2 = command.call(); + assertNotNull(git2); + assertEquals(git2.getRepository().getFullBranch(), + "refs/heads/master"); + assertEquals("refs/heads/master", allRefNames(git2 + .branchList().setListMode(ListMode.ALL).call())); } catch (Exception e) { fail(e.getMessage()); } } + public static String allRefNames(List refs) { + StringBuilder sb = new StringBuilder(); + for (Ref f : refs) { + if (sb.length() > 0) + sb.append(", "); + sb.append(f.getName()); + } + return sb.toString(); + } + public static File createTempDirectory(String name) throws IOException { final File temp; temp = File.createTempFile(name, Long.toString(System.nanoTime())); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/GitConstructionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/GitConstructionTest.java index 0e0b18732b..b6ba4c927d 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/GitConstructionTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/GitConstructionTest.java @@ -79,7 +79,7 @@ public class GitConstructionTest extends RepositoryTestCase { assertEquals(1, git.branchList().call().size()); git = Git.wrap(bareRepo); - assertEquals(2, git.branchList().setListMode(ListMode.ALL).call() + assertEquals(1, git.branchList().setListMode(ListMode.ALL).call() .size()); try { @@ -96,7 +96,7 @@ public class GitConstructionTest extends RepositoryTestCase { assertEquals(1, git.branchList().call().size()); git = Git.open(bareRepo.getDirectory()); - assertEquals(2, git.branchList().setListMode(ListMode.ALL).call() + assertEquals(1, git.branchList().setListMode(ListMode.ALL).call() .size()); git = Git.open(db.getWorkTree()); -- cgit v1.2.3 From c1525e2aa5e444a2a234de27d6b7189d5d7f715e Mon Sep 17 00:00:00 2001 From: Christian Halstrick Date: Tue, 24 May 2011 09:36:45 +0200 Subject: Make sure test repositories are closed Some repositories created during tests are not added to the 'toClose' list in LocalDiskRepositoryTestCase. Therefore when the tests end we may have open FileHandles and on Windows this may cause the tests to fail because we can't delete those files. This is fixed by adding the possibility to explicitly add repositories to the list of repos which are closed automatically. Change-Id: I1261baeef4c7d9aaedd7c34b546393bfa005bbcc Signed-off-by: Christian Halstrick --- .../org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java | 11 +++++++++++ .../tst/org/eclipse/jgit/api/CloneCommandTest.java | 9 +++++++++ .../tst/org/eclipse/jgit/api/GitConstructionTest.java | 1 + .../tst/org/eclipse/jgit/api/InitCommandTest.java | 2 ++ .../tst/org/eclipse/jgit/api/LsRemoteCommandTest.java | 4 ++++ 5 files changed, 27 insertions(+) (limited to 'org.eclipse.jgit.test/tst/org') diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java index 30d437540f..8ed51fb908 100644 --- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java +++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java @@ -302,6 +302,17 @@ public abstract class LocalDiskRepositoryTestCase { return db; } + /** + * Adds a repository to the list of repositories which is closed at the end + * of the tests + * + * @param r + * the repository to be closed + */ + public void addRepoToClose(Repository r) { + toClose.add(r); + } + /** * Creates a new unique directory for a test repository * diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java index 9d01291b7a..2e3345756d 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/CloneCommandTest.java @@ -101,6 +101,7 @@ public class CloneCommandTest extends RepositoryTestCase { command.setURI("file://" + git.getRepository().getWorkTree().getPath()); Git git2 = command.call(); + addRepoToClose(git2.getRepository()); assertNotNull(git2); ObjectId id = git2.getRepository().resolve("tag-for-blob"); assertNotNull(id); @@ -135,6 +136,8 @@ public class CloneCommandTest extends RepositoryTestCase { command.setURI("file://" + git.getRepository().getWorkTree().getPath()); Git git2 = command.call(); + addRepoToClose(git2.getRepository()); + assertNotNull(git2); assertEquals(git2.getRepository().getFullBranch(), "refs/heads/master"); @@ -152,6 +155,8 @@ public class CloneCommandTest extends RepositoryTestCase { + git.getRepository().getWorkTree().getPath()); command.setNoCheckout(true); git2 = command.call(); + addRepoToClose(git2.getRepository()); + assertNotNull(git2); assertEquals(git2.getRepository().getFullBranch(), "refs/heads/master"); @@ -169,6 +174,8 @@ public class CloneCommandTest extends RepositoryTestCase { + git.getRepository().getWorkTree().getPath()); command.setBare(true); git2 = command.call(); + addRepoToClose(git2.getRepository()); + assertNotNull(git2); assertEquals(git2.getRepository().getFullBranch(), "refs/heads/master"); @@ -191,6 +198,7 @@ public class CloneCommandTest extends RepositoryTestCase { command.setURI("file://" + git.getRepository().getWorkTree().getPath()); Git git2 = command.call(); + addRepoToClose(git2.getRepository()); assertNotNull(git2); assertEquals(git2.getRepository().getFullBranch(), "refs/heads/master"); @@ -209,6 +217,7 @@ public class CloneCommandTest extends RepositoryTestCase { + git.getRepository().getWorkTree().getPath()); command.setBare(true); git2 = command.call(); + addRepoToClose(git2.getRepository()); assertNotNull(git2); assertEquals(git2.getRepository().getFullBranch(), "refs/heads/master"); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/GitConstructionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/GitConstructionTest.java index b6ba4c927d..4d2b241dcb 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/GitConstructionTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/GitConstructionTest.java @@ -71,6 +71,7 @@ public class GitConstructionTest extends RepositoryTestCase { .setURI(db.getDirectory().toURI().toString()) .setDirectory(createUniqueTestGitDir(true)).call() .getRepository(); + addRepoToClose(bareRepo); } @Test diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/InitCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/InitCommandTest.java index 15aafc9060..28c54c269f 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/InitCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/InitCommandTest.java @@ -70,6 +70,7 @@ public class InitCommandTest extends RepositoryTestCase { InitCommand command = new InitCommand(); command.setDirectory(directory); Repository repository = command.call().getRepository(); + addRepoToClose(repository); assertNotNull(repository); } catch (Exception e) { fail(e.getMessage()); @@ -84,6 +85,7 @@ public class InitCommandTest extends RepositoryTestCase { command.setDirectory(directory); command.setBare(true); Repository repository = command.call().getRepository(); + addRepoToClose(repository); assertNotNull(repository); assertTrue(repository.isBare()); } catch (Exception e) { diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/LsRemoteCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/LsRemoteCommandTest.java index 81aa25f981..a4a5a2671b 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/LsRemoteCommandTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/LsRemoteCommandTest.java @@ -89,6 +89,8 @@ public class LsRemoteCommandTest extends RepositoryTestCase { + git.getRepository().getWorkTree().getPath()); command.setCloneAllBranches(true); Git git2 = command.call(); + addRepoToClose(git2.getRepository()); + LsRemoteCommand lsRemoteCommand = git2.lsRemote(); Collection refs = lsRemoteCommand.call(); @@ -109,6 +111,7 @@ public class LsRemoteCommandTest extends RepositoryTestCase { + git.getRepository().getWorkTree().getPath()); command.setCloneAllBranches(true); Git git2 = command.call(); + addRepoToClose(git2.getRepository()); LsRemoteCommand lsRemoteCommand = git2.lsRemote(); lsRemoteCommand.setTags(true); @@ -130,6 +133,7 @@ public class LsRemoteCommandTest extends RepositoryTestCase { + git.getRepository().getWorkTree().getPath()); command.setCloneAllBranches(true); Git git2 = command.call(); + addRepoToClose(git2.getRepository()); LsRemoteCommand lsRemoteCommand = git2.lsRemote(); lsRemoteCommand.setHeads(true); -- cgit v1.2.3 From 67a1a0993f3969357c554b2030cfd794b3c3af89 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Fri, 27 May 2011 12:04:19 -0700 Subject: Ensure the HTTP request is fully consumed Some servlet containers require the servlet to read the EOF marker from the input stream before a response can be output if the stream is using "Transfer-Encoding: chunked"... which is typical for any sort of large push to a repository over smart HTTP. Ensure the EOF is always read by the PackParser when it is handling the stream, and fail fast if there is more data present than expected since this does indicate a protocol error. Also ensure the EOF is read by UploadPack before it starts to output a partial response using packing progress meters. Change-Id: I131db9dea20b2324cb7c3272a814f21296bc64bd Signed-off-by: Shawn O. Pearce --- .../org/eclipse/jgit/transport/PackParserTest.java | 32 +++++++++++++++++++ .../src/org/eclipse/jgit/transport/PackParser.java | 36 ++++++++++++++++++++++ .../org/eclipse/jgit/transport/ReceivePack.java | 1 + .../src/org/eclipse/jgit/transport/UploadPack.java | 11 +++++++ 4 files changed, 80 insertions(+) (limited to 'org.eclipse.jgit.test/tst/org') diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PackParserTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PackParserTest.java index df89674e6f..719cc66671 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PackParserTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PackParserTest.java @@ -46,7 +46,9 @@ package org.eclipse.jgit.transport; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; import java.io.File; @@ -54,8 +56,10 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.security.MessageDigest; +import java.text.MessageFormat; import java.util.zip.Deflater; +import org.eclipse.jgit.JGitText; import org.eclipse.jgit.junit.JGitTestUtil; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Constants; @@ -69,6 +73,7 @@ import org.eclipse.jgit.storage.file.ObjectDirectoryPackParser; import org.eclipse.jgit.storage.file.PackFile; import org.eclipse.jgit.util.NB; import org.eclipse.jgit.util.TemporaryBuffer; +import org.eclipse.jgit.util.io.UnionInputStream; import org.junit.After; import org.junit.Test; @@ -177,6 +182,33 @@ public class PackParserTest extends RepositoryTestCase { p.parse(NullProgressMonitor.INSTANCE); } + @Test + public void testPackWithTrailingGarbage() throws Exception { + TestRepository d = new TestRepository(db); + RevBlob a = d.blob("a"); + + TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(1024); + packHeader(pack, 1); + pack.write((Constants.OBJ_REF_DELTA) << 4 | 4); + a.copyRawTo(pack); + deflate(pack, new byte[] { 0x1, 0x1, 0x1, 'b' }); + digest(pack); + + PackParser p = index(new UnionInputStream( + new ByteArrayInputStream(pack.toByteArray()), + new ByteArrayInputStream(new byte[] { 0x7e }))); + p.setAllowThin(true); + p.setCheckEofAfterPackFooter(true); + try { + p.parse(NullProgressMonitor.INSTANCE); + fail("Pack with trailing garbage was accepted"); + } catch (IOException err) { + assertEquals( + MessageFormat.format(JGitText.get().expectedEOFReceived, "\\x73"), + err.getMessage()); + } + } + private void packHeader(TemporaryBuffer.Heap tinyPack, int cnt) throws IOException { final byte[] hdr = new byte[8]; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java index 6f0c6c3b36..4bbe3a0048 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PackParser.java @@ -136,6 +136,8 @@ public abstract class PackParser { private boolean needBaseObjectIds; + private boolean checkEofAfterPackFooter; + private long objectCount; private PackedObjectInfo[] entries; @@ -282,6 +284,21 @@ public abstract class PackParser { this.needBaseObjectIds = b; } + /** @return true if the EOF should be read from the input after the footer. */ + public boolean isCheckEofAfterPackFooter() { + return checkEofAfterPackFooter; + } + + /** + * Ensure EOF is read from the input stream after the footer. + * + * @param b + * true if the EOF should be read; false if it is not checked. + */ + public void setCheckEofAfterPackFooter(boolean b) { + checkEofAfterPackFooter = b; + } + /** @return the new objects that were sent by the user */ public ObjectIdSubclassMap getNewObjectIds() { if (newObjectIds != null) @@ -782,6 +799,25 @@ public abstract class PackParser { System.arraycopy(buf, c, srcHash, 0, 20); use(20); + // The input stream should be at EOF at this point. We do not support + // yielding back any remaining buffered data after the pack footer, so + // protocols that embed a pack stream are required to either end their + // stream with the pack, or embed the pack with a framing system like + // the SideBandInputStream does. + + if (bAvail != 0) + throw new CorruptObjectException(MessageFormat.format( + JGitText.get().expectedEOFReceived, + "\\x" + Integer.toHexString(buf[bOffset] & 0xff))); + + if (isCheckEofAfterPackFooter()) { + int eof = in.read(); + if (0 <= eof) + throw new CorruptObjectException(MessageFormat.format( + JGitText.get().expectedEOFReceived, + "\\x" + Integer.toHexString(eof))); + } + if (!Arrays.equals(actHash, srcHash)) throw new CorruptObjectException( JGitText.get().corruptObjectPackfileChecksumIncorrect); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java index e2ab5f6780..f1f9b0f44d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java @@ -825,6 +825,7 @@ public class ReceivePack { parser.setAllowThin(true); parser.setNeedNewObjectIds(checkReferencedIsReachable); parser.setNeedBaseObjectIds(checkReferencedIsReachable); + parser.setCheckEofAfterPackFooter(!biDirectionalPipe); parser.setObjectChecking(isCheckReceivedObjects()); parser.setLockMessage(lockMsg); packLock = parser.parse(receiving, resolving); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java index 50f57130c3..0e50b937b6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -55,6 +55,7 @@ import java.util.Map; import java.util.Set; import org.eclipse.jgit.JGitText; +import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.PackProtocolException; import org.eclipse.jgit.lib.Constants; @@ -757,6 +758,16 @@ public class UploadPack { final boolean sideband = options.contains(OPTION_SIDE_BAND) || options.contains(OPTION_SIDE_BAND_64K); + if (!biDirectionalPipe) { + // Ensure the request was fully consumed. Any remaining input must + // be a protocol error. If we aren't at EOF the implementation is broken. + int eof = rawIn.read(); + if (0 <= eof) + throw new CorruptObjectException(MessageFormat.format( + JGitText.get().expectedEOFReceived, + "\\x" + Integer.toHexString(eof))); + } + ProgressMonitor pm = NullProgressMonitor.INSTANCE; OutputStream packOut = rawOut; SideBandOutputStream msgOut = null; -- cgit v1.2.3