Issue #5247 - Updates from review by greg

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
This commit is contained in:
Joakim Erdfelt 2020-09-21 10:34:28 -05:00
parent 385b11dc34
commit ebfdf8fc1b
No known key found for this signature in database
GPG Key ID: 2D0E1FB8FE4B68B4
2 changed files with 57 additions and 47 deletions

View File

@ -650,42 +650,48 @@ public class ForwardedRequestCustomizer implements Customizer
public static final int IMPLIED = 0; public static final int IMPLIED = 0;
String _host; String _host;
Priority _hostPriority = Priority.UNSET; Source _hostSource = Source.UNSET;
int _port = UNSET; int _port = UNSET;
Priority _portPriority = Priority.UNSET; Source _portSource = Source.UNSET;
public void setHost(String host, Priority priority) public void setHostPort(String host, int port, Source source)
{ {
if (priority.ordinal() > _hostPriority.ordinal()) setHost(host, source);
setPort(port, source);
}
public void setHost(String host, Source source)
{
if (source.priority() > _hostSource.priority())
{ {
_host = host; _host = host;
_hostPriority = priority; _hostSource = source;
} }
} }
public void setPort(int port, Priority priority) public void setPort(int port, Source source)
{ {
if (priority.ordinal() > _portPriority.ordinal()) if (source.priority() > _portSource.priority())
{ {
_port = port; _port = port;
_portPriority = priority; _portSource = source;
} }
} }
public void setHostPort(HostPort hostPort, Priority priority) public void setHostPort(HostPort hostPort, Source source)
{ {
if (priority.ordinal() > _hostPriority.ordinal()) if (source.priority() > _hostSource.priority())
{ {
_host = hostPort.getHost(); _host = hostPort.getHost();
_hostPriority = priority; _hostSource = source;
} }
int port = hostPort.getPort(); int port = hostPort.getPort();
// Is port supplied? // Is port supplied?
if (port > 0 && priority.ordinal() > _portPriority.ordinal()) if (port > 0 && source.priority() > _portSource.priority())
{ {
_port = hostPort.getPort(); _port = hostPort.getPort();
_portPriority = priority; _portSource = source;
} }
// Since we are Host:Port pair, the port could be unspecified // Since we are Host:Port pair, the port could be unspecified
// Meaning it's implied. // Meaning it's implied.
@ -701,21 +707,21 @@ public class ForwardedRequestCustomizer implements Customizer
public String toString() public String toString()
{ {
final StringBuilder sb = new StringBuilder("MutableHostPort{"); final StringBuilder sb = new StringBuilder("MutableHostPort{");
sb.append("host='").append(_host).append("'/").append(_hostPriority); sb.append("host='").append(_host).append("'/").append(_hostSource);
sb.append(", port=").append(_port); sb.append(", port=").append(_port);
sb.append("/").append(_portPriority); sb.append("/").append(_portSource);
sb.append('}'); sb.append('}');
return sb.toString(); return sb.toString();
} }
} }
/** /**
* Ordered Priority Enum. * Ordered Source Enum.
* <p> * <p>
* Lowest priority first. * Lowest first, Last/Highest priority wins
* </p> * </p>
*/ */
public enum Priority public enum Source
{ {
UNSET, UNSET,
XPROXIED_HTTPS, XPROXIED_HTTPS,
@ -725,7 +731,12 @@ public class ForwardedRequestCustomizer implements Customizer
XFORWARDED_FOR, XFORWARDED_FOR,
XFORWARDED_HOST, XFORWARDED_HOST,
FORWARDED, FORWARDED,
FORCED FORCED;
int priority()
{
return ordinal();
}
} }
private class Forwarded extends QuotedCSVParser private class Forwarded extends QuotedCSVParser
@ -736,7 +747,7 @@ public class ForwardedRequestCustomizer implements Customizer
MutableHostPort _authority; MutableHostPort _authority;
MutableHostPort _for; MutableHostPort _for;
String _proto; String _proto;
Priority _protoPriority = Priority.UNSET; Source _protoSource = Source.UNSET;
Boolean _secure; Boolean _secure;
public Forwarded(Request request, HttpConfiguration config) public Forwarded(Request request, HttpConfiguration config)
@ -746,8 +757,10 @@ public class ForwardedRequestCustomizer implements Customizer
_config = config; _config = config;
if (_forcedHost != null) if (_forcedHost != null)
{ {
getAuthority().setHost(_forcedHost.getHostPort().getHost(), Priority.FORCED); getAuthority().setHostPort(
getAuthority().setPort(_forcedHost.getHostPort().getPort(), Priority.FORCED); _forcedHost.getHostPort().getHost(),
_forcedHost.getHostPort().getPort(),
Source.FORCED);
} }
} }
@ -802,14 +815,14 @@ public class ForwardedRequestCustomizer implements Customizer
@SuppressWarnings("unused") @SuppressWarnings("unused")
public void handleForwardedHost(HttpField field) public void handleForwardedHost(HttpField field)
{ {
updateAuthority(getLeftMost(field.getValue()), Priority.XFORWARDED_HOST); updateAuthority(getLeftMost(field.getValue()), Source.XFORWARDED_HOST);
} }
@SuppressWarnings("unused") @SuppressWarnings("unused")
public void handleForwardedFor(HttpField field) public void handleForwardedFor(HttpField field)
{ {
HostPort hostField = new HostPort(getLeftMost(field.getValue())); HostPort hostField = new HostPort(getLeftMost(field.getValue()));
getFor().setHostPort(hostField, Priority.XFORWARDED_FOR); getFor().setHostPort(hostField, Source.XFORWARDED_FOR);
} }
@SuppressWarnings("unused") @SuppressWarnings("unused")
@ -817,7 +830,7 @@ public class ForwardedRequestCustomizer implements Customizer
{ {
if (getProxyAsAuthority()) if (getProxyAsAuthority())
return; return;
updateAuthority(getLeftMost(field.getValue()), Priority.XFORWARDED_SERVER); updateAuthority(getLeftMost(field.getValue()), Source.XFORWARDED_SERVER);
} }
@SuppressWarnings("unused") @SuppressWarnings("unused")
@ -825,13 +838,13 @@ public class ForwardedRequestCustomizer implements Customizer
{ {
int port = HostPort.parsePort(getLeftMost(field.getValue())); int port = HostPort.parsePort(getLeftMost(field.getValue()));
updatePort(port, Priority.XFORWARDED_PORT); updatePort(port, Source.XFORWARDED_PORT);
} }
@SuppressWarnings("unused") @SuppressWarnings("unused")
public void handleProto(HttpField field) public void handleProto(HttpField field)
{ {
updateProto(getLeftMost(field.getValue()), Priority.XFORWARDED_PROTO); updateProto(getLeftMost(field.getValue()), Source.XFORWARDED_PROTO);
} }
@SuppressWarnings("unused") @SuppressWarnings("unused")
@ -840,8 +853,8 @@ public class ForwardedRequestCustomizer implements Customizer
if ("on".equalsIgnoreCase(field.getValue()) || "true".equalsIgnoreCase(field.getValue())) if ("on".equalsIgnoreCase(field.getValue()) || "true".equalsIgnoreCase(field.getValue()))
{ {
_secure = true; _secure = true;
updateProto(HttpScheme.HTTPS.asString(), Priority.XPROXIED_HTTPS); updateProto(HttpScheme.HTTPS.asString(), Source.XPROXIED_HTTPS);
updatePort(getSecurePort(_config), Priority.XPROXIED_HTTPS); updatePort(getSecurePort(_config), Source.XPROXIED_HTTPS);
} }
} }
@ -867,8 +880,7 @@ public class ForwardedRequestCustomizer implements Customizer
if (value.startsWith("_") || "unknown".equals(value)) if (value.startsWith("_") || "unknown".equals(value))
break; break;
HostPort hostField = new HostPort(value); HostPort hostField = new HostPort(value);
getAuthority().setHost(hostField.getHost(), Priority.FORWARDED); getAuthority().setHostPort(hostField.getHost(), hostField.getPort(), Source.FORWARDED);
getAuthority().setPort(hostField.getPort(), Priority.FORWARDED);
break; break;
} }
case "for": case "for":
@ -876,8 +888,7 @@ public class ForwardedRequestCustomizer implements Customizer
if (value.startsWith("_") || "unknown".equals(value)) if (value.startsWith("_") || "unknown".equals(value))
break; break;
HostPort hostField = new HostPort(value); HostPort hostField = new HostPort(value);
getFor().setHost(hostField.getHost(), Priority.FORWARDED); getFor().setHostPort(hostField.getHost(), hostField.getPort(), Source.FORWARDED);
getFor().setPort(hostField.getPort(), Priority.FORWARDED);
break; break;
} }
case "host": case "host":
@ -885,41 +896,40 @@ public class ForwardedRequestCustomizer implements Customizer
if (value.startsWith("_") || "unknown".equals(value)) if (value.startsWith("_") || "unknown".equals(value))
break; break;
HostPort hostField = new HostPort(value); HostPort hostField = new HostPort(value);
getAuthority().setHost(hostField.getHost(), Priority.FORWARDED); getAuthority().setHostPort(hostField.getHost(), hostField.getPort(), Source.FORWARDED);
getAuthority().setPort(hostField.getPort(), Priority.FORWARDED);
break; break;
} }
case "proto": case "proto":
updateProto(value, Priority.FORWARDED); updateProto(value, Source.FORWARDED);
break; break;
} }
} }
} }
private void updateAuthority(String value, Priority priority) private void updateAuthority(String value, Source source)
{ {
HostPort hostField = new HostPort(value); HostPort hostField = new HostPort(value);
getAuthority().setHostPort(hostField, priority); getAuthority().setHostPort(hostField, source);
} }
private void updatePort(int port, Priority priority) private void updatePort(int port, Source source)
{ {
if (getForwardedPortAsAuthority()) if (getForwardedPortAsAuthority())
{ {
getAuthority().setPort(port, priority); getAuthority().setPort(port, source);
} }
else else
{ {
getFor().setPort(port, priority); getFor().setPort(port, source);
} }
} }
private void updateProto(String proto, Priority priority) private void updateProto(String proto, Source source)
{ {
if (priority.ordinal() > _protoPriority.ordinal()) if (source.priority() > _protoSource.priority())
{ {
_proto = proto; _proto = proto;
_protoPriority = priority; _protoSource = source;
if (_proto.equalsIgnoreCase(_config.getSecureScheme())) if (_proto.equalsIgnoreCase(_config.getSecureScheme()))
{ {

View File

@ -755,7 +755,7 @@ public class ForwardedRequestCustomizerTest
request.configure(customizer); request.configure(customizer);
String rawRequest = request.getRawRequest((header) -> header); String rawRequest = request.getRawRequest((header) -> header);
System.out.println(rawRequest); // System.out.println(rawRequest);
HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(rawRequest)); HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(rawRequest));
assertThat("status", response.getStatus(), is(200)); assertThat("status", response.getStatus(), is(200));
@ -775,7 +775,7 @@ public class ForwardedRequestCustomizerTest
.replaceFirst("X-Proxied-Https:", "Jetty-Proxied-Https:") .replaceFirst("X-Proxied-Https:", "Jetty-Proxied-Https:")
.replaceFirst("Proxy-Ssl-Id:", "Jetty-Proxy-Ssl-Id:") .replaceFirst("Proxy-Ssl-Id:", "Jetty-Proxy-Ssl-Id:")
.replaceFirst("Proxy-auth-cert:", "Jetty-Proxy-Auth-Cert:")); .replaceFirst("Proxy-auth-cert:", "Jetty-Proxy-Auth-Cert:"));
System.out.println(rawRequest); // System.out.println(rawRequest);
HttpTester.Response response = HttpTester.parseResponse(connectorConfigured.getResponse(rawRequest)); HttpTester.Response response = HttpTester.parseResponse(connectorConfigured.getResponse(rawRequest));
assertThat("status", response.getStatus(), is(200)); assertThat("status", response.getStatus(), is(200));
@ -796,7 +796,7 @@ public class ForwardedRequestCustomizerTest
request.configure(customizer); request.configure(customizer);
String rawRequest = request.getRawRequest((header) -> header); String rawRequest = request.getRawRequest((header) -> header);
System.out.println(rawRequest); // System.out.println(rawRequest);
HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(rawRequest)); HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(rawRequest));
assertThat("status", response.getStatus(), is(400)); assertThat("status", response.getStatus(), is(400));