Skip to content

Commit

Permalink
Merge pull request #5004 from eclipse/jetty-9.4.x-4985-AttributeNameSet
Browse files Browse the repository at this point in the history
Issue #4985 - fix NPE related to use of Attributes.Wrapper getAttributeNameSet()
  • Loading branch information
lachlan-roberts authored Jul 6, 2020
2 parents ae43b70 + f35d6cf commit 9a56301
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

class AsyncAttributes extends Attributes.Wrapper
{
public static final String __ASYNC_PREFIX = "javax.servlet.async.";

private String _requestURI;
private String _contextPath;
private String _servletPath;
Expand Down Expand Up @@ -67,7 +69,11 @@ public Object getAttribute(String key)
@Override
public Set<String> getAttributeNameSet()
{
Set<String> set = new HashSet<>(super.getAttributeNameSet());
Set<String> set = new HashSet<>();
super.getAttributeNameSet().stream()
.filter(name -> !name.startsWith(__ASYNC_PREFIX))
.forEach(set::add);

if (_requestURI != null)
set.add(AsyncContext.ASYNC_REQUEST_URI);
if (_contextPath != null)
Expand Down
65 changes: 30 additions & 35 deletions jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,11 @@ public String toString()

private class ForwardAttributes extends Attributes.Wrapper
{
String _requestURI;
String _contextPath;
String _servletPath;
String _pathInfo;
String _query;
private String _requestURI;
private String _contextPath;
private String _servletPath;
private String _pathInfo;
private String _query;

ForwardAttributes(Attributes attributes)
{
Expand Down Expand Up @@ -286,6 +286,7 @@ public Object getAttribute(String key)
}
}

// TODO: should this be __FORWARD_PREFIX?
if (key.startsWith(__INCLUDE_PREFIX))
return null;

Expand All @@ -296,26 +297,23 @@ public Object getAttribute(String key)
public Set<String> getAttributeNameSet()
{
HashSet<String> set = new HashSet<>();
for (String name : _attributes.getAttributeNameSet())
{
if (!name.startsWith(__INCLUDE_PREFIX) &&
!name.startsWith(__FORWARD_PREFIX))
set.add(name);
}
super.getAttributeNameSet().stream()
.filter(name -> !name.startsWith(__INCLUDE_PREFIX))
.filter(name -> !name.startsWith(__FORWARD_PREFIX))
.forEach(set::add);

if (_named == null)
{
if (_pathInfo != null)
set.add(FORWARD_PATH_INFO);
else
set.remove(FORWARD_PATH_INFO);
set.add(FORWARD_REQUEST_URI);
set.add(FORWARD_SERVLET_PATH);
set.add(FORWARD_CONTEXT_PATH);
if (_requestURI != null)
set.add(FORWARD_REQUEST_URI);
if (_servletPath != null)
set.add(FORWARD_SERVLET_PATH);
if (_contextPath != null)
set.add(FORWARD_CONTEXT_PATH);
if (_query != null)
set.add(FORWARD_QUERY_STRING);
else
set.remove(FORWARD_QUERY_STRING);
}

return set;
Expand Down Expand Up @@ -378,11 +376,11 @@ public void removeAttribute(String name)

private class IncludeAttributes extends Attributes.Wrapper
{
String _requestURI;
String _contextPath;
String _servletPath;
String _pathInfo;
String _query;
private String _requestURI;
private String _contextPath;
private String _servletPath;
private String _pathInfo;
private String _query;

IncludeAttributes(Attributes attributes)
{
Expand Down Expand Up @@ -420,25 +418,22 @@ else if (key.startsWith(__INCLUDE_PREFIX))
public Set<String> getAttributeNameSet()
{
HashSet<String> set = new HashSet<>();
for (String name : _attributes.getAttributeNameSet())
{
if (!name.startsWith(__INCLUDE_PREFIX))
set.add(name);
}
super.getAttributeNameSet().stream()
.filter(name -> !name.startsWith(__INCLUDE_PREFIX))
.forEach(set::add);

if (_named == null)
{
if (_pathInfo != null)
set.add(INCLUDE_PATH_INFO);
else
set.remove(INCLUDE_PATH_INFO);
set.add(INCLUDE_REQUEST_URI);
set.add(INCLUDE_SERVLET_PATH);
set.add(INCLUDE_CONTEXT_PATH);
if (_requestURI != null)
set.add(INCLUDE_REQUEST_URI);
if (_servletPath != null)
set.add(INCLUDE_SERVLET_PATH);
if (_contextPath != null)
set.add(INCLUDE_CONTEXT_PATH);
if (_query != null)
set.add(INCLUDE_QUERY_STRING);
else
set.remove(INCLUDE_QUERY_STRING);
}

return set;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,18 @@ public void customize(Connector connector, HttpConfiguration channelConfig, Requ

private static class ProxyAttributes extends Attributes.Wrapper
{
private final InetSocketAddress remoteAddress;
private final InetSocketAddress localAddress;
private final String _remoteAddress;
private final String _localAddress;
private final int _remotePort;
private final int _localPort;

private ProxyAttributes(InetSocketAddress remoteAddress, InetSocketAddress localAddress, Attributes attributes)
{
super(attributes);
this.remoteAddress = remoteAddress;
this.localAddress = localAddress;
_remoteAddress = remoteAddress.getAddress().getHostAddress();
_localAddress = localAddress.getAddress().getHostAddress();
_remotePort = remoteAddress.getPort();
_localPort = localAddress.getPort();
}

@Override
Expand All @@ -81,13 +85,13 @@ public Object getAttribute(String name)
switch (name)
{
case REMOTE_ADDRESS_ATTRIBUTE_NAME:
return remoteAddress.getAddress().getHostAddress();
return _remoteAddress;
case REMOTE_PORT_ATTRIBUTE_NAME:
return remoteAddress.getPort();
return _remotePort;
case LOCAL_ADDRESS_ATTRIBUTE_NAME:
return localAddress.getAddress().getHostAddress();
return _localAddress;
case LOCAL_PORT_ATTRIBUTE_NAME:
return localAddress.getPort();
return _localPort;
default:
return super.getAttribute(name);
}
Expand All @@ -97,9 +101,14 @@ public Object getAttribute(String name)
public Set<String> getAttributeNameSet()
{
Set<String> names = new HashSet<>(_attributes.getAttributeNameSet());
names.add(REMOTE_ADDRESS_ATTRIBUTE_NAME);
names.remove(REMOTE_ADDRESS_ATTRIBUTE_NAME);
names.remove(LOCAL_ADDRESS_ATTRIBUTE_NAME);

if (_remoteAddress != null)
names.add(REMOTE_ADDRESS_ATTRIBUTE_NAME);
if (_localAddress != null)
names.add(LOCAL_ADDRESS_ATTRIBUTE_NAME);
names.add(REMOTE_PORT_ATTRIBUTE_NAME);
names.add(LOCAL_ADDRESS_ATTRIBUTE_NAME);
names.add(LOCAL_PORT_ATTRIBUTE_NAME);
return names;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,47 +319,51 @@ private class SslAttributes extends Attributes.Wrapper
private final Request _request;
private final SSLSession _session;

private X509Certificate[] _certs;
private String _cipherSuite;
private Integer _keySize;
private String _sessionId;
private String _sessionAttribute;

public SslAttributes(Request request, SSLSession sslSession, Attributes attributes)
{
super(attributes);
this._request = request;
this._session = sslSession;
}

@Override
public Object getAttribute(String name)
{
Object value = _attributes.getAttribute(name);
if (value != null)
return value;
try
{
switch (name)
{
case JAVAX_SERVLET_REQUEST_X_509_CERTIFICATE:
return getSslSessionData().getCerts();

case JAVAX_SERVLET_REQUEST_CIPHER_SUITE:
return _session.getCipherSuite();

case JAVAX_SERVLET_REQUEST_KEY_SIZE:
return getSslSessionData().getKeySize();

case JAVAX_SERVLET_REQUEST_SSL_SESSION_ID:
return getSslSessionData().getIdStr();

default:
String sessionAttribute = getSslSessionAttribute();
if (!StringUtil.isEmpty(sessionAttribute) && sessionAttribute.equals(name))
return _session;
}
_certs = getSslSessionData().getCerts();
_cipherSuite = _session.getCipherSuite();
_keySize = getSslSessionData().getKeySize();
_sessionId = getSslSessionData().getIdStr();
_sessionAttribute = getSslSessionAttribute();
}
catch (Exception e)
{
if (LOG.isDebugEnabled())
LOG.debug("Unable to get secure details ", e);
LOG.warn("Unable to get secure details ", e);
}
return null;
}

@Override
public Object getAttribute(String name)
{
switch (name)
{
case JAVAX_SERVLET_REQUEST_X_509_CERTIFICATE:
return _certs;
case JAVAX_SERVLET_REQUEST_CIPHER_SUITE:
return _cipherSuite;
case JAVAX_SERVLET_REQUEST_KEY_SIZE:
return _keySize;
case JAVAX_SERVLET_REQUEST_SSL_SESSION_ID:
return _sessionId;
default:
if (!StringUtil.isEmpty(_sessionAttribute) && _sessionAttribute.equals(name))
return _session;
}

return _attributes.getAttribute(name);
}

/**
Expand Down Expand Up @@ -391,13 +395,22 @@ private SslSessionData getSslSessionData()
public Set<String> getAttributeNameSet()
{
Set<String> names = new HashSet<>(_attributes.getAttributeNameSet());
names.add(JAVAX_SERVLET_REQUEST_X_509_CERTIFICATE);
names.add(JAVAX_SERVLET_REQUEST_CIPHER_SUITE);
names.add(JAVAX_SERVLET_REQUEST_KEY_SIZE);
names.add(JAVAX_SERVLET_REQUEST_SSL_SESSION_ID);
String sessionAttribute = getSslSessionAttribute();
if (!StringUtil.isEmpty(sessionAttribute))
names.add(sessionAttribute);
names.remove(JAVAX_SERVLET_REQUEST_X_509_CERTIFICATE);
names.remove(JAVAX_SERVLET_REQUEST_CIPHER_SUITE);
names.remove(JAVAX_SERVLET_REQUEST_KEY_SIZE);
names.remove(JAVAX_SERVLET_REQUEST_SSL_SESSION_ID);

if (_certs != null)
names.add(JAVAX_SERVLET_REQUEST_X_509_CERTIFICATE);
if (_cipherSuite != null)
names.add(JAVAX_SERVLET_REQUEST_CIPHER_SUITE);
if (_keySize != null)
names.add(JAVAX_SERVLET_REQUEST_KEY_SIZE);
if (_sessionId != null)
names.add(JAVAX_SERVLET_REQUEST_SSL_SESSION_ID);
if (!StringUtil.isEmpty(_sessionAttribute))
names.add(_sessionAttribute);

return names;
}
}
Expand Down

0 comments on commit 9a56301

Please sign in to comment.