Merge branch 'stable-2.6' into stable-2.7

* stable-2.6:
  Check that patch set is current before doing a rebase
  Add trailing period on "This reverts commit ..." line
  Documentation: Reverse Proxy Configuration
  Fix: push bypass review cannot close change from UI
  Fix: 'change_key' of current patchset pushed by 'refs/changes' is still old one.
  Update the release notes for 2.6

Conflicts:
	gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

Change-Id: Ie26334c433bc23c8ec47440bbd14c54e6ca8a782

Documentation/config-reverseproxy.txt

@@ -28,37 +28,40 @@ during 'init'.
 Apache 2 Configuration
 ----------------------
 
-To run Gerrit behind an Apache server we cannot use 'mod_proxy'
+To run Gerrit behind an Apache server using 'mod_proxy', enable the
-directly, as Gerrit relies on getting unmodified escaped forward
-slashes. Depending on the setting of 'AllowEncodedSlashes',
-'mod_proxy' would either decode encoded slashes, or encode them once
-again. Hence, we resort to using 'mod_rewrite'. To enable the
 necessary Apache2 modules:
 
 ----
-  a2enmod rewrite
+  a2enmod proxy_http
   a2enmod ssl          ; # optional, needed for HTTPS / SSL
 ----
 
-Configure an Apache VirtualHost to proxy to the Gerrit daemon, setting
+Configure an Apache VirtualHost to proxy to the Gerrit daemon,
-the 'RewriteRule' line to use the 'http://' URL configured above.
+setting the 'ProxyPass' line to use the 'http://' URL configured
-Ensure the path of 'RewriteRule' (the part before '$1') and
+above.  Ensure the path of ProxyPass and httpd.listenUrl match,
-httpd.listenUrl match, or links will redirect to incorrect locations.
+or links will redirect to incorrect locations.
-
-Note that this configuration allows to pass encoded characters to the
-virtual host, which is potentially dangerous. Be sure to read up on
-this topic and that you understand the risks.
 
 ----
 	<VirtualHost *>
 	  ServerName review.example.com
 
-	  AllowEncodedSlashes NoDecode
+	  ProxyRequests Off
-	  RewriteEngine On
+	  ProxyVia Off
-	  RewriteRule ^/r/(.*) http://localhost:8081/r/$1 [NE,P]
+	  ProxyPreserveHost On
+
+	  <Proxy *>
+	    Order deny,allow
+	    Allow from all
+	  </Proxy>
+
+	  AllowEncodedSlashes On
+	  ProxyPass /r/ http://127.0.0.1:8081/r/ nocanon
 	</VirtualHost>
 ----
 
+The two options 'AllowEncodedSlashes On' and 'ProxyPass .. nocanon' are required
+since Gerrit 2.6.
+
 SSL
 ~~~
 
@@ -80,6 +83,15 @@ See the Apache 'mod_ssl' documentation for more details on how to
 configure SSL within the server, like controlling how strong of an
 encryption algorithm is required.
 
+Troubleshooting
+~~~~~~~~~~~~~~~
+
+If you are encountering 'Page Not Found' errors when opening the change
+screen, your Apache proxy is very likely decoding the passed URL.
+Make sure to either use 'AllowEncodedSlashes On' together with
+'ProxyPass .. nodecode' or alternatively a 'mod_rewrite' configuration with
+'AllowEncodedSlashes NoDecode' set.
+
 
 Nginx Configuration
 -------------------
@@ -124,6 +136,14 @@ See the Nginx 'http ssl module' documentation for more details on
 how to configure SSL within the server, like controlling how strong
 of an encryption algorithm is required.
 
+Troubleshooting
+~~~~~~~~~~~~~~~
+
+If you are encountering 'Page Not Found' errors when opening the change
+screen, your Nginx proxy is very likely decoding the passed URL.
+Make sure to use a 'proxy_pass' URL without any path (esp. no trailing
+'/' after the 'host:port').
+
 GERRIT
 ------
 Part of link:index.html[Gerrit Code Review]

ReleaseNotes/ReleaseNotes-2.6.txt

@@ -23,6 +23,17 @@ Schema Change
 a later 2.1.x version), and then to 2.6.x.  If you are upgrading from 2.2.x.x or
 newer, you may ignore this warning and upgrade directly to 2.6.x.
 
+Reverse Proxy Configuration Changes
+-----------------------------------
+
+If you are running a reverse proxy in front of Gerrit (e.g. Apache or Nginx),
+make sure to check your configuration, especially if you are encountering
+'Page Not Found' errors when opening the change screen.
+See the link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/config-reverseproxy.html[
+Reverse Proxy Configuration] for details.
+
+Gerrit now requires passed URLs to be unchanged by the proxy.
+
 Release Highlights
 ------------------
 * 42x improvement on `git clone` and `git fetch`
@@ -437,13 +448,6 @@ responses are protected from accidential sniffing and treatment as
 HTML thanks to Gson encoding HTML control characters using Unicode
 character escapes within JSON strings.
 
-* Apache reverse proxies must switch to mod_rewrite
-+
-When Apache is used as a reverse proxy the server must be reconfigured
-to use mod_rewrite and AllowEncodedSlashes.  For updated information
-link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/config-reverseproxy.html#_apache_2_configuration[
-review the Apache 2 Configuration documentation].
-
 Project Dashboards
 ~~~~~~~~~~~~~~~~~~
 * link:http://gerrit-documentation.googlecode.com/svn/Documentation/2.6/user-dashboards.html#project-dashboards[
@@ -1222,6 +1226,13 @@ confusing.
 * Prevent account's full name from being set to empty string.  Set it to
   null instead.
 
+* link:https://code.google.com/p/gerrit/issues/detail?id=1682[Issue 1682]:
+Correctly handle paths with URL-escaped characters
++
+URL-unescape the path portion of a change history token to correctly
+handle paths with URL-escapable characters, i.e. '+', ' ', etc.
+
+
 REST API
 ~~~~~~~~
 * Fix returning of 'Email Reviewers' capability via REST

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties

@@ -5,7 +5,7 @@ changesOpenInProject = Open Changes In {0}
 changesMergedInProject = Merged Changes In {0}
 changesAbandonedInProject = Abandoned Changes In {0}
 
-revertChangeDefaultMessage = Revert \"{0}\"\n\nThis reverts commit {1}
+revertChangeDefaultMessage = Revert \"{0}\"\n\nThis reverts commit {1}.
 
 changeScreenTitleId = Change {0}
 outdatedHeader = Change depends on {0} outdated change(s) and should be rebased on the latest patch sets.

gerrit-httpd/src/main/resources/com/google/gerrit/httpd/auth/container/ConfigurationError.html

@@ -49,6 +49,15 @@
 &lt;VirtualHost <span class='ServerName'>review.example.com</span><span class='ServerPort'>:80</span>&gt;
     ServerName <span class='ServerName'>review.example.com</span>
 
+    ProxyRequests Off
+    ProxyVia Off
+    ProxyPreserveHost On
+
+    &lt;Proxy *&gt;
+          Order deny,allow
+          Allow from all
+    &lt;/Proxy&gt;
+
 <div class='apache_auth'>    &lt;Location <span class='ContextPath'>/r</span>/login/&gt;
       AuthType Basic
       AuthName "Gerrit Code Review"
@@ -56,9 +65,8 @@
       ...
     &lt;/Location&gt;</div>
 
-    AllowEncodedSlashes NoDecode
+    AllowEncodedSlashes On
-    RewriteEngine On
+    ProxyPass <span class='ContextPath'>/r</span>/ http://...<span class='ContextPath'>/r</span>/ nodecode
-    RewriteRule ^<span class='ContextPath'>/r</span>/(.*) http://...<span class='ContextPath'>/r</span>/$1 [NE,P]
 &lt;/VirtualHost&gt;
     </pre>
   </body>

gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java

@@ -310,6 +310,9 @@ public class RebaseChange {
       OrmException, IOException, InvalidChangeOperationException,
       PathConflictException {
     Change change = chg;
+    if (!chg.currentPatchSetId().equals(patchSetId)) {
+      throw new InvalidChangeOperationException("patch set is not current");
+    }
     final PatchSet originalPatchSet = db.patchSets().get(patchSetId);
 
     final RevCommit rebasedCommit;

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -32,10 +32,12 @@ import com.google.common.base.Strings;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.BiMap;
 import com.google.common.collect.HashBiMap;
+import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedListMultimap;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Lists;
+import com.google.common.collect.SetMultimap;
 import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.Futures;
@@ -283,7 +285,7 @@ public class ReceiveCommits {
   private final Set<RevCommit> validCommits = new HashSet<RevCommit>();
 
   private ListMultimap<Change.Id, Ref> refsByChange;
-  private Map<ObjectId, Ref> refsById;
+  private SetMultimap<ObjectId, Ref> refsById;
   private Map<String, Ref> allRefs;
 
   private final SubmoduleOp.Factory subOpFactory;
@@ -1888,6 +1890,14 @@ public class ReceiveCommits {
                   }
                   change.setLastSha1MergeTested(null);
                   change.setCurrentPatchSet(info);
+
+                  final List<String> idList = newCommit.getFooterLines(CHANGE_ID);
+                  if (idList.isEmpty()) {
+                    change.setKey(new Change.Key("I" + newCommit.name()));
+                  } else {
+                    change.setKey(new Change.Key(idList.get(idList.size() - 1).trim()));
+                  }
+
                   ChangeUtil.updated(change);
                   return change;
                 }
@@ -2101,20 +2111,22 @@ public class ReceiveCommits {
         rw.markUninteresting(rw.parseCommit(cmd.getOldId()));
       }
 
-      final Map<ObjectId, Ref> byCommit = changeRefsById();
+      final SetMultimap<ObjectId, Ref> byCommit = changeRefsById();
       final Map<Change.Key, Change.Id> byKey = openChangesByKey(
           new Branch.NameKey(project.getNameKey(), cmd.getRefName()));
       final List<ReplaceRequest> toClose = new ArrayList<ReplaceRequest>();
       RevCommit c;
       while ((c = rw.next()) != null) {
-        final Ref ref = byCommit.get(c.copy());
+        final Set<Ref> refs = byCommit.get(c.copy());
-        if (ref != null) {
+        for (Ref ref : refs) {
-          rw.parseBody(c);
+          if (ref != null) {
-          Change.Key closedChange =
+            rw.parseBody(c);
-              closeChange(cmd, PatchSet.Id.fromRef(ref.getName()), c);
+            Change.Key closedChange =
-          closeProgress.update(1);
+                closeChange(cmd, PatchSet.Id.fromRef(ref.getName()), c);
-          if (closedChange != null) {
+            closeProgress.update(1);
-            byKey.remove(closedChange);
+            if (closedChange != null) {
+              byKey.remove(closedChange);
+            }
           }
         }
 
@@ -2193,9 +2205,9 @@ public class ReceiveCommits {
     return change.getKey();
   }
 
-  private Map<ObjectId, Ref> changeRefsById() throws IOException {
+  private SetMultimap<ObjectId, Ref> changeRefsById() throws IOException {
     if (refsById == null) {
-      refsById = new HashMap<ObjectId, Ref>();
+      refsById =  HashMultimap.create();
       for (Ref r : repo.getRefDatabase().getRefs("refs/changes/").values()) {
         if (PatchSet.isRef(r.getName())) {
           refsById.put(r.getObjectId(), r);

gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties

@@ -1,6 +1,6 @@
 # Changes to this file should also be made in
 # gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties
-revertChangeDefaultMessage = Revert \"{0}\"\n\nThis reverts commit {1}
+revertChangeDefaultMessage = Revert \"{0}\"\n\nThis reverts commit {1}.
 reviewerNotFound = {0} does not identify a registered user or group
 
 groupIsNotAllowed =  The group {0} cannot be added as reviewer.