Deprecate pushing to refs/changes
This commit prevents pushing to refs/changes. It is still possible to
allow push to refs/changes with the config flag
receive.allowPushToRefsChanges
Change-Id: I0f45589a1a1a9fee93f1718a13d01c414b3f67a8
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 1549ad7..be4316e 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3533,6 +3533,17 @@
If no groups are added, any user will be allowed to execute
'receive-pack' on the server.
+[[receive.allowPushToRefsChanges]]receive.allowPushToRefsChanges::
++
+If true, it is possible to push directly to a change using `refs/changes/`.
+The possibility to push to `refs/changes/` is deprecated and it might be
+removed in future releases.
+See link:user-upload.html#manual_replacement_mapping[Manual Replacement Mapping].
++
+False means pushing to `refs/changes/` is prohibited.
++
+Defaults to false.
+
[[receive.certNonceSeed]]receive.certNonceSeed::
+
If set to a non-empty value and server-side signed push validation is
diff --git a/Documentation/error-messages.txt b/Documentation/error-messages.txt
index ca8dc75..37eb1f6 100644
--- a/Documentation/error-messages.txt
+++ b/Documentation/error-messages.txt
@@ -34,6 +34,7 @@
* link:error-same-change-id-in-multiple-changes.html[same Change-Id in multiple changes]
* link:error-too-many-commits.html[too many commits]
* link:error-upload-denied.html[Upload denied for project \'...']
+* link:error-push-refschanges-not-allowed.html[upload to refs/changes not allowed]
* link:error-not-allowed-to-upload-merges.html[you are not allowed to upload merges]
diff --git a/Documentation/error-push-refschanges-not-allowed.txt b/Documentation/error-push-refschanges-not-allowed.txt
new file mode 100644
index 0000000..2bbdc3e
--- /dev/null
+++ b/Documentation/error-push-refschanges-not-allowed.txt
@@ -0,0 +1,13 @@
+= upload to refs/changes not allowed
+
+Pushing to `refs/changes/` is deprecated and is not allowed on this Gerrit server.
+See the documentation for link:user-upload.html#push_create[creating changes] for
+alternate ways to push to existing changes.
+
+
+GERRIT
+------
+Part of link:error-messages.html[Gerrit Error Messages]
+
+SEARCHBOX
+---------
diff --git a/Documentation/intro-user.txt b/Documentation/intro-user.txt
index b20c2c0..29e02f0 100644
--- a/Documentation/intro-user.txt
+++ b/Documentation/intro-user.txt
@@ -184,9 +184,7 @@
accidentally creating a new change instead of uploading a new patch
set. Any push without Change-Id then fails with
link:error-missing-changeid.html[missing Change-Id in commit message
-footer]. New patch sets can always be uploaded to a specific change
-(even without any Change-Id) by pushing to the change ref, e.g.
-`refs/changes/74/67374`.
+footer].
Amending and rebasing a commit preserves the Change-Id so that the new
commit automatically becomes a new patch set of the existing change,
diff --git a/Documentation/user-upload.txt b/Documentation/user-upload.txt
index b5579e4..9be39a6 100644
--- a/Documentation/user-upload.txt
+++ b/Documentation/user-upload.txt
@@ -431,6 +431,11 @@
during normal uploads.
See above for the preferred technique of replacing changes.
+
+Pushing directly to `refs/changes/` is deprecated. If you see the error
+message 'upload to refs/changes not allowed', it means that pushing directly
+to `refs/changes` is disabled on the Gerrit server and the below section does
+not apply to you.
--
To add an additional patch set to a change, replacing it with an
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index d2686d1..10d85d3 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -99,6 +99,7 @@
import com.google.gerrit.server.change.SetHashtagsOp;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.CanonicalWebUrl;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.ProjectConfigEntry;
import com.google.gerrit.server.edit.ChangeEdit;
@@ -187,6 +188,7 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
@@ -349,6 +351,7 @@
private final ReceivePack rp;
// Immutable fields derived from constructor arguments.
+ private final boolean allowPushToRefsChanges;
private final LabelTypes labelTypes;
private final NoteMap rejectCommits;
private final PermissionBackend.ForProject permissions;
@@ -397,6 +400,7 @@
AccountsUpdate.Server accountsUpdate,
AllProjectsName allProjectsName,
BatchUpdate.Factory batchUpdateFactory,
+ @GerritServerConfig Config cfg,
ChangeEditUtil editUtil,
ChangeIndexer indexer,
ChangeInserter.Factory changeInserterFactory,
@@ -480,6 +484,7 @@
this.rp = rp;
// Immutable fields derived from constructor arguments.
+ allowPushToRefsChanges = cfg.getBoolean("receive", "allowPushToRefsChanges", false);
repo = rp.getRepository();
project = projectState.getProject();
labelTypes = projectState.getLabelTypes();
@@ -861,10 +866,14 @@
Matcher m = NEW_PATCHSET_PATTERN.matcher(cmd.getRefName());
if (m.matches()) {
- // The referenced change must exist and must still be open.
- //
- Change.Id changeId = Change.Id.parse(m.group(1));
- parseReplaceCommand(cmd, changeId);
+ if (allowPushToRefsChanges) {
+ // The referenced change must exist and must still be open.
+ //
+ Change.Id changeId = Change.Id.parse(m.group(1));
+ parseReplaceCommand(cmd, changeId);
+ } else {
+ reject(cmd, "upload to refs/changes not allowed");
+ }
continue;
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index f7ca2f2..d95d2c1 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -955,7 +955,26 @@
}
@Test
+ @GerritConfig(name = "receive.allowPushToRefsChanges", value = "true")
+ public void pushToRefsChangesAllowed() throws Exception {
+ PushOneCommit.Result r = pushOneCommitToRefsChanges();
+ r.assertOkStatus();
+ }
+
+ @Test
public void pushNewPatchsetToRefsChanges() throws Exception {
+ PushOneCommit.Result r = pushOneCommitToRefsChanges();
+ r.assertErrorStatus("upload to refs/changes not allowed");
+ }
+
+ @Test
+ @GerritConfig(name = "receive.allowPushToRefsChanges", value = "false")
+ public void pushToRefsChangesNotAllowed() throws Exception {
+ PushOneCommit.Result r = pushOneCommitToRefsChanges();
+ r.assertErrorStatus("upload to refs/changes not allowed");
+ }
+
+ private PushOneCommit.Result pushOneCommitToRefsChanges() throws Exception {
PushOneCommit.Result r = pushTo("refs/for/master");
r.assertOkStatus();
PushOneCommit push =
@@ -967,8 +986,7 @@
"b.txt",
"anotherContent",
r.getChangeId());
- r = push.to("refs/changes/" + r.getChange().change().getId().get());
- r.assertOkStatus();
+ return push.to("refs/changes/" + r.getChange().change().getId().get());
}
@Test
@@ -1323,6 +1341,7 @@
}
@Test
+ @GerritConfig(name = "receive.allowPushToRefsChanges", value = "true")
public void testPushWithChangedChangeId() throws Exception {
PushOneCommit.Result r = pushTo("refs/for/master");
r.assertOkStatus();
@@ -1523,6 +1542,7 @@
}
@Test
+ @GerritConfig(name = "receive.allowPushToRefsChanges", value = "true")
public void accidentallyPushNewPatchSetDirectlyToBranchAndRecoverByPushingToRefsChanges()
throws Exception {
Change.Id id = accidentallyPushNewPatchSetDirectlyToBranch();
