From f5fa1eaf399275e9cc4a6418ecadfa04c10060b6 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Tue, 2 Oct 2018 15:18:43 -0700 Subject: [PATCH] Throw error when deepen-since excludes all commits In C Git, when a client fetches with "git fetch --shallow-since= origin ", and all commits reachable from are older than , the server dies with a message "no commits selected for shallow requests". That is, (1) the --shallow-since filter applies to the commit pointed to by the ref itself, and (2) there is a check that at least one commit is not filtered out. (The pack-protocol.txt documentation does not describe this, but the C implementation does this.) The implementation in commit 1bb430dc21 ("UploadPack: support deepen-since in protocol v2", 2018-09-27) does neither (1) nor (2), so do both of these. Change-Id: I9946327a71627626ecce34ca2d017d2add8867fc Signed-off-by: Jonathan Tan --- .../jgit/transport/UploadPackTest.java | 20 +++++++++++++++++++ .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../eclipse/jgit/revwalk/DepthGenerator.java | 4 ++++ .../eclipse/jgit/transport/UploadPack.java | 6 ++++++ 5 files changed, 32 insertions(+) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java index b74dcca58..ad3eb2e32 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java @@ -1279,6 +1279,26 @@ public class UploadPackTest { assertTrue(client.hasObject(merge.toObjectId())); } + @Test + public void testV2FetchShallowSince_noCommitsSelected() throws Exception { + PersonIdent person = new PersonIdent(remote.getRepository()); + + RevCommit tooOld = remote.commit() + .committer(new PersonIdent(person, 1500000000, 0)).create(); + + remote.update("branch1", tooOld); + + thrown.expect(PackProtocolException.class); + thrown.expectMessage("No commits selected for shallow request"); + uploadPackV2( + "command=fetch\n", + PacketLineIn.DELIM, + "deepen-since 1510000\n", + "want " + tooOld.toObjectId().getName() + "\n", + "done\n", + PacketLineIn.END); + } + @Test public void testV2FetchUnrecognizedArgument() throws Exception { thrown.expect(PackProtocolException.class); 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 b3f7b89c9..311d68b39 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -467,6 +467,7 @@ newIdMustNotBeNull=New ID must not be null newlineInQuotesNotAllowed=Newline in quotes not allowed noApplyInDelete=No apply in delete noClosingBracket=No closing {0} found for {1} at index {2}. +noCommitsSelectedForShallow=No commits selected for shallow request noCredentialsProvider=Authentication is required but no CredentialsProvider has been registered noHEADExistsAndNoExplicitStartingRevisionWasSpecified=No HEAD exists and no explicit starting revision was specified noHMACsupport=No {0} support: {1} 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 9f0e3e0c2..4479525dd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -528,6 +528,7 @@ public class JGitText extends TranslationBundle { /***/ public String newlineInQuotesNotAllowed; /***/ public String noApplyInDelete; /***/ public String noClosingBracket; + /***/ public String noCommitsSelectedForShallow; /***/ public String noCredentialsProvider; /***/ public String noHEADExistsAndNoExplicitStartingRevisionWasSpecified; /***/ public String noHMACsupport; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DepthGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DepthGenerator.java index 6c6cc95c2..ef2575858 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DepthGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/DepthGenerator.java @@ -135,6 +135,10 @@ class DepthGenerator extends Generator { if ((c.flags & RevWalk.PARSED) == 0) c.parseHeaders(walk); + if (c.getCommitTime() < deepenSince) { + continue; + } + int newDepth = c.depth + 1; for (RevCommit p : c.parents) { 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 b69f2cd92..a2174f641 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java @@ -1168,8 +1168,10 @@ public class UploadPack { } RevCommit o; + boolean atLeastOne = false; while ((o = depthWalk.next()) != null) { DepthWalk.Commit c = (DepthWalk.Commit) o; + atLeastOne = true; boolean isBoundary = (c.getDepth() == walkDepth) || c.isBoundary(); @@ -1185,6 +1187,10 @@ public class UploadPack { unshallowFunc.accept(c.copy()); } } + if (!atLeastOne) { + throw new PackProtocolException( + JGitText.get().noCommitsSelectedForShallow); + } } }