Provide the host name of the OS as fallback for the canonicalWebUrl

CommitValidators was already using the host name of the OS as fallback
whenever no canonicalWebUrl was defined. So, why not apply this policy
throughout our code?

Change-Id: I7ba0f9a14f11b4458a192c3e3355c90f3721dd0a

java/com/google/gerrit/httpd/HttpCanonicalWebUrlProvider.java

@@ -20,8 +20,10 @@ import com.google.inject.Inject;
 import com.google.inject.OutOfScopeException;
 import com.google.inject.Provider;
 import com.google.inject.ProvisionException;
+import java.util.Optional;
 import javax.servlet.http.HttpServletRequest;
 import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.util.SystemReader;
 
 /** Sets {@code CanonicalWebUrl} to current HTTP request if not configured. */
 public class HttpCanonicalWebUrlProvider extends CanonicalWebUrlProvider {
@@ -44,28 +46,31 @@ public class HttpCanonicalWebUrlProvider extends CanonicalWebUrlProvider {
       return canonicalUrl;
     }
 
-    if (requestProvider != null) {
-      // No canonical URL configured? Maybe we can get a reasonable
-      // guess from the incoming HTTP request, if we are currently
-      // inside of an HTTP request scope.
-      //
-      final HttpServletRequest req;
-      try {
-        req = requestProvider.get();
-      } catch (ProvisionException noWeb) {
-        if (noWeb.getCause() instanceof OutOfScopeException) {
-          // We can't obtain the request as we are not inside of
-          // an HTTP request scope. Callers must handle null.
-          //
-          return null;
-        }
-        throw noWeb;
-      }
-      return CanonicalWebUrl.computeFromRequest(req);
+    return guessUrlFromHttpRequest()
+        .orElseGet(() -> "http://" + SystemReader.getInstance().getHostname() + '/');
+  }
+
+  private Optional<String> guessUrlFromHttpRequest() {
+    if (requestProvider == null) {
+      // We have no way of guessing our HTTP url.
+      return Optional.empty();
     }
 
-    // We have no way of guessing our HTTP url.
+    // No canonical URL configured? Maybe we can get a reasonable
+    // guess from the incoming HTTP request, if we are currently
+    // inside of an HTTP request scope.
     //
-    return null;
+    final HttpServletRequest req;
+    try {
+      req = requestProvider.get();
+    } catch (ProvisionException noWeb) {
+      if (noWeb.getCause() instanceof OutOfScopeException) {
+        // We can't obtain the request as we are not inside of
+        // an HTTP request scope. Callers must handle null.
+        return Optional.empty();
+      }
+      throw noWeb;
+    }
+    return Optional.of(CanonicalWebUrl.computeFromRequest(req));
   }
 }

java/com/google/gerrit/server/git/validators/CommitValidators.java

@@ -20,7 +20,6 @@ import static com.google.gerrit.reviewdb.client.RefNames.REFS_CONFIG;
 import static java.util.stream.Collectors.toList;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.CharMatcher;
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.FooterConstants;
 import com.google.gerrit.common.Nullable;
@@ -372,8 +371,8 @@ public class CommitValidators {
       if (hostKeys.isEmpty()) {
         String p = "${gitdir}/hooks/commit-msg";
         return String.format(
-            "  gitdir=$(git rev-parse --git-dir); curl -o %s %s/tools/hooks/commit-msg ; chmod +x %s",
-            p, getGerritUrl(canonicalWebUrl), p);
+            "  gitdir=$(git rev-parse --git-dir); curl -o %s %stools/hooks/commit-msg ; chmod +x %s",
+            p, canonicalWebUrl, p);
       }
 
       // SSH keys exist, so the hook can be installed with scp.
@@ -882,20 +881,6 @@ public class CommitValidators {
     return new CommitValidationMessage(sb.toString(), false);
   }
 
-  /**
-   * Get the Gerrit URL.
-   *
-   * @return the canonical URL (with any trailing slash removed) if it is configured, otherwise fall
-   *     back to "http://hostname" where hostname is the value returned by {@link
-   *     #getGerritHost(String)}.
-   */
-  private static String getGerritUrl(String canonicalWebUrl) {
-    if (canonicalWebUrl != null) {
-      return CharMatcher.is('/').trimTrailingFrom(canonicalWebUrl);
-    }
-    return "http://" + getGerritHost(canonicalWebUrl);
-  }
-
   /**
    * Get the Gerrit hostname.
    *
@@ -903,17 +888,14 @@ public class CommitValidators {
    *     the hostname is.
    */
   private static String getGerritHost(String canonicalWebUrl) {
-    String host;
     if (canonicalWebUrl != null) {
       try {
-        host = new URL(canonicalWebUrl).getHost();
-      } catch (MalformedURLException e) {
-        host = SystemReader.getInstance().getHostname();
+        return new URL(canonicalWebUrl).getHost();
+      } catch (MalformedURLException ignored) {
       }
-    } else {
-      host = SystemReader.getInstance().getHostname();
     }
-    return host;
+
+    return SystemReader.getInstance().getHostname();
   }
 
   private static void addError(String error, List<CommitValidationMessage> messages) {