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;
+ }
}
}
