Improve Attributes Handling (#4816)

* Spun out from #4814 Improve Attributes Handling

Improve attribute handling to reduce garbage and improve lookup.
Introduced a Wrapper so that request can remove any layers on reset.

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4814 - Exposing AttributeMap.getAttributeNameSet() on Attributes.

The underlying AttributesMap already has a .getAttributeNameSet()
method, expose it on the Attributes interface.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

* Allow a set to override a secure attribute.

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4814 - Attributes.getAttributeNames() is now defaulted

The Attributes.getAttributeNames() will use the
.getAttributeNameSet() by default now.

Updated all Attributes.Wrapper impls to use this new behavior

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

Co-authored-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
This commit is contained in:
Greg Wilkins 2020-04-28 10:16:29 +02:00 committed by GitHub
parent 8929791e6c
commit 8fcbf6d590
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 251 additions and 161 deletions

View File

@ -19,9 +19,8 @@
package org.eclipse.jetty.server;
import java.io.IOException;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Set;
import javax.servlet.DispatcherType;
import javax.servlet.RequestDispatcher;
import javax.servlet.ServletException;
@ -252,10 +251,8 @@ public class Dispatcher implements RequestDispatcher
return String.format("Dispatcher@0x%x{%s,%s}", hashCode(), _named, _uri);
}
private class ForwardAttributes implements Attributes
private class ForwardAttributes extends Attributes.Wrapper
{
final Attributes _attr;
String _requestURI;
String _contextPath;
String _servletPath;
@ -264,7 +261,7 @@ public class Dispatcher implements RequestDispatcher
ForwardAttributes(Attributes attributes)
{
_attr = attributes;
super(attributes);
}
@Override
@ -272,32 +269,35 @@ public class Dispatcher implements RequestDispatcher
{
if (Dispatcher.this._named == null)
{
if (key.equals(FORWARD_PATH_INFO))
return _pathInfo;
if (key.equals(FORWARD_REQUEST_URI))
return _requestURI;
if (key.equals(FORWARD_SERVLET_PATH))
return _servletPath;
if (key.equals(FORWARD_CONTEXT_PATH))
return _contextPath;
if (key.equals(FORWARD_QUERY_STRING))
return _query;
switch (key)
{
case FORWARD_PATH_INFO:
return _pathInfo;
case FORWARD_REQUEST_URI:
return _requestURI;
case FORWARD_SERVLET_PATH:
return _servletPath;
case FORWARD_CONTEXT_PATH:
return _contextPath;
case FORWARD_QUERY_STRING:
return _query;
default:
break;
}
}
if (key.startsWith(__INCLUDE_PREFIX))
return null;
return _attr.getAttribute(key);
return _attributes.getAttribute(key);
}
@Override
public Enumeration<String> getAttributeNames()
public Set<String> getAttributeNameSet()
{
HashSet<String> set = new HashSet<>();
Enumeration<String> e = _attr.getAttributeNames();
while (e.hasMoreElements())
for (String name : _attributes.getAttributeNameSet())
{
String name = e.nextElement();
if (!name.startsWith(__INCLUDE_PREFIX) &&
!name.startsWith(__FORWARD_PREFIX))
set.add(name);
@ -318,7 +318,7 @@ public class Dispatcher implements RequestDispatcher
set.remove(FORWARD_QUERY_STRING);
}
return Collections.enumeration(set);
return set;
}
@Override
@ -326,32 +326,40 @@ public class Dispatcher implements RequestDispatcher
{
if (_named == null && key.startsWith("javax.servlet."))
{
if (key.equals(FORWARD_PATH_INFO))
_pathInfo = (String)value;
else if (key.equals(FORWARD_REQUEST_URI))
_requestURI = (String)value;
else if (key.equals(FORWARD_SERVLET_PATH))
_servletPath = (String)value;
else if (key.equals(FORWARD_CONTEXT_PATH))
_contextPath = (String)value;
else if (key.equals(FORWARD_QUERY_STRING))
_query = (String)value;
else if (value == null)
_attr.removeAttribute(key);
else
_attr.setAttribute(key, value);
switch (key)
{
case FORWARD_PATH_INFO:
_pathInfo = (String)value;
return;
case FORWARD_REQUEST_URI:
_requestURI = (String)value;
return;
case FORWARD_SERVLET_PATH:
_servletPath = (String)value;
return;
case FORWARD_CONTEXT_PATH:
_contextPath = (String)value;
return;
case FORWARD_QUERY_STRING:
_query = (String)value;
return;
default:
if (value == null)
_attributes.removeAttribute(key);
else
_attributes.setAttribute(key, value);
}
}
else if (value == null)
_attr.removeAttribute(key);
_attributes.removeAttribute(key);
else
_attr.setAttribute(key, value);
_attributes.setAttribute(key, value);
}
@Override
public String toString()
{
return "FORWARD+" + _attr.toString();
return "FORWARD+" + _attributes.toString();
}
@Override
@ -367,10 +375,8 @@ public class Dispatcher implements RequestDispatcher
}
}
private class IncludeAttributes implements Attributes
private class IncludeAttributes extends Attributes.Wrapper
{
final Attributes _attr;
String _requestURI;
String _contextPath;
String _servletPath;
@ -379,7 +385,7 @@ public class Dispatcher implements RequestDispatcher
IncludeAttributes(Attributes attributes)
{
_attr = attributes;
super(attributes);
}
@Override
@ -387,31 +393,34 @@ public class Dispatcher implements RequestDispatcher
{
if (Dispatcher.this._named == null)
{
if (key.equals(INCLUDE_PATH_INFO))
return _pathInfo;
if (key.equals(INCLUDE_SERVLET_PATH))
return _servletPath;
if (key.equals(INCLUDE_CONTEXT_PATH))
return _contextPath;
if (key.equals(INCLUDE_QUERY_STRING))
return _query;
if (key.equals(INCLUDE_REQUEST_URI))
return _requestURI;
switch (key)
{
case INCLUDE_PATH_INFO:
return _pathInfo;
case INCLUDE_SERVLET_PATH:
return _servletPath;
case INCLUDE_CONTEXT_PATH:
return _contextPath;
case INCLUDE_QUERY_STRING:
return _query;
case INCLUDE_REQUEST_URI:
return _requestURI;
default:
break;
}
}
else if (key.startsWith(__INCLUDE_PREFIX))
return null;
return _attr.getAttribute(key);
return _attributes.getAttribute(key);
}
@Override
public Enumeration<String> getAttributeNames()
public Set<String> getAttributeNameSet()
{
HashSet<String> set = new HashSet<>();
Enumeration<String> e = _attr.getAttributeNames();
while (e.hasMoreElements())
for (String name : _attributes.getAttributeNameSet())
{
String name = e.nextElement();
if (!name.startsWith(__INCLUDE_PREFIX))
set.add(name);
}
@ -431,7 +440,7 @@ public class Dispatcher implements RequestDispatcher
set.remove(INCLUDE_QUERY_STRING);
}
return Collections.enumeration(set);
return set;
}
@Override
@ -439,31 +448,40 @@ public class Dispatcher implements RequestDispatcher
{
if (_named == null && key.startsWith("javax.servlet."))
{
if (key.equals(INCLUDE_PATH_INFO))
_pathInfo = (String)value;
else if (key.equals(INCLUDE_REQUEST_URI))
_requestURI = (String)value;
else if (key.equals(INCLUDE_SERVLET_PATH))
_servletPath = (String)value;
else if (key.equals(INCLUDE_CONTEXT_PATH))
_contextPath = (String)value;
else if (key.equals(INCLUDE_QUERY_STRING))
_query = (String)value;
else if (value == null)
_attr.removeAttribute(key);
else
_attr.setAttribute(key, value);
switch (key)
{
case INCLUDE_PATH_INFO:
_pathInfo = (String)value;
return;
case INCLUDE_REQUEST_URI:
_requestURI = (String)value;
return;
case INCLUDE_SERVLET_PATH:
_servletPath = (String)value;
return;
case INCLUDE_CONTEXT_PATH:
_contextPath = (String)value;
return;
case INCLUDE_QUERY_STRING:
_query = (String)value;
return;
default:
if (value == null)
_attributes.removeAttribute(key);
else
_attributes.setAttribute(key, value);
}
}
else if (value == null)
_attr.removeAttribute(key);
_attributes.removeAttribute(key);
else
_attr.setAttribute(key, value);
_attributes.setAttribute(key, value);
}
@Override
public String toString()
{
return "INCLUDE+" + _attr.toString();
return "INCLUDE+" + _attributes.toString();
}
@Override

View File

@ -1867,8 +1867,14 @@ public class Request implements HttpServletRequest
_async = null;
_asyncNotSupportedSource = null;
_handled = false;
_attributes = Attributes.unwrap(_attributes);
if (_attributes != null)
_attributes.clearAttributes();
{
if (AttributesMap.class.equals(_attributes.getClass()))
_attributes.clearAttributes();
else
_attributes = null;
}
_contentType = null;
_characterEncoding = null;
_contextPath = null;

View File

@ -19,6 +19,8 @@
package org.eclipse.jetty.server;
import java.security.cert.X509Certificate;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
@ -33,6 +35,8 @@ import org.eclipse.jetty.http.PreEncodedHttpField;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.io.ssl.SslConnection;
import org.eclipse.jetty.io.ssl.SslConnection.DecryptedEndPoint;
import org.eclipse.jetty.util.Attributes;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.TypeUtil;
import org.eclipse.jetty.util.annotation.Name;
import org.eclipse.jetty.util.log.Log;
@ -49,11 +53,10 @@ import org.eclipse.jetty.util.ssl.X509;
public class SecureRequestCustomizer implements HttpConfiguration.Customizer
{
private static final Logger LOG = Log.getLogger(SecureRequestCustomizer.class);
/**
* The name of the SSLSession attribute that will contain any cached information.
*/
public static final String CACHED_INFO_ATTR = CachedInfo.class.getName();
public static final String JAVAX_SERVLET_REQUEST_X_509_CERTIFICATE = "javax.servlet.request.X509Certificate";
public static final String JAVAX_SERVLET_REQUEST_CIPHER_SUITE = "javax.servlet.request.cipher_suite";
public static final String JAVAX_SERVLET_REQUEST_KEY_SIZE = "javax.servlet.request.key_size";
public static final String JAVAX_SERVLET_REQUEST_SSL_SESSION_ID = "javax.servlet.request.ssl_session_id";
private String sslSessionAttribute = "org.eclipse.jetty.servlet.request.ssl_session";
@ -262,61 +265,22 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
if (_sniHostCheck || _sniRequired)
{
String name = request.getServerName();
X509 x509 = (X509)sslSession.getValue(SniX509ExtendedKeyManager.SNI_X509);
if (LOG.isDebugEnabled())
LOG.debug("Host {} with SNI {}", name, x509);
LOG.debug("Host {} with SNI {}", request.getServerName(), x509);
if (x509 == null)
{
if (_sniRequired)
throw new BadMessageException(400, "SNI required");
}
else if (_sniHostCheck && !x509.matches(name))
else if (_sniHostCheck && !x509.matches(request.getServerName()))
{
throw new BadMessageException(400, "Host does not match SNI");
}
}
try
{
String cipherSuite = sslSession.getCipherSuite();
Integer keySize;
X509Certificate[] certs;
String idStr;
CachedInfo cachedInfo = (CachedInfo)sslSession.getValue(CACHED_INFO_ATTR);
if (cachedInfo != null)
{
keySize = cachedInfo.getKeySize();
certs = cachedInfo.getCerts();
idStr = cachedInfo.getIdStr();
}
else
{
keySize = SslContextFactory.deduceKeyLength(cipherSuite);
certs = getCertChain(request, sslSession);
byte[] bytes = sslSession.getId();
idStr = TypeUtil.toHexString(bytes);
cachedInfo = new CachedInfo(keySize, certs, idStr);
sslSession.putValue(CACHED_INFO_ATTR, cachedInfo);
}
if (certs != null)
request.setAttribute("javax.servlet.request.X509Certificate", certs);
request.setAttribute("javax.servlet.request.cipher_suite", cipherSuite);
request.setAttribute("javax.servlet.request.key_size", keySize);
request.setAttribute("javax.servlet.request.ssl_session_id", idStr);
String sessionAttribute = getSslSessionAttribute();
if (sessionAttribute != null && !sessionAttribute.isEmpty())
request.setAttribute(sessionAttribute, sslSession);
}
catch (Exception e)
{
LOG.warn(Log.EXCEPTION, e);
}
request.setAttributes(new SslAttributes(request, sslSession, request.getAttributes()));
}
private X509Certificate[] getCertChain(Request request, SSLSession sslSession)
@ -327,10 +291,8 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
if (sslConnectionFactory != null)
{
SslContextFactory sslContextFactory = sslConnectionFactory.getSslContextFactory();
if (sslConnectionFactory != null)
{
if (sslContextFactory != null)
return sslContextFactory.getX509CertChain(sslSession);
}
}
// Fallback, either no SslConnectionFactory or no SslContextFactory instance found
@ -353,36 +315,66 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
return String.format("%s@%x", this.getClass().getSimpleName(), hashCode());
}
/**
* Simple bundle of information that is cached in the SSLSession. Stores the
* effective keySize and the client certificate chain.
*/
private static class CachedInfo
private class SslAttributes extends Attributes.Wrapper
{
private final X509Certificate[] _certs;
private final Integer _keySize;
private final String _idStr;
private final Request _request;
private final SSLSession _session;
CachedInfo(Integer keySize, X509Certificate[] certs, String idStr)
public SslAttributes(Request request, SSLSession sslSession, Attributes attributes)
{
this._keySize = keySize;
this._certs = certs;
this._idStr = idStr;
super(attributes);
this._request = request;
this._session = sslSession;
}
X509Certificate[] getCerts()
@Override
public Object getAttribute(String name)
{
return _certs;
Object value = _attributes.getAttribute(name);
if (value != null)
return value;
try
{
switch (name)
{
case JAVAX_SERVLET_REQUEST_X_509_CERTIFICATE:
return SecureRequestCustomizer.this.getCertChain(_request, _session);
case JAVAX_SERVLET_REQUEST_CIPHER_SUITE:
return _session.getCipherSuite();
case JAVAX_SERVLET_REQUEST_KEY_SIZE:
return SslContextFactory.deduceKeyLength(_session.getCipherSuite());
case JAVAX_SERVLET_REQUEST_SSL_SESSION_ID:
return TypeUtil.toHexString(_session.getId());
default:
String sessionAttribute = getSslSessionAttribute();
if (!StringUtil.isEmpty(sessionAttribute) && sessionAttribute.equals(name))
return _session;
}
}
catch (Exception e)
{
if (LOG.isDebugEnabled())
LOG.debug("Unable to get secure details ", e);
}
return null;
}
Integer getKeySize()
@Override
public Set<String> getAttributeNameSet()
{
return _keySize;
}
String getIdStr()
{
return _idStr;
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);
return names;
}
}
}

View File

@ -26,6 +26,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Enumeration;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.Future;
import javax.servlet.ServletContext;
@ -594,6 +595,12 @@ public class Server extends HandlerWrapper implements Attributes
return _attributes.getAttributeNames();
}
@Override
public Set<String> getAttributeNameSet()
{
return _attributes.getAttributeNameSet();
}
/*
* @see org.eclipse.util.AttributesMap#removeAttribute(java.lang.String)
*/

View File

@ -516,6 +516,12 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
return AttributesMap.getAttributeNamesCopy(_attributes);
}
@Override
public Set<String> getAttributeNameSet()
{
return _attributes.getAttributeNameSet();
}
/**
* @return Returns the attributes.
*/

View File

@ -18,7 +18,6 @@
package org.eclipse.jetty.server.handler;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Set;
import javax.servlet.ServletContextAttributeEvent;
@ -77,10 +76,8 @@ public class ManagedAttributeListener implements ServletContextListener, Servlet
public void contextInitialized(ServletContextEvent event)
{
// Update existing attributes
Enumeration<String> e = event.getServletContext().getAttributeNames();
while (e.hasMoreElements())
for (String name : _context.getServletContext().getAttributeNameSet())
{
String name = e.nextElement();
if (_managedAttributes.contains(name))
updateBean(name, null, event.getServletContext().getAttribute(name));
}
@ -89,10 +86,8 @@ public class ManagedAttributeListener implements ServletContextListener, Servlet
@Override
public void contextDestroyed(ServletContextEvent event)
{
Enumeration<String> e = _context.getServletContext().getAttributeNames();
while (e.hasMoreElements())
for (String name : _context.getServletContext().getAttributeNameSet())
{
String name = e.nextElement();
if (_managedAttributes.contains(name))
updateBean(name, event.getServletContext().getAttribute(name), null);
}

View File

@ -18,7 +18,6 @@
package org.eclipse.jetty.server.handler.jmx;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Map;
@ -42,10 +41,8 @@ public class ContextHandlerMBean extends AbstractHandlerMBean
{
Map<String, Object> map = new HashMap<String, Object>();
Attributes attrs = ((ContextHandler)_managed).getAttributes();
Enumeration<String> en = attrs.getAttributeNames();
while (en.hasMoreElements())
for (String name : attrs.getAttributeNameSet())
{
String name = en.nextElement();
Object value = attrs.getAttribute(name);
map.put(name, value);
}

View File

@ -18,7 +18,9 @@
package org.eclipse.jetty.util;
import java.util.Collections;
import java.util.Enumeration;
import java.util.Set;
/**
* Attributes.
@ -32,7 +34,66 @@ public interface Attributes
Object getAttribute(String name);
Enumeration<String> getAttributeNames();
Set<String> getAttributeNameSet();
default Enumeration<String> getAttributeNames()
{
return Collections.enumeration(getAttributeNameSet());
}
void clearAttributes();
static Attributes unwrap(Attributes attributes)
{
while (attributes instanceof Wrapper)
{
attributes = ((Wrapper)attributes).getAttributes();
}
return attributes;
}
abstract class Wrapper implements Attributes
{
protected final Attributes _attributes;
public Wrapper(Attributes attributes)
{
_attributes = attributes;
}
public Attributes getAttributes()
{
return _attributes;
}
@Override
public void removeAttribute(String name)
{
_attributes.removeAttribute(name);
}
@Override
public void setAttribute(String name, Object attribute)
{
_attributes.setAttribute(name, attribute);
}
@Override
public Object getAttribute(String name)
{
return _attributes.getAttribute(name);
}
@Override
public Set<String> getAttributeNameSet()
{
return _attributes.getAttributeNameSet();
}
@Override
public void clearAttributes()
{
_attributes.clearAttributes();
}
}
}

View File

@ -94,6 +94,7 @@ public class AttributesMap implements Attributes, Dumpable
return Collections.enumeration(getAttributeNameSet());
}
@Override
public Set<String> getAttributeNameSet()
{
return keySet();

View File

@ -23,6 +23,7 @@ import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import org.eclipse.jetty.util.Attributes;
@ -60,6 +61,12 @@ public class AttributeContainerMap extends ContainerLifeCycle implements Attribu
return Collections.enumeration(_map.keySet());
}
@Override
public Set<String> getAttributeNameSet()
{
return _map.keySet();
}
@Override
public synchronized void clearAttributes()
{