Preserve request method on force base url (#1778)

The redirect which is used to force base url uses now 307 instead of 302 in order to preserve the request method. Closes #1771
2026-01-28 18:29:10 +01:00 · 2021-08-23 20:02:12 +02:00
parent 7f9f4e566c
commit da2b34e528
3 changed files with 146 additions and 110 deletions
--- a/gradle/changelog/force_baseurl.yaml
+++ b/gradle/changelog/force_baseurl.yaml
@@ -0,0 +1,2 @@
+- type: fixed
+  description: Preserve request method on force base url ([#1771](https://github.com/scm-manager/scm-manager/issues/1771) and [#1778](https://github.com/scm-manager/scm-manager/pull/1778))
--- a/scm-webapp/src/main/java/sonia/scm/filter/BaseUrlFilter.java
+++ b/scm-webapp/src/main/java/sonia/scm/filter/BaseUrlFilter.java
@@ -21,10 +21,8 @@
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 * SOFTWARE.
 */
-    
-package sonia.scm.filter;

-//~--- non-JDK imports --------------------------------------------------------
+package sonia.scm.filter;

 import com.google.common.annotations.VisibleForTesting;
 import com.google.inject.Inject;
@@ -34,8 +32,6 @@ import sonia.scm.util.HttpUtil;
 import sonia.scm.util.Util;
 import sonia.scm.web.filter.HttpFilter;

-//~--- JDK imports ------------------------------------------------------------
-
 import java.io.IOException;

 import javax.servlet.FilterChain;
@@ -50,110 +46,49 @@ import sonia.scm.Priority;
 */
@Priority(Filters.PRIORITY_BASEURL)
@WebElement(Filters.PATTERN_ALL)
-public class BaseUrlFilter extends HttpFilter
-{
+public class BaseUrlFilter extends HttpFilter {
+
+  /** scm configuration */
+  private final ScmConfiguration configuration;

-  /**
-   * Constructs ...
-   *
-   *
-   * @param configuration
-   */
  @Inject
-  public BaseUrlFilter(ScmConfiguration configuration)
-  {
+  public BaseUrlFilter(ScmConfiguration configuration) {
    this.configuration = configuration;
  }

-  //~--- methods --------------------------------------------------------------
-
-  /**
-   * Method description
-   *
-   *
-   * @param requestUrl
-   * @param baseUrl
-   *
-   * @return
-   */
  @VisibleForTesting
-  boolean startsWith(String requestUrl, String baseUrl)
-  {
-    return HttpUtil.normalizeUrl(requestUrl).startsWith(
-      HttpUtil.normalizeUrl(baseUrl));
+  boolean startsWith(String requestUrl, String baseUrl) {
+    return HttpUtil.normalizeUrl(requestUrl).startsWith(HttpUtil.normalizeUrl(baseUrl));
  }

-  /**
-   * Method description
-   *
-   *
-   * @param request
-   * @param response
-   * @param chain
-   *
-   * @throws IOException
-   * @throws ServletException
-   */
  @Override
-  protected void doFilter(HttpServletRequest request,
-    HttpServletResponse response, FilterChain chain)
-    throws IOException, ServletException
-  {
-    if (Util.isEmpty(configuration.getBaseUrl()))
-    {
+  protected void doFilter(
+    HttpServletRequest request, HttpServletResponse response, FilterChain chain
+  ) throws IOException, ServletException {
+    if (Util.isEmpty(configuration.getBaseUrl())) {
      configuration.setBaseUrl(createDefaultBaseUrl(request));
    }

-    if (!configuration.isForceBaseUrl() || isBaseUrl(request))
-    {
+    if (!configuration.isForceBaseUrl() || isBaseUrl(request)) {
      chain.doFilter(request, response);
-    }
-    else
-    {
-      String url = HttpUtil.getCompleteUrl(configuration,
-                     HttpUtil.getStrippedURI(request));
-
-      response.sendRedirect(url);
+    } else {
+      String url = HttpUtil.getCompleteUrl(configuration, HttpUtil.getStrippedURI(request));
+      response.setStatus(HttpServletResponse.SC_TEMPORARY_REDIRECT);
+      response.setHeader(HttpUtil.HEADER_LOCATION, url);
    }
  }

-  /**
-   * Method description
-   *
-   *
-   * @param request
-   *
-   * @return
-   */
-  private String createDefaultBaseUrl(HttpServletRequest request)
-  {
+  private String createDefaultBaseUrl(HttpServletRequest request) {
    StringBuilder sb = new StringBuilder(request.getScheme());

    sb.append("://").append(request.getServerName()).append(":");
-    sb.append(String.valueOf(request.getServerPort()));
+    sb.append(request.getServerPort());
    sb.append(request.getContextPath());

    return sb.toString();
  }

-  //~--- get methods ----------------------------------------------------------
-
-  /**
-   * Method description
-   *
-   *
-   * @param request
-   *
-   * @return
-   */
-  private boolean isBaseUrl(HttpServletRequest request)
-  {
-    return startsWith(request.getRequestURL().toString(),
-      configuration.getBaseUrl());
+  private boolean isBaseUrl(HttpServletRequest request) {
+    return startsWith(request.getRequestURL().toString(), configuration.getBaseUrl());
  }
-
-  //~--- fields ---------------------------------------------------------------
-
-  /** scm configuration */
-  private final ScmConfiguration configuration;
 }
--- a/scm-webapp/src/test/java/sonia/scm/filter/BaseUrlFilterTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/filter/BaseUrlFilterTest.java
@@ -21,40 +21,139 @@
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 * SOFTWARE.
 */
-    
+
 package sonia.scm.filter;

-//~--- non-JDK imports --------------------------------------------------------
-
-import org.junit.Test;
-
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.config.ScmConfiguration;

-import static org.junit.Assert.*;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import java.io.IOException;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+

 /**
- *
 * @author Sebastian Sdorra
 */
-public class BaseUrlFilterTest
-{
+@ExtendWith(MockitoExtension.class)
+class BaseUrlFilterTest {
+
+
+  @Mock
+  private HttpServletResponse response;
+
+  @Mock
+  private FilterChain chain;
+
+  private ScmConfiguration configuration;
+
+  private BaseUrlFilter filter;
+
+  @BeforeEach
+  void setUpFilter() {
+    configuration = new ScmConfiguration();
+    filter = new BaseUrlFilter(configuration);
+  }

-  /**
-   * Method description
-   *
-   */
  @Test
-  public void startsWithTest()
-  {
-    BaseUrlFilter filter = new BaseUrlFilter(new ScmConfiguration());
+  void shouldSetBaseUrl() throws ServletException, IOException {
+    HttpServletRequest request = mockRequest("https", "hitchhiker.com", 443, "/scm");
+
+    filter.doFilter(request, response, chain);
+
+    verify(chain).doFilter(request, response);
+    assertThat(configuration.getBaseUrl()).isEqualTo("https://hitchhiker.com:443/scm");
+  }
+
+  @Test
+  void shouldSendRedirect() throws ServletException, IOException {
+    configuration.setBaseUrl("https://hitchhiker.com:443/scm");
+    configuration.setForceBaseUrl(true);
+    HttpServletRequest request = mockRequest("http://192.168.1.42:8081", "/scm", "/api/v2");
+
+    filter.doFilter(request, response, chain);
+
+    verifyNoInteractions(chain);
+    verify(response).setStatus(307);
+    verify(response).setHeader("Location", "https://hitchhiker.com:443/scm/api/v2");
+  }
+
+  @Test
+  void shouldNotSendRedirectIfDisabled() throws ServletException, IOException {
+    configuration.setBaseUrl("https://hitchhiker.com:443/scm");
+    configuration.setForceBaseUrl(false);
+    HttpServletRequest request = mockRequest("http://192.168.1.42:8081", "/scm", "/api/v2");
+
+    filter.doFilter(request, response, chain);
+
+    verify(chain).doFilter(request, response);
+  }
+
+  @Test
+  void shouldNotSendRedirect() throws ServletException, IOException {
+    configuration.setBaseUrl("https://hitchhiker.com:443/scm");
+    configuration.setForceBaseUrl(true);
+    HttpServletRequest request = mockRequest("https://hitchhiker.com", "/scm", "/api/v2/users");
+
+    filter.doFilter(request, response, chain);
+
+    verify(chain).doFilter(request, response);
+  }
+
+  private HttpServletRequest mockRequest(String baseUrl, String contextPath, String path) {
+    HttpServletRequest request = mock(HttpServletRequest.class);
+    lenient().when(request.getRequestURL()).thenReturn(new StringBuffer(baseUrl).append(contextPath).append(path));
+    lenient().when(request.getContextPath()).thenReturn(contextPath);
+    lenient().when(request.getRequestURI()).thenReturn(contextPath + path);
+    return request;
+  }
+
+  private HttpServletRequest mockRequest(String scheme, String serverName, int port, String contextPath) {
+    HttpServletRequest request = mock(HttpServletRequest.class);
+    when(request.getScheme()).thenReturn(scheme);
+    when(request.getServerName()).thenReturn(serverName);
+    when(request.getServerPort()).thenReturn(port);
+    when(request.getContextPath()).thenReturn(contextPath);
+    return request;
+  }
+
+  @Nested
+  class StartsWithTests {
+
+    @Test
+    void shouldReturnTrue() {
+      assertThat(
+        filter.startsWith("http://www.scm-manager.org/scm", "http://www.scm-manager.org/scm")
+      ).isTrue();
+      assertThat(
+        filter.startsWith("http://www.scm-manager.org:80/scm", "http://www.scm-manager.org/scm")
+      ).isTrue();
+      assertThat(
+        filter.startsWith("https://www.scm-manager.org/scm", "https://www.scm-manager.org:443/scm")
+      ).isTrue();
+    }
+
+    @Test
+    void shouldReturnFalse() {
+      assertThat(
+        filter.startsWith("http://www.scm-manager.org/acb", "http://www.scm-manager.org/scm")
+      ).isFalse();
+    }

-    assertTrue(filter.startsWith("http://www.scm-manager.org/scm",
-      "http://www.scm-manager.org/scm"));
-    assertTrue(filter.startsWith("http://www.scm-manager.org:80/scm",
-      "http://www.scm-manager.org/scm"));
-    assertTrue(filter.startsWith("https://www.scm-manager.org/scm",
-      "https://www.scm-manager.org:443/scm"));
-    assertFalse(filter.startsWith("http://www.scm-manager.org/acb",
-      "http://www.scm-manager.org/scm"));
  }
 }