Fix OutOfScopeException getting the serverName from HTTP request Whenever the caller was coming from an HTTP thread that was not coming from a Guice filter (e.g. GitHub OAuth filter) the request for the canonicalWebUrl was throwing an OutOfScopeException. One sample scenario was the following stack of calls: c.g.g.m.v.VirtualHostHttpCanonicalWebUrlProvider.lambda$getServerName java.base/java.util.Optional.map c.g.g.m.v.VirtualHostHttpCanonicalWebUrlProvider.getServerName c.g.g.m.v.VirtualHostHttpCanonicalWebUrlProvider.get c.g.g.p.g.o.CanonicalWebUrls.getCannonicalWebUrl c.g.g.p.g.o.CanonicalWebUrls.getOAuthFinalRedirectUrl c.g.g.p.g.o.OAuthProtocol.getAuthorizationUrl c.g.g.p.g.o.OAuthProtocol.loginPhase1 c.g.g.p.g.o.GitHubLogin.login c.g.g.p.g.o.OAuthWebFilter.login c.g.g.p.g.o.OAuthWebFilter.doFilter c.g.g.p.g.o.OAuthFilter.doFilter org.eclipse.jetty.servlet.FilterHolder.doFilter Because the request for a canonical web url was coming from OAuthFilter.doFilter invoked directly from Jetty, the Guice filter did not manage to inject the current request, making the lamba execution of the map to throw an unchecked exception. By swapping the check between the local thread the scoped HTTP request and catching the OutOfScopeException, the above condition would just return into an empty server name. Change-Id: Ie645fa3e07d128d6fbdbd0624196e7c43d35929c
diff --git a/src/main/java/com/gerritforge/gerrit/modules/virtualhost/VirtualHostHttpCanonicalWebUrlProvider.java b/src/main/java/com/gerritforge/gerrit/modules/virtualhost/VirtualHostHttpCanonicalWebUrlProvider.java index c2f19e0..77e5995 100644 --- a/src/main/java/com/gerritforge/gerrit/modules/virtualhost/VirtualHostHttpCanonicalWebUrlProvider.java +++ b/src/main/java/com/gerritforge/gerrit/modules/virtualhost/VirtualHostHttpCanonicalWebUrlProvider.java
@@ -15,19 +15,24 @@ package com.gerritforge.gerrit.modules.virtualhost; import com.google.common.annotations.VisibleForTesting; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.httpd.HttpCanonicalWebUrlProvider; import com.google.gerrit.server.config.GerritServerConfig; import com.google.inject.Inject; +import com.google.inject.OutOfScopeException; import com.google.inject.Provider; import com.google.inject.Singleton; import java.net.URI; import java.util.Optional; +import javax.annotation.Nullable; import javax.servlet.http.HttpServletRequest; import org.apache.http.client.utils.URIBuilder; import org.eclipse.jgit.lib.Config; @Singleton public class VirtualHostHttpCanonicalWebUrlProvider extends HttpCanonicalWebUrlProvider { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private final URI serverUri; private Provider<HttpServletRequest> requestProvider; @@ -56,8 +61,21 @@ } private Optional<String> getServerName() { - return Optional.ofNullable(requestProvider) - .map(provider -> provider.get().getServerName()) - .or(CurrentServerName::get); + return CurrentServerName.get() + .or( + () -> + Optional.ofNullable(requestProvider) + .map(VirtualHostHttpCanonicalWebUrlProvider::extractServerNameWhenInScope)); + } + + @Nullable + private static String extractServerNameWhenInScope(Provider<HttpServletRequest> provider) { + try { + return provider.get().getServerName(); + } catch (OutOfScopeException e) { + logger.atWarning().withCause(e).log( + "Unable to determine the virtual-host servername: current thread is out of an HTTP request scope or outside a call stack coming from a GuiceServlet filter"); + return null; + } } }