opendev/gerrit
Code Issues Proposed changes

Allow running without gerrit.canonicalWebUrl

Browse Source 
Gerrit's CanonicalWebUrl class is already responsible of getting
the full URL from either gerrit.config or the incoming HTTP request.

However, over time, other parts of Gerrit injected directly the
configured string through @CanonicalWebUrl annotation and did not
manage correctly the situation where the configuration is absent.

The only situation where gerrit.canonicalWebUrl is mandatory is
OAuth authentication, because the OAuth provider needs to have
a fixed callback URL with a single hostname.

Change-Id: Idee1d36eb08c53a0d06604b9a737695322e124d1
This commit is contained in:
Luca Milanesio 2017-07-14 17:21:04 +01:00
parent 4aef830108
commit 53eab1080b
3 changed files with 15 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/IndexServlet.java
View File

@ -17,6 +17,7 @@ package com.google.gerrit.httpd.raw;
import static java.nio.charset.StandardCharsets.UTF_8; import static java.nio.charset.StandardCharsets.UTF_8;
import static javax.servlet.http.HttpServletResponse.SC_OK; import static javax.servlet.http.HttpServletResponse.SC_OK;
import com.google.common.base.Strings;
import com.google.common.io.Resources; import com.google.common.io.Resources;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.template.soy.SoyFileSet; import com.google.template.soy.SoyFileSet;
@ -61,6 +62,10 @@ public class IndexServlet extends HttpServlet {
} }
static String computeCanonicalPath(String canonicalURL) throws URISyntaxException { static String computeCanonicalPath(String canonicalURL) throws URISyntaxException {
if (Strings.isNullOrEmpty(canonicalURL)) {
return "";
}
// If we serving from a sub-directory rather than root, determine the path // If we serving from a sub-directory rather than root, determine the path
// from the cannonical web URL. // from the cannonical web URL.
URI uri = new URI(canonicalURL); URI uri = new URI(canonicalURL);

6
gerrit-server/src/main/java/com/google/gerrit/server/account/GetOAuthToken.java
View File

@ -38,7 +38,7 @@ class GetOAuthToken implements RestReadView<AccountResource> {
private final Provider<CurrentUser> self; private final Provider<CurrentUser> self;
private final OAuthTokenCache tokenCache; private final OAuthTokenCache tokenCache;
private final String hostName; private final Provider<String> canonicalWebUrlProvider;
@Inject @Inject
GetOAuthToken( GetOAuthToken(
@ -47,7 +47,7 @@ class GetOAuthToken implements RestReadView<AccountResource> {
@CanonicalWebUrl Provider<String> urlProvider) { @CanonicalWebUrl Provider<String> urlProvider) {
this.self = self; this.self = self;
this.tokenCache = tokenCache; this.tokenCache = tokenCache;
this.hostName = getHostName(urlProvider.get()); this.canonicalWebUrlProvider = urlProvider;
} }
@Override @Override
@ -63,7 +63,7 @@ class GetOAuthToken implements RestReadView<AccountResource> {
} }
OAuthTokenInfo accessTokenInfo = new OAuthTokenInfo(); OAuthTokenInfo accessTokenInfo = new OAuthTokenInfo();
accessTokenInfo.username = a.getUserName(); accessTokenInfo.username = a.getUserName();
accessTokenInfo.resourceHost = hostName; accessTokenInfo.resourceHost = getHostName(canonicalWebUrlProvider.get());
accessTokenInfo.accessToken = accessToken.getToken(); accessTokenInfo.accessToken = accessToken.getToken();
accessTokenInfo.providerId = accessToken.getProviderId(); accessTokenInfo.providerId = accessToken.getProviderId();
accessTokenInfo.expiresAt = Long.toString(accessToken.getExpiresAt()); accessTokenInfo.expiresAt = Long.toString(accessToken.getExpiresAt());

8
gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginLoader.java
View File

@ -17,6 +17,7 @@ package com.google.gerrit.server.plugins;
import com.google.common.base.CharMatcher; import com.google.common.base.CharMatcher;
import com.google.common.base.Joiner; import com.google.common.base.Joiner;
import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.collect.ComparisonChain; import com.google.common.collect.ComparisonChain;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
@ -580,9 +581,14 @@ public class PluginLoader implements LifecycleListener {
} }
private String getPluginCanonicalWebUrl(String name) { private String getPluginCanonicalWebUrl(String name) {
String canonicalWebUrl = urlProvider.get();
if (Strings.isNullOrEmpty(canonicalWebUrl)) {
return "/plugins/" + name;
}
String url = String url =
String.format( String.format(
"%s/plugins/%s/", CharMatcher.is('/').trimTrailingFrom(urlProvider.get()), name); "%s/plugins/%s/", CharMatcher.is('/').trimTrailingFrom(canonicalWebUrl), name);
return url; return url;
} }