Skip to content

Commit

Permalink
Fix parsing of ipv6 addresses in HydraEmbeddedServletContainer
Browse files Browse the repository at this point in the history
Closes #980
  • Loading branch information
theotherp committed Jan 3, 2025
1 parent d43c576 commit 201f684
Show file tree
Hide file tree
Showing 4 changed files with 233 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
package org.nzbhydra.auth;

import jakarta.servlet.ServletException;
import org.apache.catalina.Valve;
import org.apache.catalina.connector.Request;
import org.apache.catalina.connector.Response;
import org.apache.catalina.valves.ValveBase;
import org.apache.logging.log4j.util.Strings;
import org.apache.tomcat.util.buf.MessageBytes;
import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactory;
Expand All @@ -48,64 +50,75 @@ public void customize(ConfigurableServletWebServerFactory factory) {
containerFactory.addContextValves(new ValveBase() {
@Override
public void invoke(Request request, Response response) throws IOException, ServletException {
int originalPort = -1;
final String forwardedPort = request.getHeader("X-Forwarded-Port");
if (forwardedPort != null) {
try {
originalPort = request.getServerPort();
request.setServerPort(Integer.parseInt(forwardedPort));
} catch (final NumberFormatException e) {
logger.debug("ignoring forwarded port {}", forwardedPort);
}
}

final MessageBytes serverNameMB = request.getCoyoteRequest().serverName();
String originalServerName = null;
String forwardedHost = request.getHeader("X-Forwarded-Host");
if (forwardedHost == null) {
forwardedHost = request.getHeader("host");
}
if (Strings.isNotBlank(forwardedHost)) {
String[] split = forwardedHost.split("[ ,]");
forwardedHost = split[0];
int colonIndex = forwardedHost.indexOf(":");
if (colonIndex > -1) {
if (originalPort == -1) {
originalPort = request.getServerPort();
}
request.setServerPort(Integer.parseInt(forwardedHost.substring(colonIndex + 1)));
forwardedHost = forwardedHost.substring(0, colonIndex);
}
originalServerName = serverNameMB.getString();
serverNameMB.setString(forwardedHost);
}

Boolean originallySecure = null;
final String forwardedProto = request.getHeader("X-Forwarded-Proto");
if (forwardedProto != null) {
originallySecure = request.isSecure();
request.setSecure(forwardedProto.equalsIgnoreCase("https"));
}

Valve nextValve = getNext();
Result result = parseRequest(request);
try {
getNext().invoke(request, response);
nextValve.invoke(request, response);
} finally {
if (originallySecure != null) {
request.setSecure(originallySecure);
if (result.originallySecure() != null) {
request.setSecure(result.originallySecure());
}
if (forwardedHost != null) {
serverNameMB.setString(originalServerName);
if (result.forwardedHost() != null) {
result.serverNameMB().setString(result.originalServerName());
}
if (forwardedPort != null) {
request.setServerPort(originalPort);
if (result.forwardedPort() != null) {
request.setServerPort(result.originalPort());
}

}

}


});
containerFactory.addContextCustomizers(context -> context.setMapperContextRootRedirectEnabled(true));
}

@NotNull
static Result parseRequest(Request request) {
int originalPort = -1;
final String forwardedPort = request.getHeader("X-Forwarded-Port");
if (forwardedPort != null) {
try {
originalPort = request.getServerPort();
request.setServerPort(Integer.parseInt(forwardedPort));
} catch (final NumberFormatException e) {
logger.debug("ignoring forwarded port {}", forwardedPort);
}
}

final MessageBytes serverNameMB = request.getCoyoteRequest().serverName();
String originalServerName = null;
String forwardedHost = request.getHeader("X-Forwarded-Host");
if (forwardedHost == null) {
forwardedHost = request.getHeader("host");
}
if (Strings.isNotBlank(forwardedHost)) {
String[] split = forwardedHost.split("[ ,]");
forwardedHost = split[0];
int colonIndex = forwardedHost.lastIndexOf(":");
if (colonIndex > -1) {
if (originalPort == -1) {
originalPort = request.getServerPort();
}
request.setServerPort(Integer.parseInt(forwardedHost.substring(colonIndex + 1)));
forwardedHost = forwardedHost.substring(0, colonIndex);
}
originalServerName = serverNameMB.getString();
serverNameMB.setString(forwardedHost);
}

Boolean originallySecure = null;
final String forwardedProto = request.getHeader("X-Forwarded-Proto");
if (forwardedProto != null) {
originallySecure = request.isSecure();
request.setSecure(forwardedProto.equalsIgnoreCase("https"));
}
Result result = new Result(originalPort, forwardedPort, serverNameMB, originalServerName, forwardedHost, originallySecure);
return result;
}

record Result(int originalPort, String forwardedPort, MessageBytes serverNameMB, String originalServerName, String forwardedHost, Boolean originallySecure) {
}


}
4 changes: 4 additions & 0 deletions core/src/main/resources/changelog.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
#@formatter:off

- version: "v7.11.3"
date: "2025-01-03"
changes:
- type: "fix"
text: "Repeating a TV search from the search history would instead search for all TV results."
- type: "fix"
text: "Filtering for 3D using the quick filter button filtered for the wrong term. See #977"
- type: "fix"
text: "IPv6 reverse proxies would result in error 500. See #980"
final: true
- version: "v7.11.2"
date: "2024-12-30"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
/*
* (C) Copyright 2025 TheOtherP ([email protected])
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.nzbhydra.auth;

import org.apache.catalina.connector.Request;
import org.apache.tomcat.util.buf.MessageBytes;
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.junit.jupiter.MockitoExtension;
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.*;
import static org.nzbhydra.auth.HydraEmbeddedServletContainer.parseRequest;

@ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.LENIENT)
class HydraEmbeddedServletContainerTest {

@Mock
private Request request;
@Mock
private MessageBytes serverNameMB;
@Mock
private org.apache.coyote.Request coyoteRequest;

@BeforeEach
void setUp() {
when(request.getCoyoteRequest()).thenReturn(coyoteRequest);
when(coyoteRequest.serverName()).thenReturn(serverNameMB);
when(request.getHeader("X-Forwarded-Port")).thenReturn(null);
when(request.getHeader("X-Forwarded-Host")).thenReturn(null);
when(request.getHeader("X-Forwarded-Proto")).thenReturn(null);
// when(request.getHeader("host")).thenReturn(null);
}

@Test
void shouldHandleForwardedPort() {
when(request.getHeader("X-Forwarded-Port")).thenReturn("8443");
when(request.getHeader("host")).thenReturn("host");
when(request.getServerPort()).thenReturn(8080);

HydraEmbeddedServletContainer.Result result = parseRequest(request);

verify(request).setServerPort(8443);
assertThat(result.originalPort()).isEqualTo(8080);
assertThat(result.forwardedPort()).isEqualTo("8443");
}

@Test
void shouldIgnoreInvalidForwardedPort() {
when(request.getHeader("X-Forwarded-Port")).thenReturn("invalid");
when(request.getServerPort()).thenReturn(8080);
when(request.getHeader("host")).thenReturn("host");

HydraEmbeddedServletContainer.Result result = parseRequest(request);

verify(request, never()).setServerPort(anyInt());
assertThat(result.originalPort()).isEqualTo(8080);
}

@Test
void shouldHandleForwardedHost() {
when(request.getHeader("X-Forwarded-Host")).thenReturn("example.com");
when(serverNameMB.getString()).thenReturn("original.com");

HydraEmbeddedServletContainer.Result result = parseRequest(request);

verify(serverNameMB).setString("example.com");
assertThat(result.originalServerName()).isEqualTo("original.com");
assertThat(result.forwardedHost()).isEqualTo("example.com");
}

@Test
void shouldHandleIpv6ForwardedHost() {
when(request.getHeader("X-Forwarded-Host")).thenReturn("[2001:db8:85a3:8d3:1319:8a2e:370:7348]:443");
when(serverNameMB.getString()).thenReturn("original.com");
when(request.getServerPort()).thenReturn(8080);

HydraEmbeddedServletContainer.Result result = parseRequest(request);

verify(request).setServerPort(443);
verify(serverNameMB).setString("[2001:db8:85a3:8d3:1319:8a2e:370:7348]");
assertThat(result.originalPort()).isEqualTo(8080);
assertThat(result.forwardedHost()).isEqualTo("[2001:db8:85a3:8d3:1319:8a2e:370:7348]");
}

@Test
void shouldHandleForwardedHostWithPort() {
when(request.getHeader("X-Forwarded-Host")).thenReturn("example.com:9443");
when(serverNameMB.getString()).thenReturn("original.com");
when(request.getServerPort()).thenReturn(8080);

HydraEmbeddedServletContainer.Result result = parseRequest(request);

verify(request).setServerPort(9443);
verify(serverNameMB).setString("example.com");
assertThat(result.originalPort()).isEqualTo(8080);
assertThat(result.forwardedHost()).isEqualTo("example.com");
}

@Test
void shouldFallbackToHostHeader() {
when(request.getHeader("X-Forwarded-Host")).thenReturn(null);
when(request.getHeader("host")).thenReturn("example.com");
when(serverNameMB.getString()).thenReturn("original.com");

HydraEmbeddedServletContainer.Result result = parseRequest(request);

verify(serverNameMB).setString("example.com");
assertThat(result.forwardedHost()).isEqualTo("example.com");
}

@Test
void shouldHandleMultipleForwardedHosts() {
when(request.getHeader("X-Forwarded-Host")).thenReturn("example.com, proxy.com");
when(serverNameMB.getString()).thenReturn("original.com");

HydraEmbeddedServletContainer.Result result = parseRequest(request);

verify(serverNameMB).setString("example.com");
assertThat(result.forwardedHost()).isEqualTo("example.com");
}

@Test
void shouldHandleForwardedProto() {
when(request.getHeader("X-Forwarded-Proto")).thenReturn("https");
when(request.getHeader("host")).thenReturn("host");
when(request.isSecure()).thenReturn(false);

HydraEmbeddedServletContainer.Result result = parseRequest(request);

verify(request).setSecure(true);
assertThat(result.originallySecure()).isFalse();
}

@Test
void shouldHandleNoHeaders() {
when(request.getHeader(anyString())).thenReturn(null);

HydraEmbeddedServletContainer.Result result = parseRequest(request);

verify(request, never()).setServerPort(anyInt());
verify(serverNameMB, never()).setString(anyString());
verify(request, never()).setSecure(anyBoolean());
assertThat(result.originalPort()).isEqualTo(-1);
assertThat(result.originalServerName()).isNull();
assertThat(result.originallySecure()).isNull();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
mock-maker-inline

0 comments on commit 201f684

Please sign in to comment.