about summary refs log tree commit diff
path: root/third_party/gerrit/0004-Add-titles-to-CLs-over-HTTP.patch
diff options
context:
space:
mode:
authorLuke Granger-Brown <git@lukegb.com>2020-07-02T22·28+0100
committerlukegb <lukegb@tvl.fyi>2020-07-03T18·18+0000
commit3f6518ce99de8d8fa330ae551b2dc49b2094c712 (patch)
tree57d8abbcfb7225f1360e0d69446817df3ebfe3f1 /third_party/gerrit/0004-Add-titles-to-CLs-over-HTTP.patch
parent26bb34823d884a619985cf91262f180e0ad4d207 (diff)
fix(gerrit): return HTML titles in more cases r/1199
At present, we don't return HTML titles if there's a trailing slash,
or a patchset. Instead, just consume the / and anything after it.

This also fixes /123, because this is HTTP redirected to the full path
*with a trailing slash* which otherwise wouldn't get the title
injected.

Change-Id: Idfd0e67752880a37dce0b400a3c1cfc53fac2912
Reviewed-on: https://cl.tvl.fyi/c/depot/+/859
Reviewed-by: BuildkiteCI
Reviewed-by: tazjin <mail@tazj.in>
Tested-by: BuildkiteCI
Diffstat (limited to 'third_party/gerrit/0004-Add-titles-to-CLs-over-HTTP.patch')
-rw-r--r--third_party/gerrit/0004-Add-titles-to-CLs-over-HTTP.patch217
1 files changed, 217 insertions, 0 deletions
diff --git a/third_party/gerrit/0004-Add-titles-to-CLs-over-HTTP.patch b/third_party/gerrit/0004-Add-titles-to-CLs-over-HTTP.patch
new file mode 100644
index 000000000000..bf372add84b9
--- /dev/null
+++ b/third_party/gerrit/0004-Add-titles-to-CLs-over-HTTP.patch
@@ -0,0 +1,217 @@
+From ead56bd63491ba4fd3c24fb1e9af36a3fa2141bf Mon Sep 17 00:00:00 2001
+From: Luke Granger-Brown <git@lukegb.com>
+Date: Thu, 2 Jul 2020 23:03:02 +0100
+Subject: [PATCH 4/4] Add titles to CLs over HTTP
+
+---
+ .../gerrit/httpd/raw/IndexHtmlUtil.java       | 14 +++-
+ .../google/gerrit/httpd/raw/IndexServlet.java |  7 +-
+ .../google/gerrit/httpd/raw/StaticModule.java |  5 +-
+ .../gerrit/httpd/raw/TitleComputer.java       | 67 +++++++++++++++++++
+ .../gerrit/httpd/raw/PolyGerritIndexHtml.soy  |  4 +-
+ 5 files changed, 89 insertions(+), 8 deletions(-)
+ create mode 100644 java/com/google/gerrit/httpd/raw/TitleComputer.java
+
+diff --git a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
+index 41d2f83975..323567b4a4 100644
+--- a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
++++ b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
+@@ -42,6 +42,7 @@ import java.util.Arrays;
+ import java.util.Collections;
+ import java.util.HashMap;
+ import java.util.Map;
++import java.util.Optional;
+ import java.util.Set;
+ import java.util.function.Function;
+ import java.util.regex.Matcher;
+@@ -110,7 +111,8 @@ public class IndexHtmlUtil {
+       String faviconPath,
+       Map<String, String[]> urlParameterMap,
+       Function<String, SanitizedContent> urlInScriptTagOrdainer,
+-      String requestedURL)
++      String requestedURL,
++      TitleComputer titleComputer)
+       throws URISyntaxException, RestApiException {
+     ImmutableMap.Builder<String, Object> data = ImmutableMap.builder();
+     data.putAll(
+@@ -121,7 +123,7 @@ public class IndexHtmlUtil {
+                 urlParameterMap,
+                 urlInScriptTagOrdainer,
+                 requestedURL))
+-        .putAll(dynamicTemplateData(gerritApi));
++        .putAll(dynamicTemplateData(gerritApi, requestedURL, titleComputer));
+ 
+     Set<String> enabledExperiments = experimentData(urlParameterMap);
+     if (!enabledExperiments.isEmpty()) {
+@@ -131,7 +133,9 @@ public class IndexHtmlUtil {
+   }
+ 
+   /** Returns dynamic parameters of {@code index.html}. */
+-  public static ImmutableMap<String, Object> dynamicTemplateData(GerritApi gerritApi)
++  public static ImmutableMap<String, Object> dynamicTemplateData(GerritApi gerritApi,
++                                                                 String requestedURL,
++                                                                 TitleComputer titleComputer)
+       throws RestApiException {
+     ImmutableMap.Builder<String, Object> data = ImmutableMap.builder();
+     Map<String, SanitizedContent> initialData = new HashMap<>();
+@@ -159,6 +163,10 @@ public class IndexHtmlUtil {
+     }
+ 
+     data.put("gerritInitialData", initialData);
++
++    Optional<String> title = titleComputer.computeTitle(requestedURL);
++    title.ifPresent(s -> data.put("title", s));
++
+     return data.build();
+   }
+ 
+diff --git a/java/com/google/gerrit/httpd/raw/IndexServlet.java b/java/com/google/gerrit/httpd/raw/IndexServlet.java
+index 97d22701de..089ef4725f 100644
+--- a/java/com/google/gerrit/httpd/raw/IndexServlet.java
++++ b/java/com/google/gerrit/httpd/raw/IndexServlet.java
+@@ -44,12 +44,14 @@ public class IndexServlet extends HttpServlet {
+   private final GerritApi gerritApi;
+   private final SoySauce soySauce;
+   private final Function<String, SanitizedContent> urlOrdainer;
++  private TitleComputer titleComputer;
+ 
+   IndexServlet(
+       @Nullable String canonicalUrl,
+       @Nullable String cdnPath,
+       @Nullable String faviconPath,
+-      GerritApi gerritApi) {
++      GerritApi gerritApi,
++      TitleComputer titleComputer) {
+     this.canonicalUrl = canonicalUrl;
+     this.cdnPath = cdnPath;
+     this.faviconPath = faviconPath;
+@@ -63,6 +65,7 @@ public class IndexServlet extends HttpServlet {
+         (s) ->
+             UnsafeSanitizedContentOrdainer.ordainAsSafe(
+                 s, SanitizedContent.ContentKind.TRUSTED_RESOURCE_URI);
++    this.titleComputer = titleComputer;
+   }
+ 
+   @Override
+@@ -74,7 +77,7 @@ public class IndexServlet extends HttpServlet {
+       // TODO(hiesel): Remove URL ordainer as parameter once Soy is consistent
+       ImmutableMap<String, Object> templateData =
+           IndexHtmlUtil.templateData(
+-              gerritApi, canonicalUrl, cdnPath, faviconPath, parameterMap, urlOrdainer, requestUrl);
++              gerritApi, canonicalUrl, cdnPath, faviconPath, parameterMap, urlOrdainer, requestUrl, titleComputer);
+       renderer = soySauce.renderTemplate("com.google.gerrit.httpd.raw.Index").setData(templateData);
+     } catch (URISyntaxException | RestApiException e) {
+       throw new IOException(e);
+diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java
+index 414a120194..e1b6fb082d 100644
+--- a/java/com/google/gerrit/httpd/raw/StaticModule.java
++++ b/java/com/google/gerrit/httpd/raw/StaticModule.java
+@@ -220,11 +220,12 @@ public class StaticModule extends ServletModule {
+     HttpServlet getPolyGerritUiIndexServlet(
+         @CanonicalWebUrl @Nullable String canonicalUrl,
+         @GerritServerConfig Config cfg,
+-        GerritApi gerritApi) {
++        GerritApi gerritApi,
++        TitleComputer titleComputer) {
+       String cdnPath =
+           options.useDevCdn() ? options.devCdn() : cfg.getString("gerrit", null, "cdnPath");
+       String faviconPath = cfg.getString("gerrit", null, "faviconPath");
+-      return new IndexServlet(canonicalUrl, cdnPath, faviconPath, gerritApi);
++      return new IndexServlet(canonicalUrl, cdnPath, faviconPath, gerritApi, titleComputer);
+     }
+ 
+     @Provides
+diff --git a/java/com/google/gerrit/httpd/raw/TitleComputer.java b/java/com/google/gerrit/httpd/raw/TitleComputer.java
+new file mode 100644
+index 0000000000..8fd2053ad0
+--- /dev/null
++++ b/java/com/google/gerrit/httpd/raw/TitleComputer.java
+@@ -0,0 +1,67 @@
++package com.google.gerrit.httpd.raw;
++
++import com.google.common.flogger.FluentLogger;
++import com.google.gerrit.entities.Change;
++import com.google.gerrit.extensions.restapi.ResourceConflictException;
++import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
++import com.google.gerrit.server.change.ChangeResource;
++import com.google.gerrit.server.permissions.PermissionBackendException;
++import com.google.gerrit.server.restapi.change.ChangesCollection;
++import com.google.inject.Inject;
++import com.google.inject.Provider;
++import com.google.inject.Singleton;
++
++import java.net.MalformedURLException;
++import java.net.URL;
++import java.util.Optional;
++import java.util.regex.Matcher;
++import java.util.regex.Pattern;
++
++@Singleton
++public class TitleComputer {
++  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
++
++  @Inject
++  public TitleComputer(Provider<ChangesCollection> changes) {
++    this.changes = changes;
++  }
++
++  public Optional<String> computeTitle(String requestedURI) {
++    URL url = null;
++    try {
++      url = new URL(requestedURI);
++    } catch (MalformedURLException e) {
++      logger.atWarning().log("Failed to turn %s into a URL.", requestedURI);
++      return Optional.empty();
++    }
++
++    // Try to turn this into a change.
++    Optional<Change.Id> changeId = tryExtractChange(url.getPath());
++    if (changeId.isPresent()) {
++      return titleFromChangeId(changeId.get());
++    }
++
++    return Optional.empty();
++  }
++
++  private static final Pattern extractChangeIdRegex = Pattern.compile("^/(?:c/.*/\\+/)?(?<changeId>[0-9]+)(?:/[0-9]+)?(?:/.*)?$");
++  private final Provider<ChangesCollection> changes;
++
++  private Optional<Change.Id> tryExtractChange(String path) {
++    Matcher m = extractChangeIdRegex.matcher(path);
++    if (!m.matches()) {
++      return Optional.empty();
++    }
++    return Change.Id.tryParse(m.group("changeId"));
++  }
++
++  private Optional<String> titleFromChangeId(Change.Id changeId) {
++    ChangesCollection changesCollection = changes.get();
++    try {
++      ChangeResource changeResource = changesCollection.parse(changeId);
++      return Optional.of(changeResource.getChange().getSubject());
++    } catch (ResourceConflictException | ResourceNotFoundException | PermissionBackendException e) {
++      return Optional.empty();
++    }
++  }
++}
+diff --git a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
+index d162714399..0ba228ad00 100644
+--- a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
++++ b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
+@@ -33,10 +33,12 @@
+   {@param? preloadChangePage: ?}
+   {@param? preloadDiffPage: ?}
+   {@param? userIsAuthenticated: ?}
++  {@param? title: ?}
+   <!DOCTYPE html>{\n}
+   <html lang="en">{\n}
+   <meta charset="utf-8">{\n}
+-  <meta name="description" content="Gerrit Code Review">{\n}
++  {if $title}<title>{$title} · Gerrit Code Review</title>{\n}{/if}
++  <meta name="description" content="{if $title}{$title} · {/if}Gerrit Code Review">{\n}
+   <meta name="referrer" content="never">{\n}
+   <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=0">{\n}
+ 
+-- 
+2.25.1
+