diff --git a/Documentation/config-reverseproxy.txt b/Documentation/config-reverseproxy.txt index 0857442fa3..064fe2e548 100644 --- a/Documentation/config-reverseproxy.txt +++ b/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' -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 +To run Gerrit behind an Apache server using 'mod_proxy', 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 -the 'RewriteRule' line to use the 'http://' URL configured above. -Ensure the path of 'RewriteRule' (the part before '$1') and -httpd.listenUrl match, 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. +Configure an Apache VirtualHost to proxy to the Gerrit daemon, +setting the 'ProxyPass' line to use the 'http://' URL configured +above. Ensure the path of ProxyPass and httpd.listenUrl match, +or links will redirect to incorrect locations. ---- ServerName review.example.com - AllowEncodedSlashes NoDecode - RewriteEngine On - RewriteRule ^/r/(.*) http://localhost:8081/r/$1 [NE,P] + ProxyRequests Off + ProxyVia Off + ProxyPreserveHost On + + + Order deny,allow + Allow from all + + + AllowEncodedSlashes On + ProxyPass /r/ http://127.0.0.1:8081/r/ nocanon ---- +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] diff --git a/ReleaseNotes/ReleaseNotes-2.6.txt b/ReleaseNotes/ReleaseNotes-2.6.txt index ef481c5435..12986f61c0 100644 --- a/ReleaseNotes/ReleaseNotes-2.6.txt +++ b/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 diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties index 6c99081ba0..e02c27c755 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties +++ b/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. diff --git a/gerrit-httpd/src/main/resources/com/google/gerrit/httpd/auth/container/ConfigurationError.html b/gerrit-httpd/src/main/resources/com/google/gerrit/httpd/auth/container/ConfigurationError.html index 0bc3369211..a05e1ea90e 100644 --- a/gerrit-httpd/src/main/resources/com/google/gerrit/httpd/auth/container/ConfigurationError.html +++ b/gerrit-httpd/src/main/resources/com/google/gerrit/httpd/auth/container/ConfigurationError.html @@ -49,6 +49,15 @@ <VirtualHost review.example.com:80> ServerName review.example.com + ProxyRequests Off + ProxyVia Off + ProxyPreserveHost On + + <Proxy *> + Order deny,allow + Allow from all + </Proxy> +
<Location /r/login/> AuthType Basic AuthName "Gerrit Code Review" @@ -56,9 +65,8 @@ ... </Location>
- AllowEncodedSlashes NoDecode - RewriteEngine On - RewriteRule ^/r/(.*) http://.../r/$1 [NE,P] + AllowEncodedSlashes On + ProxyPass /r/ http://.../r/ nodecode </VirtualHost> diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java index c9f7dce7c6..d7bf5a36b5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java +++ b/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; 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 db239441ae..eb172ab34a 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 @@ -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 validCommits = new HashSet(); private ListMultimap refsByChange; - private Map refsById; + private SetMultimap refsById; private Map allRefs; private final SubmoduleOp.Factory subOpFactory; @@ -1888,6 +1890,14 @@ public class ReceiveCommits { } change.setLastSha1MergeTested(null); change.setCurrentPatchSet(info); + + final List 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 byCommit = changeRefsById(); + final SetMultimap byCommit = changeRefsById(); final Map byKey = openChangesByKey( new Branch.NameKey(project.getNameKey(), cmd.getRefName())); final List toClose = new ArrayList(); RevCommit c; while ((c = rw.next()) != null) { - final Ref ref = byCommit.get(c.copy()); - if (ref != null) { - rw.parseBody(c); - Change.Key closedChange = - closeChange(cmd, PatchSet.Id.fromRef(ref.getName()), c); - closeProgress.update(1); - if (closedChange != null) { - byKey.remove(closedChange); + final Set refs = byCommit.get(c.copy()); + for (Ref ref : refs) { + if (ref != null) { + rw.parseBody(c); + Change.Key closedChange = + closeChange(cmd, PatchSet.Id.fromRef(ref.getName()), c); + closeProgress.update(1); + if (closedChange != null) { + byKey.remove(closedChange); + } } } @@ -2193,9 +2205,9 @@ public class ReceiveCommits { return change.getKey(); } - private Map changeRefsById() throws IOException { + private SetMultimap changeRefsById() throws IOException { if (refsById == null) { - refsById = new HashMap(); + refsById = HashMultimap.create(); for (Ref r : repo.getRefDatabase().getRefs("refs/changes/").values()) { if (PatchSet.isRef(r.getName())) { refsById.put(r.getObjectId(), r); diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties b/gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties index 11384a14f6..f05f23b63c 100644 --- a/gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties +++ b/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.