Merge branch 'stable-3.9' into stable-3.10

* stable-3.9:
  Manage change number prefixed URL to project/+/changeNum

Release-Notes: skip
Change-Id: I21ca67f2027f974a142fc4e91fa4551a5db607b5
diff --git a/java/com/google/gerrit/acceptance/GerritServerRestSession.java b/java/com/google/gerrit/acceptance/GerritServerRestSession.java
index c2c77fe..9605f50 100644
--- a/java/com/google/gerrit/acceptance/GerritServerRestSession.java
+++ b/java/com/google/gerrit/acceptance/GerritServerRestSession.java
@@ -142,7 +142,8 @@
     return execute(delete);
   }
 
-  private String getUrl(String endPoint) {
+  @Override
+  public String getUrl(String endPoint) {
     return url + (account != null ? "/a" : "") + endPoint;
   }
 }
diff --git a/java/com/google/gerrit/acceptance/RestResponse.java b/java/com/google/gerrit/acceptance/RestResponse.java
index a9c14aa..a53c015 100644
--- a/java/com/google/gerrit/acceptance/RestResponse.java
+++ b/java/com/google/gerrit/acceptance/RestResponse.java
@@ -103,4 +103,9 @@
     assertStatus(SC_MOVED_TEMPORARILY);
     assertThat(URI.create(getHeader("Location")).getPath()).isEqualTo(path);
   }
+
+  public void assertTemporaryRedirectUri(String uri) throws Exception {
+    assertStatus(SC_MOVED_TEMPORARILY);
+    assertThat(getHeader("Location")).isEqualTo(uri);
+  }
 }
diff --git a/java/com/google/gerrit/acceptance/RestSession.java b/java/com/google/gerrit/acceptance/RestSession.java
index 0865e31..3fefd5b 100644
--- a/java/com/google/gerrit/acceptance/RestSession.java
+++ b/java/com/google/gerrit/acceptance/RestSession.java
@@ -52,4 +52,6 @@
   RestResponse delete(String endPoint) throws Exception;
 
   RestResponse deleteWithHeaders(String endPoint, Header... headers) throws Exception;
+
+  String getUrl(String endPoint);
 }
diff --git a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
index 2c80c9b..ca937fd 100644
--- a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
+++ b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
@@ -16,6 +16,7 @@
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.PageLinks;
 import com.google.gerrit.entities.Change;
@@ -85,6 +86,7 @@
     if (finalSegment != null) {
       path += finalSegment;
     }
-    UrlModule.toGerrit(path, req, rsp);
+    String queryString = Strings.emptyToNull(req.getQueryString());
+    UrlModule.toGerrit(path + (queryString != null ? "?" + queryString : ""), req, rsp);
   }
 }
diff --git a/java/com/google/gerrit/httpd/UrlModule.java b/java/com/google/gerrit/httpd/UrlModule.java
index aedf8e9..f62fbe8 100644
--- a/java/com/google/gerrit/httpd/UrlModule.java
+++ b/java/com/google/gerrit/httpd/UrlModule.java
@@ -45,6 +45,9 @@
 class UrlModule extends ServletModule {
   private final AuthConfig authConfig;
 
+  private static final String CHANGE_NUMBER_REGEX = "(?:/c)?/([1-9][0-9]*)";
+  private static final String PATCH_SET_REGEX = "([1-9][0-9]*(\\.\\.[1-9][0-9]*)?)";
+
   UrlModule(AuthConfig authConfig) {
     this.authConfig = authConfig;
   }
@@ -72,7 +75,12 @@
     serveRegex("^/settings/?$").with(screen(PageLinks.SETTINGS));
     serveRegex("^/register$").with(registerScreen(false));
     serveRegex("^/register/(.+)$").with(registerScreen(true));
-    serveRegex("^(?:/c)?/([1-9][0-9]*)/?.*$").with(NumericChangeIdRedirectServlet.class);
+    serveRegex("^" + CHANGE_NUMBER_REGEX + "(/" + PATCH_SET_REGEX + ")?/?$")
+        .with(NumericChangeIdRedirectServlet.class);
+    serveRegex("^" + CHANGE_NUMBER_REGEX + "/" + PATCH_SET_REGEX + "?/[^+]+$")
+        .with(NumericChangeIdRedirectServlet.class);
+    serveRegex("^" + CHANGE_NUMBER_REGEX + "/comment/\\w+/?$")
+        .with(NumericChangeIdRedirectServlet.class);
     serveRegex("^/p/(.*)$").with(queryProjectNew());
     serveRegex("^/r/(.+)/?$").with(DirectChangeByCommit.class);
 
diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java
index b00294f..42e10d8 100644
--- a/java/com/google/gerrit/httpd/raw/StaticModule.java
+++ b/java/com/google/gerrit/httpd/raw/StaticModule.java
@@ -63,7 +63,17 @@
 
 public class StaticModule extends ServletModule {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-  public static final String CHANGE_NUMBER_URI_REGEX = "^(?:/c)?/([1-9][0-9]*)/?.*";
+  // This constant is copied and NOT reused from UrlModule because of the need for
+  // StaticModule and UrlModule to be used in isolation. The requirement comes
+  // from the way Google includes these two classes in their setup.
+  private static final String CHANGE_NUMBER_REGEX = "(?:/c)?/([1-9][0-9]*)";
+  // Regex matching the direct links to comments using only the change number
+  // 1234/comment/abc_def
+  public static final String CHANGE_NUMBER_URI_REGEX =
+      "^"
+          + CHANGE_NUMBER_REGEX
+          + "(/[1-9][0-9]*(\\.\\.[1-9][0-9]*)?(/[^+]*)?)?(/comment/[^+]+)?/?$";
+
   private static final Pattern CHANGE_NUMBER_URI_PATTERN = Pattern.compile(CHANGE_NUMBER_URI_REGEX);
 
   /**
diff --git a/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java
index 8011fb0..a54c7ca 100644
--- a/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java
@@ -454,12 +454,21 @@
     ChangeData changeData = createChange().getChange();
     int changeNumber = changeData.getId().get();
 
-    String finalSegment = "any/Thing";
+    assertChangeNumberWithSuffixRedirected(changeNumber, "1..2");
+    assertChangeNumberWithSuffixRedirected(changeNumber, "2");
+    assertChangeNumberWithSuffixRedirected(changeNumber, "2/COMMIT_MSG");
+    assertChangeNumberWithSuffixRedirected(changeNumber, "2?foo=bar");
+    assertChangeNumberWithSuffixRedirected(changeNumber, "2/path/to/source/file/MyClass.java");
+  }
 
-    String redirectUri = String.format("/c/%s/+/%d/%s", project.get(), changeNumber, finalSegment);
+  private void assertChangeNumberWithSuffixRedirected(int changeNumber, String suffix)
+      throws Exception {
+    String redirectUri =
+        anonymousRestSession.getUrl(
+            String.format("/c/%s/+/%d/%s", project.get(), changeNumber, suffix));
     anonymousRestSession
-        .get(String.format("/c/%d/%s", changeNumber, finalSegment))
-        .assertTemporaryRedirect(redirectUri);
+        .get(String.format("/c/%d/%s", changeNumber, suffix))
+        .assertTemporaryRedirectUri(redirectUri);
   }
 
   @Test
@@ -467,12 +476,12 @@
     int changeNumber = createChange().getChange().getId().get();
     String commentId = "ff3303fd_8341647b";
 
-    String redirectUri =
+    String redirectPath =
         String.format("/c/%s/+/%d/comment/%s", project.get(), changeNumber, commentId);
 
     anonymousRestSession
         .get(String.format("/%s/comment/%s", changeNumber, commentId))
-        .assertTemporaryRedirect(redirectUri);
+        .assertTemporaryRedirect(redirectPath);
   }
 
   @Test
diff --git a/javatests/com/google/gerrit/httpd/raw/StaticModuleTest.java b/javatests/com/google/gerrit/httpd/raw/StaticModuleTest.java
index 28ec30d..e973a26 100644
--- a/javatests/com/google/gerrit/httpd/raw/StaticModuleTest.java
+++ b/javatests/com/google/gerrit/httpd/raw/StaticModuleTest.java
@@ -15,19 +15,33 @@
 package com.google.gerrit.httpd.raw;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.httpd.raw.StaticModule.PolyGerritFilter.isPolyGerritIndex;
 
-import com.google.common.collect.ImmutableList;
 import org.junit.Test;
 
 public class StaticModuleTest {
 
   @Test
   public void doNotMatchPolyGerritIndex() {
-    ImmutableList.of(
-            "/c/123456/anyString",
-            "/123456/anyString",
-            "/c/123456/comment/9ab75172_67d798e1",
-            "/123456/comment/9ab75172_67d798e1")
-        .forEach(url -> assertThat(StaticModule.PolyGerritFilter.isPolyGerritIndex(url)).isFalse());
+    assertThat(isPolyGerritIndex("/123456")).isFalse();
+    assertThat(isPolyGerritIndex("/123456/")).isFalse();
+    assertThat(isPolyGerritIndex("/123456/1")).isFalse();
+    assertThat(isPolyGerritIndex("/123456/1/")).isFalse();
+    assertThat(isPolyGerritIndex("/c/123456/comment/9ab75172_67d798e1")).isFalse();
+    assertThat(isPolyGerritIndex("/123456/comment/9ab75172_67d798e1")).isFalse();
+    assertThat(isPolyGerritIndex("/123456/comment/9ab75172_67d798e1/")).isFalse();
+    assertThat(isPolyGerritIndex("/123456/1..2")).isFalse();
+    assertThat(isPolyGerritIndex("/c/123456/1..2")).isFalse();
+    assertThat(isPolyGerritIndex("/c/2/1/COMMIT_MSG")).isFalse();
+    assertThat(isPolyGerritIndex("/c/2/1/path/to/source/file/MyClass.java")).isFalse();
+  }
+
+  @Test
+  public void matchPolyGerritIndex() {
+    assertThat(isPolyGerritIndex("/c/test/+/123456/anyString")).isTrue();
+    assertThat(isPolyGerritIndex("/c/test/+/123456/comment/9ab75172_67d798e1")).isTrue();
+    assertThat(isPolyGerritIndex("/c/321/+/123456/anyString")).isTrue();
+    assertThat(isPolyGerritIndex("/c/321/+/123456/comment/9ab75172_67d798e1")).isTrue();
+    assertThat(isPolyGerritIndex("/c/321/anyString")).isTrue();
   }
 }