diff --git a/ReleaseNotes/ReleaseNotes-2.10.7.txt b/ReleaseNotes/ReleaseNotes-2.10.7.txt new file mode 100644 index 0000000000..28cf37bb0b --- /dev/null +++ b/ReleaseNotes/ReleaseNotes-2.10.7.txt @@ -0,0 +1,19 @@ +Release notes for Gerrit 2.10.7 +=============================== + +There are no schema changes from link:ReleaseNotes-2.10.6.html[2.10.6]. + +Download: +link:https://gerrit-releases.storage.googleapis.com/gerrit-2.10.7.war[ +https://gerrit-releases.storage.googleapis.com/gerrit-2.10.7.war] + +Bug Fixes +--------- + +* link:https://code.google.com/p/gerrit/issues/detail?id=3361[Issue 3361]: +Synchronize Myers diff and Histogram diff invocations to prevent pack file +corruption. ++ +See also the link:https://bugs.eclipse.org/bugs/show_bug.cgi?id=467467[ +bug report on JGit]. + diff --git a/ReleaseNotes/ReleaseNotes-2.11.4.txt b/ReleaseNotes/ReleaseNotes-2.11.4.txt new file mode 100644 index 0000000000..fecd6a62b8 --- /dev/null +++ b/ReleaseNotes/ReleaseNotes-2.11.4.txt @@ -0,0 +1,111 @@ +Release notes for Gerrit 2.11.4 +=============================== + +Gerrit 2.11.4 is now available: + +link:https://gerrit-releases.storage.googleapis.com/gerrit-2.11.4.war[ +https://gerrit-releases.storage.googleapis.com/gerrit-2.11.4.war] + +Gerrit 2.11.4 includes the bug fixes done with +link:ReleaseNotes-2.10.7.html[Gerrit 2.10.7]. These bug fixes are *not* listed +in these release notes. + +There are no schema changes from link:ReleaseNotes-2.11.3.html[2.11.3]. + + +Bug Fixes +--------- + +* Fix NullPointerException in `ls-project` command with `--has-acl-for` option. ++ +Using the `--has-acl-for` option for external groups (e.g. LDAP groups) was +causing a NullPointerException. + +* link:https://code.google.com/p/gerrit/issues/detail?id=3328[Issue 3328]: +Allow to push a tag that points to a non-commit object. ++ +When pushing a tag that points to a non-commit object, like +link:https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tag/?id=v2.6.11[ +`v2.6.11` on linux-stable] which points to a tree, or +link:https://git.eclipse.org/c/jgit/jgit.git/tag/?id=spearce-gpg-pub[ +`spearce-gpg-pub` on jgit] which points to a blob, Gerrit rejected the push with +the error message 'missing object(s)'. ++ +Note: This was previously fixed in Gerrit version 2.11.1, but was inadvertently +reverted in 2.11.2 and 2.11.3. + +* link:https://code.google.com/p/gerrit/issues/detail?id=2817[Issue 2817]: +Insert Change-Id into access right changes. ++ +When modifications of access rights were saved for review, the change +did not have a Change-Id in the commit message. + +* Fix duplicated log lines after reloading a plugin. ++ +If a plugin was reloaded, logs emitted from the plugin were duplicated. + +* Remove `--recheck-mergeable` option from `reindex` command documentation. ++ +The `--recheck-mergeable` option was removed in Gerrit version 2.11. + +* Use the correct validation policy for commits created by Gerrit. ++ +Commits created by Gerrit were being validated in the same way as commits +received from users. + +* link:https://code.google.com/p/gerrit/issues/detail?id=3557[Issue 3557]: +Disallow invalid reference patterns in project configuration. ++ +When editing a project configuration by using the UI or by submitting a change +to `refs/meta/config`, it was possible to add a permission to an invalid +reference pattern. This caused the project to be unavailable and the `ls-projects` +command to fail whenever this project was encountered. + +* link:https://code.google.com/p/gerrit/issues/detail?id=3574[Issue 3574]: +Fix review labels with `AnyWithBlock` function. ++ +The review labels with `AnyWithBlock` with 0 and +1 values blocked submit when +reviewers were added. + +* Fix ref in tag list for signed/annotated tags. ++ +The tag name from the header was used, rather than the ref name. In some cases +this resulted in the wrong tag ref being listed. + +* singleusergroup plugin: enable to add a user within a project's ACL using `user/username`. ++ +A user could not be added to a project's ACL unless the user already had READ +permission in the project's ACL. + +* Prevent user from bypassing ref-update hook through gerrit-created commits. ++ +If the user used the cherry-pick ability in the UI or via the REST API, they +could put a commit on a branch that bypassed the requirements of the ref-update +hook (such as that certain branches require QA-tickets to be referenced in the +commit message). + +* Allow InternalUsers to see drafts. ++ +According to the documentation, InternalUsers should have full read access. +This was not true, since InternalUsers could not see drafts. + +* link:https://code.google.com/p/gerrit/issues/detail?id=2683[Issue 2683]: +Fix non-ASCII password authentication failure under tomcat (LDAP). ++ +The authentication with LDAP failed when the password contained non-ASCII +characters such as ä, ö, Ä, and Ö. + +* Do not double decode the login URL token. ++ +The login URL token used to redirect from the login servlet to the target page +is already decoded and should not be decoded again. + +* Include approvals from magic branch in change message. ++ +When using the `%l` option to apply a review label on uploaded changes or +patch sets, the applied label was not mentioned in the change message. + +* Fire the `comment-added` hook for approvals from the magic branch. ++ +When using the `%l` option to apply a review label on uploaded changes or +patch sets, the `comment-added` hook was not being fired. diff --git a/ReleaseNotes/index.txt b/ReleaseNotes/index.txt index 262dc4f786..e1831adb32 100644 --- a/ReleaseNotes/index.txt +++ b/ReleaseNotes/index.txt @@ -4,6 +4,7 @@ Gerrit Code Review - Release Notes [[2_11]] Version 2.11.x -------------- +* link:ReleaseNotes-2.11.4.html[2.11.4] * link:ReleaseNotes-2.11.3.html[2.11.3] * link:ReleaseNotes-2.11.2.html[2.11.2] * link:ReleaseNotes-2.11.1.html[2.11.1] @@ -12,6 +13,7 @@ Version 2.11.x [[2_10]] Version 2.10.x -------------- +* link:ReleaseNotes-2.10.7.html[2.10.7] * link:ReleaseNotes-2.10.6.html[2.10.6] * link:ReleaseNotes-2.10.5.html[2.10.5] * link:ReleaseNotes-2.10.4.html[2.10.4] diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java index 080b767240..ab1b90dc4e 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java @@ -16,8 +16,11 @@ package com.google.gerrit.acceptance.git; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.TruthJUnit.assume; +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.concurrent.TimeUnit.SECONDS; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.PushOneCommit; @@ -32,10 +35,16 @@ import com.google.gerrit.testutil.ConfigSuite; import com.google.inject.Inject; import org.eclipse.jgit.lib.Config; +import org.joda.time.DateTime; +import org.joda.time.DateTimeUtils; +import org.joda.time.DateTimeUtils.MillisProvider; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; import java.util.Set; +import java.util.concurrent.atomic.AtomicLong; public abstract class AbstractPushForReview extends AbstractDaemonTest { @ConfigSuite.Config @@ -53,6 +62,24 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { private String sshUrl; + @BeforeClass + public static void setTimeForTesting() { + final long clockStepMs = MILLISECONDS.convert(1, SECONDS); + final AtomicLong clockMs = new AtomicLong( + new DateTime(2009, 9, 30, 17, 0, 0).getMillis()); + DateTimeUtils.setCurrentMillisProvider(new MillisProvider() { + @Override + public long getMillis() { + return clockMs.getAndAdd(clockStepMs); + } + }); + } + + @AfterClass + public static void restoreTime() { + DateTimeUtils.setCurrentMillisSystem(); + } + @Before public void setUp() throws Exception { sshUrl = sshSession.getUrl(); @@ -177,6 +204,8 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { assertThat(cr.all).hasSize(1); assertThat(cr.all.get(0).name).isEqualTo("Administrator"); assertThat(cr.all.get(0).value).isEqualTo(1); + assertThat(Iterables.getLast(ci.messages).message).isEqualTo( + "Uploaded patch set 1: Code-Review+1."); PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo, PushOneCommit.SUBJECT, @@ -185,9 +214,20 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { ci = get(r.getChangeId()); cr = ci.labels.get("Code-Review"); + assertThat(Iterables.getLast(ci.messages).message).isEqualTo( + "Uploaded patch set 2: Code-Review+2."); + assertThat(cr.all).hasSize(1); assertThat(cr.all.get(0).name).isEqualTo("Administrator"); assertThat(cr.all.get(0).value).isEqualTo(2); + + push = + pushFactory.create(db, admin.getIdent(), testRepo, PushOneCommit.SUBJECT, + "c.txt", "moreContent", r.getChangeId()); + r = push.to("refs/for/master/%l=Code-Review+2"); + ci = get(r.getChangeId()); + assertThat(Iterables.getLast(ci.messages).message).isEqualTo( + "Uploaded patch set 3."); } @Test diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK index 446a183d55..6caa0f2b33 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/BUCK @@ -22,7 +22,10 @@ acceptance_tests( java_library( name = 'push_for_review', srcs = ['AbstractPushForReview.java'], - deps = ['//gerrit-acceptance-tests:lib'], + deps = [ + '//gerrit-acceptance-tests:lib', + '//lib/joda:joda-time', + ], ) java_library( diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/LoginUrlToken.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/LoginUrlToken.java index 6c17c87787..177ff048b1 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/LoginUrlToken.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/LoginUrlToken.java @@ -17,7 +17,6 @@ package com.google.gerrit.httpd; import com.google.common.base.CharMatcher; import com.google.common.base.Strings; import com.google.gerrit.common.PageLinks; -import com.google.gerrit.extensions.restapi.Url; import javax.servlet.http.HttpServletRequest; @@ -25,11 +24,11 @@ public class LoginUrlToken { private static final String DEFAULT_TOKEN = '#' + PageLinks.MINE; public static String getToken(final HttpServletRequest req){ - String encodedToken = req.getPathInfo(); - if (Strings.isNullOrEmpty(encodedToken)) { + String token = req.getPathInfo(); + if (Strings.isNullOrEmpty(token)) { return DEFAULT_TOKEN; } else { - return CharMatcher.is('/').trimLeadingFrom(Url.decode(encodedToken)); + return CharMatcher.is('/').trimLeadingFrom(token); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index d60d2c2b2c..e0d89ae08a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -341,6 +341,12 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { accountCache.get(change.getOwner()).getAccount(), hashtags, null, hashtags, db); } + + if (approvals != null && !approvals.isEmpty()) { + hooks.doCommentAddedHook(change, + ((IdentifiedUser) refControl.getCurrentUser()).getAccount(), + patchSet, null, approvals, db); + } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 8aa2bf88bf..032d290f82 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -71,6 +71,7 @@ import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetInfo; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; @@ -1761,6 +1762,8 @@ public class ReceiveCommits { } recipients.add(getRecipientsFromFooters(accountResolver, ps, footerLines)); recipients.remove(me); + StringBuilder msgs = renderMessageWithApprovals(ps.getPatchSetId(), + approvals, Collections.emptyMap()); try (ObjectInserter oi = repo.newObjectInserter(); BatchUpdate bu = batchUpdateFactory.create( db, change.getProject(), currentUser, change.getCreatedOn())) { @@ -1769,7 +1772,7 @@ public class ReceiveCommits { .setReviewers(recipients.getReviewers()) .setExtraCC(recipients.getCcOnly()) .setApprovals(approvals) - .setMessage("Uploaded patch set " + ps.getPatchSetId() + ".") + .setMessage(msgs.toString() + ".") .setRequestScopePropagator(requestScopePropagator) .setSendMail(true) .setUpdateRef(false)); @@ -1881,6 +1884,27 @@ public class ReceiveCommits { } } + private StringBuilder renderMessageWithApprovals(int patchSetId, + Map n, Map c) { + StringBuilder msgs = new StringBuilder("Uploaded patch set " + patchSetId); + if (!n.isEmpty()) { + boolean first = true; + for (Map.Entry e : n.entrySet()) { + if (c.containsKey(e.getKey()) + && c.get(e.getKey()).getValue() == e.getValue()) { + continue; + } + if (first) { + msgs.append(":"); + first = false; + } + msgs.append(" ") + .append(LabelVote.create(e.getKey(), e.getValue()).format()); + } + } + return msgs; + } + private class ReplaceRequest { final Change.Id ontoChange; final RevCommit newCommit; @@ -2118,29 +2142,52 @@ public class ReceiveCommits { return Futures.makeChecked(future, INSERT_EXCEPTION); } - private ChangeMessage newChangeMessage(ReviewDb db, ChangeKind changeKind) + private ChangeMessage newChangeMessage(ReviewDb db, ChangeKind changeKind, + Map approvals) throws OrmException { msg = new ChangeMessage(new ChangeMessage.Key(change.getId(), ChangeUtil .messageUUID(db)), currentUser.getAccountId(), newPatchSet.getCreatedOn(), newPatchSet.getId()); - String message = "Uploaded patch set " + newPatchSet.getPatchSetId(); + StringBuilder msgs = renderMessageWithApprovals( + newPatchSet.getPatchSetId(), approvals, scanLabels(db, approvals)); switch (changeKind) { case TRIVIAL_REBASE: case NO_CHANGE: - message += ": Patch Set " + priorPatchSet.get() + " was rebased"; + msgs.append(": Patch Set " + priorPatchSet.get() + " was rebased"); break; case NO_CODE_CHANGE: - message += ": Commit message was updated"; + msgs.append(": Commit message was updated"); break; case REWORK: default: break; } - msg.setMessage(message + "."); + msg.setMessage(msgs.toString() + "."); return msg; } + private Map scanLabels(ReviewDb db, + Map approvals) + throws OrmException { + Map current = new HashMap<>(); + // We optimize here and only retrieve current when approvals provided + if (!approvals.isEmpty()) { + for (PatchSetApproval a : approvalsUtil.byPatchSetUser( + db, changeCtl, priorPatchSet, currentUser.getAccountId())) { + if (a.isSubmit()) { + continue; + } + + LabelType lt = labelTypes.byLabel(a.getLabelId()); + if (lt != null) { + current.put(lt.getName(), a); + } + } + } + return current; + } + PatchSet.Id upsertEdit() { if (cmd.getResult() == NOT_ATTEMPTED) { cmd.execute(rp); @@ -2205,7 +2252,8 @@ public class ReceiveCommits { changeKind = changeKindCache.getChangeKind( projectControl.getProjectState(), repo, priorCommit, newCommit); - cmUtil.addChangeMessage(db, update, newChangeMessage(db, changeKind)); + cmUtil.addChangeMessage(db, update, newChangeMessage(db, changeKind, + approvals)); if (mergedIntoRef == null) { // Change should be new, so it can go through review again. @@ -2304,6 +2352,11 @@ public class ReceiveCommits { change, currentUser.getAccount(), newPatchSet, db, newCommit.getName()); } + if (!approvals.isEmpty()) { + hooks.doCommentAddedHook(change, currentUser.getAccount(), newPatchSet, + null, approvals, db); + } + if (magicBranch != null && magicBranch.submit) { submit(changeCtl, newPatchSet); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java index 3d466f8a6e..06ad9500e3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java @@ -14,12 +14,17 @@ package com.google.gerrit.server.git.validators; +import static com.google.gerrit.reviewdb.client.RefNames.REFS_CHANGES; +import static com.google.gerrit.reviewdb.client.RefNames.REFS_CONFIG; +import static org.eclipse.jgit.lib.Constants.R_HEADS; + import com.google.common.base.CharMatcher; +import com.google.gerrit.common.ChangeHookRunner.HookResult; +import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.FooterConstants; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.PageLinks; import com.google.gerrit.extensions.registration.DynamicSet; -import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.config.CanonicalWebUrl; @@ -39,6 +44,7 @@ import com.jcraft.jsch.HostKey; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.notes.NoteMap; @@ -82,6 +88,7 @@ public class CommitValidators { private final String installCommitMsgHookCommand; private final SshInfo sshInfo; private final Repository repo; + private final ChangeHooks hooks; private final DynamicSet commitValidationListeners; @Inject @@ -89,6 +96,7 @@ public class CommitValidators { @CanonicalWebUrl @Nullable final String canonicalWebUrl, @GerritServerConfig final Config config, final DynamicSet commitValidationListeners, + final ChangeHooks hooks, @Assisted final SshInfo sshInfo, @Assisted final Repository repo, @Assisted final RefControl refControl) { this.gerritIdent = gerritIdent; @@ -98,6 +106,7 @@ public class CommitValidators { config.getString("gerrit", null, "installCommitMsgHookCommand"); this.sshInfo = sshInfo; this.repo = repo; + this.hooks = hooks; this.commitValidationListeners = commitValidationListeners; } @@ -155,6 +164,7 @@ public class CommitValidators { } validators.add(new ConfigValidator(refControl, repo)); validators.add(new PluginCommitValidationListener(commitValidationListeners)); + validators.add(new ChangeHookValidator(refControl, hooks)); List messages = new LinkedList<>(); @@ -308,7 +318,7 @@ public class CommitValidators { CommitReceivedEvent receiveEvent) throws CommitValidationException { IdentifiedUser currentUser = (IdentifiedUser) refControl.getCurrentUser(); - if (RefNames.REFS_CONFIG.equals(refControl.getRefName())) { + if (REFS_CONFIG.equals(refControl.getRefName())) { List messages = new LinkedList<>(); try { @@ -535,6 +545,48 @@ public class CommitValidators { } } + /** Reject commits that don't pass user-supplied ref-update hook. */ + public static class ChangeHookValidator implements + CommitValidationListener { + private final RefControl refControl; + private final ChangeHooks hooks; + + public ChangeHookValidator(RefControl refControl, ChangeHooks hooks) { + this.refControl = refControl; + this.hooks = hooks; + } + + @Override + public List onCommitReceived( + CommitReceivedEvent receiveEvent) throws CommitValidationException { + + if (refControl.getCurrentUser().isIdentifiedUser()) { + IdentifiedUser user = (IdentifiedUser) refControl.getCurrentUser(); + + String refname = receiveEvent.refName; + ObjectId old = receiveEvent.commit.getParent(0); + + if (receiveEvent.command.getRefName().startsWith(REFS_CHANGES)) { + /* + * If the ref-update hook tries to distinguish behavior between pushes to + * refs/heads/... and refs/for/..., make sure we send it the correct refname. + * Also, if this is targetting refs/for/, make sure we behave the same as + * what a push to refs/for/ would behave; in particular, setting oldrev to + * 0000000000000000000000000000000000000000. + */ + refname = refname.replace(R_HEADS, "refs/for/refs/heads/"); + old = ObjectId.zeroId(); + } + HookResult result = hooks.doRefUpdateHook(receiveEvent.project, refname, + user.getAccount(), old, receiveEvent.commit); + if (result != null && result.getExitValue() != 0) { + throw new CommitValidationException(result.toString().trim()); + } + } + return Collections.emptyList(); + } + } + private static CommitValidationMessage getInvalidEmailError(RevCommit c, String type, PersonIdent who, IdentifiedUser currentUser, String canonicalWebUrl) { StringBuilder sb = new StringBuilder();