diff --git a/gradle/changelog/omit_default_port.yaml b/gradle/changelog/omit_default_port.yaml new file mode 100644 index 0000000000..b1536d53bf --- /dev/null +++ b/gradle/changelog/omit_default_port.yaml @@ -0,0 +1,2 @@ +- type: changed + description: Omit default port in protocol urls ([#2014](https://github.com/scm-manager/scm-manager/pull/2014)) diff --git a/scm-webapp/src/main/java/sonia/scm/DefaultRootURL.java b/scm-webapp/src/main/java/sonia/scm/DefaultRootURL.java index fb21e7b6f0..8c4a53e9a2 100644 --- a/scm-webapp/src/main/java/sonia/scm/DefaultRootURL.java +++ b/scm-webapp/src/main/java/sonia/scm/DefaultRootURL.java @@ -24,6 +24,10 @@ package sonia.scm; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; import com.google.inject.OutOfScopeException; import com.google.inject.ProvisionException; import org.slf4j.Logger; @@ -37,6 +41,7 @@ import javax.servlet.http.HttpServletRequest; import java.net.MalformedURLException; import java.net.URL; import java.util.Optional; +import java.util.concurrent.ExecutionException; /** * Default implementation of {@link RootURL}. @@ -50,21 +55,29 @@ public class DefaultRootURL implements RootURL { private final Provider requestProvider; private final ScmConfiguration configuration; + private final LoadingCache urlCache; + @Inject public DefaultRootURL(Provider requestProvider, ScmConfiguration configuration) { + this(requestProvider, configuration, new UrlFromString()); + } + + @VisibleForTesting + DefaultRootURL(Provider requestProvider, ScmConfiguration configuration, UrlFromString cacheLoader) { this.requestProvider = requestProvider; this.configuration = configuration; + this.urlCache = CacheBuilder.newBuilder().maximumSize(10).build(cacheLoader); } @Override public URL get() { - String url = fromRequest().orElse(configuration.getBaseUrl()); + String url = fromRequest().orElseGet(configuration::getBaseUrl); if (url == null) { throw new IllegalStateException("The configured base url is empty. This can only happened if SCM-Manager has not received any requests."); } try { - return new URL(url); - } catch (MalformedURLException e) { + return urlCache.get(url); + } catch (ExecutionException e) { throw new IllegalStateException(String.format("base url \"%s\" is malformed", url), e); } } @@ -81,4 +94,16 @@ public class DefaultRootURL implements RootURL { throw ex; } } + + @VisibleForTesting + static class UrlFromString extends CacheLoader { + @Override + public URL load(String urlString) throws MalformedURLException { + URL url = new URL(urlString); + if (url.getPort() == url.getDefaultPort()) { + return new URL(url.getProtocol(), url.getHost(), -1, url.getFile()); + } + return url; + } + } } diff --git a/scm-webapp/src/test/java/sonia/scm/DefaultRootURLTest.java b/scm-webapp/src/test/java/sonia/scm/DefaultRootURLTest.java index 4190c6288a..e8a6a457f2 100644 --- a/scm-webapp/src/test/java/sonia/scm/DefaultRootURLTest.java +++ b/scm-webapp/src/test/java/sonia/scm/DefaultRootURLTest.java @@ -30,6 +30,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; +import org.mockito.Spy; import org.mockito.junit.jupiter.MockitoExtension; import sonia.scm.config.ScmConfiguration; import sonia.scm.util.HttpUtil; @@ -40,6 +41,8 @@ import java.net.MalformedURLException; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -56,12 +59,14 @@ class DefaultRootURLTest { private ScmConfiguration configuration; - private RootURL rootURL; + private DefaultRootURL rootURL; + @Spy + private DefaultRootURL.UrlFromString cacheLoader; @BeforeEach void init() { configuration = new ScmConfiguration(); - rootURL = new DefaultRootURL(requestProvider, configuration); + rootURL = new DefaultRootURL(requestProvider, configuration, cacheLoader); } @Test @@ -84,6 +89,13 @@ class DefaultRootURLTest { assertThat(rootURL.getAsString()).isEqualTo(URL_CONFIG); } + @Test + void shouldSuppressDefaultPorts() { + bindNonHttpScope(); + configuration.setBaseUrl("https://hitchhiker.com:443/from-configuration"); + assertThat(rootURL.getAsString()).isEqualTo(URL_CONFIG); + } + private void bindNonHttpScope() { when(requestProvider.get()).thenThrow( new ProvisionException("no request available", new OutOfScopeException("out of scope")) @@ -106,7 +118,6 @@ class DefaultRootURLTest { IllegalStateException exception = assertThrows(IllegalStateException.class, () -> rootURL.get()); assertThat(exception.getMessage()).contains("malformed", "non_url"); - assertThat(exception.getCause()).isInstanceOf(MalformedURLException.class); } @Test @@ -123,6 +134,16 @@ class DefaultRootURLTest { assertThat(rootURL.get()).hasHost("hitchhiker.com"); } + @Test + void shouldUseUrlCache() throws MalformedURLException { + bindForwardedRequestUrl(); + + rootURL.get(); + rootURL.get(); + + verify(cacheLoader).load(any()); + } + private void bindForwardedRequestUrl() { when(requestProvider.get()).thenReturn(request); when(request.getHeader(HttpUtil.HEADER_X_FORWARDED_HOST)).thenReturn("hitchhiker.com");