ReceivePack: reject creation of one level refs such as refs/main
Cgit describes valid reference names in the `git-check-ref-format(1)`
manual page which lists the constraints. Excluding the `refs/` prefix,
the name must contain at least one slash to enforce a category such as
`heads/`, `tags/`.
On Gerrit, jGit accepts a user pushing to `refs/main` but when
replicating the reference using ssh and cgit, receive-pack on the remote
rejects it with:
Error: Failed replicate of refs/main to X, reason: funny refname
It is a special case in cgit `builtin/receive-pack.c`:
--------8<-----------8<-----------8<-----------8<-----------8<-----------
static const char *update(struct command *cmd, struct shallow_info *si)
{
...
/* only refs/... are allowed */
if (!starts_with(name, "refs/") || check_refname_format(name +
5, 0)) {
rp_error("refusing to create funny ref '%s' remotely",
name);
return "funny refname";
}
--------8<-----------8<-----------8<-----------8<-----------8<-----------
So that if the part coming after `refs/` has a single component
(`master`) it is rejected.
Bug: 582000
Change-Id: Ica2bdc58e81459f08cfe42c2961a3c06622c30df
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryResolveTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryResolveTest.java
index 7f1dfc3..2773715 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryResolveTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RepositoryResolveTest.java
@@ -277,6 +277,7 @@ public void invalidNames() throws AmbiguousObjectException, IOException {
assertTrue(Repository.isValidRefName("x/a]b")); // odd, yes..
assertTrue(Repository.isValidRefName("x/\u00a0")); // unicode is fine,
// even hard space
+ assertFalse(Repository.isValidRefName("single-component"));
assertFalse(Repository.isValidRefName("x/.a"));
assertFalse(Repository.isValidRefName("x/a."));
assertFalse(Repository.isValidRefName("x/a..b"));
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackTest.java
index 595b970..bcc7082 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackTest.java
@@ -42,22 +42,57 @@
package org.eclipse.jgit.transport;
+import java.util.ArrayList;
+
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
-
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import org.eclipse.jgit.errors.PackProtocolException;
import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.junit.TestRepository.BranchBuilder;
+import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.transport.ReceiveCommand.Result;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.After;
+import org.junit.Before;
import org.junit.Test;
/** Tests for receive-pack utilities. */
public class ReceivePackTest {
+ private URIish uri;
+ private TestProtocol<Object> testProtocol;
+ private InMemoryRepository client;
+ private InMemoryRepository server;
+ private TestRepository clientTestRepo;
+ private TestRepository serverTestRepo;
+
+ @Before
+ public void setUp() throws Exception {
+ client = new InMemoryRepository(new DfsRepositoryDescription("client"));
+ server = new InMemoryRepository(new DfsRepositoryDescription("server"));
+ clientTestRepo = new TestRepository<>(client);
+ serverTestRepo = new TestRepository<>(server);
+
+ testProtocol = new TestProtocol<>(null, (Object req, Repository db) -> {
+ ReceivePack rp = new ReceivePack(db);
+ return rp;
+ });
+ uri = testProtocol.register(new Object(), server);
+ }
+
+ @After
+ public void tearDown() {
+ Transport.unregister(testProtocol);
+ }
+
@Test
public void parseCommand() throws Exception {
String o = "0000000000000000000000000000000000000000";
@@ -81,6 +116,54 @@ public void parseCommand() throws Exception {
assertParseCommandFails(o + " " + "X" + n.substring(1) + " " + r);
}
+ /**
+ * Ensure receive-pack rejects a push to a one level ref (refs/master).
+ */
+ @Test
+ public void rejectsCreationOfOneLevelRef() throws Exception {
+ // Create refs/master on the client so we can attempt to push it
+ RevCommit clientCommit;
+ BranchBuilder bb = clientTestRepo.branch("refs/master");
+ clientCommit = bb.commit().noFiles().message("Testing").create();
+
+ ArrayList<RemoteRefUpdate> refUpdates = new ArrayList<>();
+ refUpdates.add(new RemoteRefUpdate(
+ clientTestRepo.getRepository(), "refs/master", clientCommit,
+ "refs/master", false, /* force update */
+ null, /* no local tracking ref */
+ ObjectId.zeroId() // expected advertisement
+ ));
+ Transport tn = testProtocol.open(uri, client, "server");
+ PushResult result = tn.push(NullProgressMonitor.INSTANCE, refUpdates);
+ RemoteRefUpdate update = result.getRemoteUpdate("refs/master");
+
+ assertEquals(RemoteRefUpdate.Status.REJECTED_OTHER_REASON,
+ update.getStatus());
+ assertEquals(JGitText.get().funnyRefname, update.getMessage());
+ }
+
+ /**
+ * Allow deletion of one level refs (refs/master).
+ */
+ @Test
+ public void acceptsDeletionOfOneLevelRef() throws Exception {
+ BranchBuilder bb = serverTestRepo.branch("refs/master");
+ bb.update(bb.commit().noFiles().message("Testing").create());
+
+ ArrayList<RemoteRefUpdate> refUpdates = new ArrayList<>();
+ refUpdates.add(new RemoteRefUpdate(null, null, ObjectId.zeroId(),
+ "refs/master", false, // force update
+ null, // no local tracking ref
+ null // expected advertisement
+ ));
+ Transport tn = testProtocol.open(uri, client, "server");
+ PushResult result = tn.push(NullProgressMonitor.INSTANCE, refUpdates);
+ RemoteRefUpdate update = result.getRemoteUpdate("refs/master");
+
+ assertEquals(RemoteRefUpdate.Status.OK, update.getStatus());
+ assertEquals(null, update.getMessage());
+ }
+
private void assertParseCommandFails(String input) {
try {
ReceivePack.parseCommand(input);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
index c9dc6da..01a4361 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
@@ -1375,6 +1375,8 @@ public RepositoryState getRepositoryState() {
*
* For portability reasons '\' is excluded
*
+ * @see <a href="http://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html"
+ * >git-check-ref-format(1) manual page</a>
* @param refName a {@link java.lang.String} object.
* @return true if refName is a valid ref name
*/
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 8b05d41..ec7d4c1 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
@@ -1658,6 +1658,18 @@ private void validateCommands() {
continue;
}
+ if (cmd.getRefName().startsWith(Constants.R_REFS)
+ && !Repository.isValidRefName(
+ cmd.getRefName().substring(Constants.R_REFS.length())
+ )
+ ) {
+ // Reject the creation of one level references such as
+ // refs/master to match cgit implementation.
+ cmd.setResult(Result.REJECTED_OTHER_REASON,
+ JGitText.get().funnyRefname);
+ continue;
+ }
+
if (ref != null) {
// A well behaved client shouldn't have sent us a
// create command for a ref we advertised to it.
@@ -1753,8 +1765,18 @@ private void validateCommands() {
}
}
- if (!cmd.getRefName().startsWith(Constants.R_REFS)
- || !Repository.isValidRefName(cmd.getRefName())) {
+ if (!cmd.getRefName().startsWith(Constants.R_REFS)) {
+ cmd.setResult(Result.REJECTED_OTHER_REASON,
+ JGitText.get().funnyRefname);
+ }
+
+ if (!Repository.isValidRefName(cmd.getRefName())
+ // But accept deletion of one level refs (refs/main)
+ && !(
+ cmd.getType() == ReceiveCommand.Type.DELETE
+ && Repository.isValidRefName(cmd.getRefName().substring(Constants.R_REFS.length()))
+ )
+ ) {
cmd.setResult(Result.REJECTED_OTHER_REASON,
JGitText.get().funnyRefname);
}
