Correctly redirect legacy URIs with any final segment
URIs like `/c/1/123`, `/c/1/1..3` or ``/c/1/anyThing` currently returns a 404.
The legacy format, without project name in the URI,
is not correctly handled.
Handle it in the backend.
Bug: Issue 346377876
Bug: Issue 346618792
Release-Notes: skip
Change-Id: Id9fff1923a9c6e5cb3cbc458af5fbb9f0dd23af9
diff --git a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
index 48dcea2..2c80c9b 100644
--- a/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
+++ b/java/com/google/gerrit/httpd/NumericChangeIdRedirectServlet.java
@@ -14,6 +14,9 @@
package com.google.gerrit.httpd;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -24,6 +27,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Arrays;
import java.util.Optional;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@@ -54,13 +58,13 @@
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse rsp) throws IOException {
String uriPath = req.getPathInfo();
- // Check if we are processing a comment url, like "/c/1/comment/ff3303fd_8341647b/".
- int commentIdx = uriPath.indexOf("/comment");
- String idString = commentIdx == -1 ? uriPath : uriPath.substring(0, commentIdx);
- if (idString.endsWith("/")) {
- idString = idString.substring(0, idString.length() - 1);
- }
+ ImmutableList<String> uriSegments =
+ Arrays.stream(uriPath.split("/", 2)).collect(toImmutableList());
+
+ String idString = uriSegments.get(0);
+ String finalSegment = (uriSegments.size() > 1) ? uriSegments.get(1) : null;
+
Optional<Change.Id> id = Change.Id.tryParse(idString);
if (id.isEmpty()) {
rsp.sendError(HttpServletResponse.SC_NOT_FOUND);
@@ -78,9 +82,8 @@
}
String path =
PageLinks.toChange(changeResource.getProject(), changeResource.getChange().getId());
- if (commentIdx > -1) {
- // path already contain a trailing /, hence we start from "commentIdx + 1"
- path = path + uriPath.substring(commentIdx + 1);
+ if (finalSegment != null) {
+ path += finalSegment;
}
UrlModule.toGerrit(path, req, rsp);
}
diff --git a/java/com/google/gerrit/httpd/UrlModule.java b/java/com/google/gerrit/httpd/UrlModule.java
index aad6b57..db81b8a 100644
--- a/java/com/google/gerrit/httpd/UrlModule.java
+++ b/java/com/google/gerrit/httpd/UrlModule.java
@@ -72,8 +72,7 @@
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("^(?:/c)?/([1-9][0-9]*)/comment/\\w+/?$").with(NumericChangeIdRedirectServlet.class);
+ serveRegex("^(?:/c)?/([1-9][0-9]*)/?.*$").with(NumericChangeIdRedirectServlet.class);
serveRegex("^/p/(.*)$").with(queryProjectNew());
serveRegex("^/r/(.+)/?$").with(DirectChangeByCommit.class);
diff --git a/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java
index 0393f2b..e72b0f4 100644
--- a/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java
@@ -32,6 +32,7 @@
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.httpd.restapi.ParameterParser;
import com.google.gerrit.httpd.restapi.RestApiServlet;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import java.io.IOException;
import java.util.List;
@@ -449,6 +450,19 @@
}
@Test
+ public void testNumericChangeIdWithExtraSegments() throws Exception {
+ ChangeData changeData = createChange().getChange();
+ int changeNumber = changeData.getId().get();
+
+ String finalSegment = "any/Thing";
+
+ String redirectUri = String.format("/c/%s/+/%d/%s", project.get(), changeNumber, finalSegment);
+ anonymousRestSession
+ .get(String.format("/c/%d/%s", changeNumber, finalSegment))
+ .assertTemporaryRedirect(redirectUri);
+ }
+
+ @Test
public void testCommentLinkWithoutPrefixRedirects() throws Exception {
int changeNumber = createChange().getChange().getId().get();
String commentId = "ff3303fd_8341647b";
