diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/filters/RemoteAddressFilter.java b/service/src/main/java/org/whispersystems/textsecuregcm/filters/RemoteAddressFilter.java index 750cb9b47..d1f94cf41 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/filters/RemoteAddressFilter.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/filters/RemoteAddressFilter.java @@ -5,17 +5,20 @@ package org.whispersystems.textsecuregcm.filters; +import com.google.common.annotations.VisibleForTesting; +import java.io.IOException; +import java.util.Optional; +import javax.annotation.Nullable; import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.whispersystems.textsecuregcm.util.HeaderUtils; import org.whispersystems.textsecuregcm.util.HttpServletRequestUtil; -import java.io.IOException; /** * Sets a {@link HttpServletRequest} attribute (that will also be available as a @@ -47,7 +50,7 @@ public class RemoteAddressFilter implements Filter { remoteAddress = HttpServletRequestUtil.getRemoteAddress(httpServletRequest); } else { final String forwardedFor = httpServletRequest.getHeader(com.google.common.net.HttpHeaders.X_FORWARDED_FOR); - remoteAddress = HeaderUtils.getMostRecentProxy(forwardedFor) + remoteAddress = getMostRecentProxy(forwardedFor) .orElseGet(() -> HttpServletRequestUtil.getRemoteAddress(httpServletRequest)); } @@ -59,4 +62,25 @@ public class RemoteAddressFilter implements Filter { chain.doFilter(request, response); } + + /** + * Returns the most recent proxy in a chain described by an {@code X-Forwarded-For} header. + * + * @param forwardedFor the value of an X-Forwarded-For header + * @return the IP address of the most recent proxy in the forwarding chain, or empty if none was found or + * {@code forwardedFor} was null + * @see X-Forwarded-For - HTTP | + * MDN + */ + @VisibleForTesting + static Optional getMostRecentProxy(@Nullable final String forwardedFor) { + return Optional.ofNullable(forwardedFor) + .map(ff -> { + final int idx = forwardedFor.lastIndexOf(',') + 1; + return idx < forwardedFor.length() + ? forwardedFor.substring(idx).trim() + : null; + }) + .filter(StringUtils::isNotBlank); + } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/HeaderUtils.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/HeaderUtils.java index 01ff4928a..c4aa3f622 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/HeaderUtils.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/HeaderUtils.java @@ -12,8 +12,6 @@ import java.nio.charset.StandardCharsets; import java.util.Base64; import java.util.Optional; import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import org.apache.commons.lang3.StringUtils; import org.whispersystems.textsecuregcm.auth.ExternalServiceCredentials; public final class HeaderUtils { @@ -47,28 +45,6 @@ public final class HeaderUtils { return TIMESTAMP_HEADER + ":" + System.currentTimeMillis(); } - /** - * Returns the most recent proxy in a chain described by an {@code X-Forwarded-For} header. - * - * @param forwardedFor the value of an X-Forwarded-For header - * - * @return the IP address of the most recent proxy in the forwarding chain, or empty if none was found or - * {@code forwardedFor} was null - * - * @see X-Forwarded-For - HTTP | MDN - */ - @Nonnull - public static Optional getMostRecentProxy(@Nullable final String forwardedFor) { - return Optional.ofNullable(forwardedFor) - .map(ff -> { - final int idx = forwardedFor.lastIndexOf(',') + 1; - return idx < forwardedFor.length() - ? forwardedFor.substring(idx).trim() - : null; - }) - .filter(StringUtils::isNotBlank); - } - /** * Parses a Base64-encoded value of the `Authorization` header * in the form of `Basic dXNlcm5hbWU6cGFzc3dvcmQ=`. diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/filters/RemoteAddressFilterTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/filters/RemoteAddressFilterTest.java index 1fdf20995..10e9effb9 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/filters/RemoteAddressFilterTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/filters/RemoteAddressFilterTest.java @@ -5,18 +5,24 @@ package org.whispersystems.textsecuregcm.filters; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.params.provider.Arguments.arguments; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.common.net.HttpHeaders; +import java.util.Optional; +import java.util.stream.Stream; import javax.servlet.FilterChain; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.MethodSource; class RemoteAddressFilterTest { @@ -57,4 +63,23 @@ class RemoteAddressFilterTest { verify(filterChain).doFilter(any(ServletRequest.class), any(ServletResponse.class)); } + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") + @ParameterizedTest + @MethodSource("argumentsForGetMostRecentProxy") + void getMostRecentProxy(final String forwardedFor, final Optional expectedMostRecentProxy) { + assertEquals(expectedMostRecentProxy, RemoteAddressFilter.getMostRecentProxy(forwardedFor)); + } + + private static Stream argumentsForGetMostRecentProxy() { + return Stream.of( + arguments(null, Optional.empty()), + arguments("", Optional.empty()), + arguments(" ", Optional.empty()), + arguments("203.0.113.195,", Optional.empty()), + arguments("203.0.113.195, ", Optional.empty()), + arguments("203.0.113.195", Optional.of("203.0.113.195")), + arguments("203.0.113.195, 70.41.3.18, 150.172.238.178", Optional.of("150.172.238.178")) + ); + } + } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/util/HeaderUtilsTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/util/HeaderUtilsTest.java deleted file mode 100644 index 92f2a0b9f..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/util/HeaderUtilsTest.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright 2021 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.util; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.params.provider.Arguments.arguments; - -import java.util.Optional; -import java.util.stream.Stream; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; - -class HeaderUtilsTest { - - @SuppressWarnings("OptionalUsedAsFieldOrParameterType") - @ParameterizedTest - @MethodSource("argumentsForGetMostRecentProxy") - void getMostRecentProxy(final String forwardedFor, final Optional expectedMostRecentProxy) { - assertEquals(expectedMostRecentProxy, HeaderUtils.getMostRecentProxy(forwardedFor)); - } - - private static Stream argumentsForGetMostRecentProxy() { - return Stream.of( - arguments(null, Optional.empty()), - arguments("", Optional.empty()), - arguments(" ", Optional.empty()), - arguments("203.0.113.195,", Optional.empty()), - arguments("203.0.113.195, ", Optional.empty()), - arguments("203.0.113.195", Optional.of("203.0.113.195")), - arguments("203.0.113.195, 70.41.3.18, 150.172.238.178", Optional.of("150.172.238.178")) - ); - } -}