Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #7250 - Correct HostHeaderCustomizer logic and add new RejectMissingAuthorityCustomizer #7292

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions jetty-server/src/main/config/etc/jetty-host-header-customizer.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?xml version="1.0"?><!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "http://www.eclipse.org/jetty/configure_9_3.dtd">
<Configure id="httpConfig" class="org.eclipse.jetty.server.HttpConfiguration">
<Call name="addCustomizer">
<Arg>
<New class="org.eclipse.jetty.server.HostHeaderCustomizer">
<Arg>
<Property name="jetty.hostheader.serverName" />
</Arg>
<Arg>
<Property name="jetty.hostheader.serverPort" />
</Arg>
</New>
</Arg>
</Call>
</Configure>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0"?><!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "http://www.eclipse.org/jetty/configure_9_3.dtd">
<Configure id="httpConfig" class="org.eclipse.jetty.server.HttpConfiguration">
<Call name="addCustomizer">
<Arg>
<New class="org.eclipse.jetty.server.RejectMissingAuthorityCustomizer" />
</Arg>
</Call>
</Configure>
23 changes: 23 additions & 0 deletions jetty-server/src/main/config/modules/host-header-customizer.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html

[description]
Adds a Host Header Customizer to the HTTP Connector to enforce a
Request Host header on HTTP/1.0 and HTTP/2 requests.

[tags]
connector

[depend]
http

[xml]
etc/jetty-host-header-customizer.xml

[ini-template]
### HostHeaderCustomizer Configuration

## The Server Name to force on null Host headers
# jetty.hostheader.serverName=serverName

## The Server Port to force on null Host headers
# jetty.hostheader.serverPort=80
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html

[description]
Reject Missing HTTP Authority Customizer.
If an HTTP request arrives without a valid Authority it is rejected with a 400 Bad Request.
This means empty Host headers, missing Host headers, or authority not being set from other
means such as the ForwardedRequestCustomizer from Forwarding headers.

[tags]
connector

[depend]
http

[xml]
etc/jetty-reject-missing-authority-customizer.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
import java.util.Objects;
import javax.servlet.http.HttpServletRequest;

import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpVersion;

/**
* Customizes requests that lack the {@code Host} header (for example, HTTP 1.0 requests).
* <p>
Expand All @@ -38,15 +41,14 @@
*/
public class HostHeaderCustomizer implements HttpConfiguration.Customizer
{
private final String serverName;
private final int serverPort;
private final String hostValue;

/**
* @param serverName the {@code serverName} to set on the request (the {@code serverPort} will not be set)
*/
public HostHeaderCustomizer(String serverName)
{
this(serverName, 0);
this(serverName, -1);
}

/**
Expand All @@ -55,14 +57,29 @@ public HostHeaderCustomizer(String serverName)
*/
public HostHeaderCustomizer(String serverName, int serverPort)
{
this.serverName = Objects.requireNonNull(serverName);
this.serverPort = serverPort;
String host = Objects.requireNonNull(serverName);
if (serverPort > 0)
host += ":" + serverPort;
hostValue = host;
}

@Override
public void customize(Connector connector, HttpConfiguration channelConfig, Request request)
{
if (request.getHeader("Host") == null)
request.setAuthority(serverName, serverPort); // TODO set the field as well?
String hostHeaderValue = request.getHeader("Host");

// No Host Header
if (request.getHttpVersion().getVersion() != HttpVersion.HTTP_1_1.getVersion() &&
hostHeaderValue == null)
{
if (request.getHttpURI().isAbsolute())
{
request.getHttpFields().put(HttpHeader.HOST, request.getHttpURI().getAuthority());
}
else
{
request.getHttpFields().put(HttpHeader.HOST, hostValue);
}
}
Comment on lines +69 to +83
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since getHeader is a linear lookup, it is best to avoid doing it if possible, and then to use efficient non string lookup:

Suggested change
String hostHeaderValue = request.getHeader("Host");
// No Host Header
if (request.getHttpVersion().getVersion() != HttpVersion.HTTP_1_1.getVersion() &&
hostHeaderValue == null)
{
if (request.getHttpURI().isAbsolute())
{
request.getHttpFields().put(HttpHeader.HOST, request.getHttpURI().getAuthority());
}
else
{
request.getHttpFields().put(HttpHeader.HOST, hostValue);
}
}
// If not 1.1 and no Host Header
if (request.getHttpVersion().getVersion() != HttpVersion.HTTP_1_1.getVersion() &&
!request.getHttpFields().contains(HttpHeader.HOST))
{
if (request.getHttpURI().isAbsolute())
{
request.getHttpFields().put(HttpHeader.HOST, request.getHttpURI().getAuthority());
}
else
{
request.getHttpFields().put(HttpHeader.HOST, hostValue);
}
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
//
// ========================================================================
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//

package org.eclipse.jetty.server;

import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HostPortHttpField;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.util.StringUtil;

/**
* Reject requests that have no request level authority.
*
* <p>
* This addresses requests that either have schemes with no authority
* per https://datatracker.ietf.org/doc/html/rfc7230#section-5.4
* Or badly declared requests that have a non-absolute URI in the request line and an empty or missing `Host` header.
* </p>
*
* <p>
* This will return a 400 Bad Request on failed authority checks.
* </p>
*/
public class RejectMissingAuthorityCustomizer implements HttpConfiguration.Customizer
{
@Override
public void customize(Connector connector, HttpConfiguration channelConfig, Request request)
{
if (!hasRequestAuthority(request))
{
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Missing authority");
}
}

private boolean hasRequestAuthority(Request request)
{
MetaData.Request metadata = request.getMetaData();

if (metadata != null)
{
// Investigate HttpURI (eg: from HTTP/2 :authority)?
if (StringUtil.isNotBlank(metadata.getURI().getHost()))
return true;

// Investigate Host Header (eg: from HTTP/1 request fields)
HttpField host = metadata.getFields().getField(HttpHeader.HOST);
if (host != null)
{
if (!(host instanceof HostPortHttpField) && StringUtil.isNotBlank(host.getValue()))
{
return true;
}

if (host instanceof HostPortHttpField)
{
HostPortHttpField authority = (HostPortHttpField)host;
metadata.getURI().setAuthority(authority.getHost(), authority.getPort());
if (StringUtil.isNotBlank(authority.getHost()))
return true;
}
}
}
Comment on lines +65 to +80
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this way reduces the number of tests in the hot path:

Suggested change
if (host != null)
{
if (!(host instanceof HostPortHttpField) && StringUtil.isNotBlank(host.getValue()))
{
return true;
}
if (host instanceof HostPortHttpField)
{
HostPortHttpField authority = (HostPortHttpField)host;
metadata.getURI().setAuthority(authority.getHost(), authority.getPort());
if (StringUtil.isNotBlank(authority.getHost()))
return true;
}
}
}
if (host != null)
if (host instanceof HostPortHttpField)
{
HostPortHttpField authority = (HostPortHttpField)host;
metadata.getURI().setAuthority(authority.getHost(), authority.getPort());
if (StringUtil.isNotBlank(authority.getHost()))
return true;
}
else if (host != null && StringUtil.isNotBlank(host.getValue()))
{
return true;
}
}


return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1418,7 +1418,7 @@ private String findServerName()
HttpField host = metadata == null ? null : metadata.getFields().getField(HttpHeader.HOST);
if (host != null)
{
if (!(host instanceof HostPortHttpField) && host.getValue() != null && !host.getValue().isEmpty())
if (!(host instanceof HostPortHttpField) && StringUtil.isNotBlank(host.getValue()))
host = new HostPortHttpField(host.getValue());
if (host instanceof HostPortHttpField)
{
Expand Down Expand Up @@ -1472,7 +1472,7 @@ private int findServerPort()
MetaData.Request metadata = _metaData;
// Return host from header field
HttpField host = metadata == null ? null : metadata.getFields().getField(HttpHeader.HOST);
if (host != null)
if ((host != null) && StringUtil.isNotBlank(host.getValue()))
{
// TODO is this needed now?
HostPortHttpField authority = (host instanceof HostPortHttpField)
Expand Down
Loading