Skip to content

Commit

Permalink
jetty-9 fixed context restart for constraint test
Browse files Browse the repository at this point in the history
  • Loading branch information
@gregw
gregw committed Aug 20, 2012
1 parent 4f30fb1 commit 1d89bef
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 80 deletions.
4 changes: 1 addition & 3 deletions jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,8 @@ protected void doStart() throws Exception
@Override
protected void doStop() throws Exception
{
_constraintMap.clear();
_constraintMappings.clear();
_roles.clear();
super.doStop();
_constraintMap.clear();
}

protected void processConstraintMapping(ConstraintMapping mapping)
Expand Down
19 changes: 11 additions & 8 deletions ...-security/src/main/java/org/eclipse/jetty/security/authentication/LoginAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,20 @@ protected HttpSession renewSession(HttpServletRequest request, HttpServletRespon
{
HttpSession httpSession = request.getSession(false);

synchronized (httpSession)
if (_renewSession && httpSession!=null)
{
//if we should renew sessions, and there is an existing session that may have been seen by non-authenticated users
//(indicated by SESSION_SECURED not being set on the session) then we should change id
if (_renewSession && httpSession!=null && httpSession.getAttribute(AbstractSessionManager.SESSION_KNOWN_ONLY_TO_AUTHENTICATED)!=Boolean.TRUE)
synchronized (httpSession)
{
HttpSession newSession = AbstractSessionManager.renewSession(request, httpSession,true);
LOG.debug("renew {}->{}",httpSession.getId(),newSession.getId());
httpSession=newSession;
//if we should renew sessions, and there is an existing session that may have been seen by non-authenticated users
//(indicated by SESSION_SECURED not being set on the session) then we should change id
if (httpSession.getAttribute(AbstractSessionManager.SESSION_KNOWN_ONLY_TO_AUTHENTICATED)!=Boolean.TRUE)
{
HttpSession newSession = AbstractSessionManager.renewSession(request, httpSession,true);
LOG.debug("renew {}->{}",httpSession.getId(),newSession.getId());
httpSession=newSession;
}
}
return httpSession;
}
return httpSession;
}
}
44 changes: 8 additions & 36 deletions jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,6 @@ public void testFormDispatch() throws Exception
@Test
public void testFormRedirect() throws Exception
{
Log.getLogger(SecurityHandler.class).setDebugEnabled(true);
Log.getLogger(LoginAuthenticator.class).setDebugEnabled(true);
Log.getLogger(FormAuthenticator.class).setDebugEnabled(true);

_security.setAuthenticator(new FormAuthenticator("/testLoginPage","/testErrorPage",false));
_security.setStrict(false);
_server.start();
Expand Down Expand Up @@ -783,9 +779,8 @@ public void testRoleRef() throws Exception
_security.setHandler(check);
_security.setAuthenticator(new BasicAuthenticator());
_security.setStrict(false);
_server.start();

System.out.println(_server.dump());
_server.start();

String response;
response = _connector.getResponses("GET /ctx/noauth/info HTTP/1.0\r\n\r\n", 100000, TimeUnit.MILLISECONDS);
Expand All @@ -798,40 +793,12 @@ public void testRoleRef() throws Exception

_server.stop();

/*
* FIXME: this seems to indicate there is an issue with the way the server is stopping and starting now
*
* Note that ConstraintSecurityHandler loses all of its brains when the server starts and stops, but that
* change was made in 2/2011 and this wasn't exposed til now, which seems to indicate that previously
* when the server stopped that doStop() didn't make it down to the constraint handler...and now it does.
*
* also, seems to be an issue in local connector, I had to add a new one for it to be able to work here as well
* so issues in stop/start there as well
*/

_connector = new LocalConnector(_server);
_server.setConnectors(new Connector[]{_connector});
ContextHandler _context = new ContextHandler();
SessionHandler _session = new SessionHandler();

_context.setContextPath("/ctx");
_server.setHandler(_context);
_context.setHandler(_session);

_security = new ConstraintSecurityHandler();
_session.setHandler(_security);
RequestHandler _handler = new RequestHandler();
_security.setHandler(_handler);

RoleRefHandler roleref = new RoleRefHandler();
roleref.setHandler(_security.getHandler());
_security.setHandler(roleref);
roleref.setHandler(check);

_security.setConstraintMappings(getConstraintMappings(),getKnownRoles());

_server.start();

System.out.println(_server.dump());
_server.start();

response = _connector.getResponses("GET /ctx/auth/info HTTP/1.0\r\n" +
"Authorization: Basic " + B64Code.encode("user2:password") + "\r\n" +
Expand Down Expand Up @@ -885,6 +852,7 @@ public void testRelaxedMethod() throws Exception
}
private class RequestHandler extends AbstractHandler
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response ) throws IOException, ServletException
{
baseRequest.setHandled(true);
Expand Down Expand Up @@ -916,16 +884,19 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques

UserIdentity.Scope scope = new UserIdentity.Scope()
{
@Override
public String getContextPath()
{
return "/";
}

@Override
public String getName()
{
return "someServlet";
}

@Override
public Map<String, String> getRoleRefMap()
{
Map<String, String> map = new HashMap<>();
Expand All @@ -949,6 +920,7 @@ public Map<String, String> getRoleRefMap()

private class RoleCheckHandler extends AbstractHandler
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response ) throws IOException, ServletException
{
((Request) request).setHandled(true);
Expand Down
56 changes: 23 additions & 33 deletions jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,9 @@ public static Context getCurrentContext()
private Object _requestAttributeListeners;
private Map<String, Object> _managedAttributes;
private String[] _protectedTargets;

private boolean _shutdown = false;

private boolean _available = true;
private volatile int _availability; // 0=STOPPED, 1=AVAILABLE, 2=SHUTDOWN, 3=UNAVAILABLE

private final static int __STOPPED = 0, __AVAILABLE = 1, __SHUTDOWN = 2, __UNAVAILABLE = 3;

public enum Availability { AVAILABLE,SHUTDOWN,UNAVAILABLE};
private volatile Availability _availability;

/* ------------------------------------------------------------ */
/**
Expand Down Expand Up @@ -629,12 +625,15 @@ public void restrictEventListener (EventListener listener)
/**
* @return true if this context is accepting new requests
*/
@ManagedAttribute("false if this context is accepting new requests. true for graceful shutdown, which allows existing requests to complete")
@ManagedAttribute("true for graceful shutdown, which allows existing requests to complete")
public boolean isShutdown()
{
synchronized (this)
switch(_availability)
{
return !_shutdown;
case SHUTDOWN:
return true;
default:
return false;
}
}

Expand All @@ -644,13 +643,10 @@ public boolean isShutdown()
* requests can complete, but no new requests are accepted.
*
*/
@Override
public void shutdown()
{
synchronized (this)
{
_shutdown = true;
_availability = isRunning() ? __SHUTDOWN : __STOPPED;
}
_availability = isRunning() ? Availability.SHUTDOWN : Availability.UNAVAILABLE;
}

/* ------------------------------------------------------------ */
Expand All @@ -659,10 +655,7 @@ public void shutdown()
*/
public boolean isAvailable()
{
synchronized (this)
{
return _available;
}
return _availability==Availability.AVAILABLE;
}

/* ------------------------------------------------------------ */
Expand All @@ -673,8 +666,10 @@ public void setAvailable(boolean available)
{
synchronized (this)
{
_available = available;
_availability = isRunning()?(_shutdown?__SHUTDOWN:_available?__AVAILABLE:__UNAVAILABLE):__STOPPED;
if (available && isRunning())
_availability = Availability.AVAILABLE;
else if (!available || !isRunning())
_availability = Availability.UNAVAILABLE;
}
}

Expand All @@ -697,7 +692,7 @@ public void setLogger(Logger logger)
@Override
protected void doStart() throws Exception
{
_availability = __STOPPED;
_availability = Availability.UNAVAILABLE;

if (_contextPath == null)
throw new IllegalStateException("Null contextPath");
Expand Down Expand Up @@ -726,10 +721,7 @@ protected void doStart() throws Exception
// defers the calling of super.doStart()
startContext();

synchronized(this)
{
_availability = _shutdown?__SHUTDOWN:_available?__AVAILABLE:__UNAVAILABLE;
}
_availability = Availability.AVAILABLE;
}
finally
{
Expand Down Expand Up @@ -806,7 +798,7 @@ public void callContextDestroyed (ServletContextListener l, ServletContextEvent
@Override
protected void doStop() throws Exception
{
_availability = __STOPPED;
_availability = Availability.UNAVAILABLE;

ClassLoader old_classloader = null;
Thread current_thread = null;
Expand Down Expand Up @@ -867,10 +859,8 @@ public boolean checkContext(final String target, final Request baseRequest, fina

switch (_availability)
{
case __STOPPED:
case __SHUTDOWN:
return false;
case __UNAVAILABLE:
case UNAVAILABLE:
case SHUTDOWN:
baseRequest.setHandled(true);
response.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
return false;
Expand Down Expand Up @@ -1517,8 +1507,8 @@ public String toString()
b.append(s.charAt(0)).append('.');
}
}
b.append(getClass().getSimpleName());
b.append('{').append(getContextPath()).append(',').append(getBaseResource());
b.append(getClass().getSimpleName()).append('@').append(Integer.toString(hashCode(),16));
b.append('{').append(getContextPath()).append(',').append(getBaseResource()).append(',').append(_availability);

if (vhosts != null && vhosts.length > 0)
b.append(',').append(vhosts[0]);
Expand Down
15 changes: 15 additions & 0 deletions jetty-server/src/test/java/org/eclipse/jetty/server/LocalConnectorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,21 @@ public void testOneGET() throws Exception
assertThat(response,containsString("pathInfo=/R1"));
}

@Test
public void testStopStart() throws Exception
{
String response=_connector.getResponses("GET /R1 HTTP/1.0\r\n\r\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,containsString("pathInfo=/R1"));

_server.stop();
_server.start();

response=_connector.getResponses("GET /R2 HTTP/1.0\r\n\r\n");
assertThat(response,containsString("HTTP/1.1 200 OK"));
assertThat(response,containsString("pathInfo=/R2"));
}

@Test
public void testTwoGETs() throws Exception
{
Expand Down

0 comments on commit 1d89bef

Please sign in to comment.