Skip to content

Commit

Permalink
Improve Attributes Handling (#4816)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>

* 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 <[email protected]>

* Allow a set to override a secure attribute.

Signed-off-by: Greg Wilkins <[email protected]>

* 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 <[email protected]>

Co-authored-by: Joakim Erdfelt <[email protected]>
  • Loading branch information
@gregw @joakime
gregw and joakime authored Apr 28, 2020
1 parent 8929791 commit 8fcbf6d
Show file tree
Hide file tree
Showing 10 changed files with 250 additions and 160 deletions.
172 changes: 95 additions & 77 deletions jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -252,10 +251,8 @@ public String toString()
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;
Expand All @@ -264,40 +261,43 @@ private class ForwardAttributes implements Attributes

ForwardAttributes(Attributes attributes)
{
_attr = attributes;
super(attributes);
}

@Override
public Object getAttribute(String key)
{
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);
Expand All @@ -318,40 +318,48 @@ public Enumeration<String> getAttributeNames()
set.remove(FORWARD_QUERY_STRING);
}

return Collections.enumeration(set);
return set;
}

@Override
public void setAttribute(String key, Object value)
{
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
Expand All @@ -367,10 +375,8 @@ public void removeAttribute(String name)
}
}

private class IncludeAttributes implements Attributes
private class IncludeAttributes extends Attributes.Wrapper
{
final Attributes _attr;

String _requestURI;
String _contextPath;
String _servletPath;
Expand All @@ -379,39 +385,42 @@ private class IncludeAttributes implements Attributes

IncludeAttributes(Attributes attributes)
{
_attr = attributes;
super(attributes);
}

@Override
public Object getAttribute(String key)
{
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);
}
Expand All @@ -431,39 +440,48 @@ public Enumeration<String> getAttributeNames()
set.remove(INCLUDE_QUERY_STRING);
}

return Collections.enumeration(set);
return set;
}

@Override
public void setAttribute(String key, Object value)
{
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
Expand Down
8 changes: 7 additions & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/Request.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1867,8 +1867,14 @@ protected void recycle()
_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;
Expand Down
Loading

0 comments on commit 8fcbf6d

Please sign in to comment.