Omit default port in protocol urls (#2014)

This omits the port in the protocol urls when the port is the default port for the protocol.
So if you have your server https://my.scm.net/scm and the repository admin/test, the protocol url is no longer https://my.scm.net:443/scm/repo/admin/test, but simply https://my.scm.net/scm/repo/admin/test without the :443.

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))

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<HttpServletRequest> requestProvider;
   private final ScmConfiguration configuration;
 
+  private final LoadingCache<String, URL> urlCache;
+
   @Inject
   public DefaultRootURL(Provider<HttpServletRequest> requestProvider, ScmConfiguration configuration) {
+    this(requestProvider, configuration, new UrlFromString());
+  }
+
+  @VisibleForTesting
+  DefaultRootURL(Provider<HttpServletRequest> 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<String, URL> {
+    @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;
+    }
+  }
 }

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");