Skip to content

Commit

Permalink
Issue #10323 Fix ee10 Request.isRequestedSessionIdValid (#10331)
Browse files Browse the repository at this point in the history 
* Issue #10323 Fix ee10 Request.isRequestedSessionIdValid
  • Loading branch information
@janbartel
janbartel authored Aug 18, 2023
1 parent 5160ad2 commit fe18310
Show file tree
Hide file tree
Showing 3 changed files with 258 additions and 14 deletions.
2 changes: 1 addition & 1 deletion ...10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ public String changeSessionId()
public boolean isRequestedSessionIdValid()
{
AbstractSessionManager.RequestedSession requestedSession = getServletRequestInfo().getRequestedSession();
return requestedSession != null && requestedSession.sessionId() != null && !requestedSession.sessionIdFromCookie();
return requestedSession != null && requestedSession.sessionId() != null && requestedSession.session() != null;
}

@Override
Expand Down
194 changes: 187 additions & 7 deletions ...0/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/SessionHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.PrintWriter;
import java.net.URI;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
import java.util.function.Consumer;
import java.util.function.Function;

Expand Down Expand Up @@ -57,6 +60,7 @@
import org.eclipse.jetty.toolchain.test.IO;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.URIUtil;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;

Expand Down Expand Up @@ -359,6 +363,162 @@ public void testSimpleSessionCreation() throws Exception
}
}

@Test
public void testRequestedSessionIdFromCookie() throws Exception
{
String contextPath = "/";
String servletMapping = "/server";

Server server = new Server();
ServerConnector connector = new ServerConnector(server);
server.addConnector(connector);

DefaultSessionIdManager sessionIdManager = new DefaultSessionIdManager(server);
server.addBean(sessionIdManager, true);
DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
server.addBean(cacheFactory);

SessionDataStoreFactory storeFactory = new NullSessionDataStoreFactory();
server.addBean(storeFactory);

HouseKeeper housekeeper = new HouseKeeper();
housekeeper.setIntervalSec(-1); //turn off scavenging
sessionIdManager.setSessionHouseKeeper(housekeeper);

ServletContextHandler context = new ServletContextHandler();
context.setContextPath(contextPath);
server.setHandler(context);
SessionHandler sessionHandler = new SessionHandler();
sessionHandler.setSessionIdManager(sessionIdManager);
sessionHandler.setMaxInactiveInterval(-1); //immortal session
context.setSessionHandler(sessionHandler);

TestRequestedSessionIdServlet servlet = new TestRequestedSessionIdServlet();
ServletHolder holder = new ServletHolder(servlet);
context.addServlet(holder, servletMapping);

server.start();
int port = connector.getLocalPort();
try (StacklessLogging stackless = new StacklessLogging(SessionHandlerTest.class.getPackage()))
{
HttpClient client = new HttpClient();
client.start();

//test with no session cookie
String path = contextPath + (contextPath.endsWith("/") && servletMapping.startsWith("/") ? servletMapping.substring(1) : servletMapping);
String url = "http://localhost:" + port + path;
ContentResponse response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK, response.getStatus());
assertThat(response.getContentAsString(), containsString("valid=false"));

//test with a cookie for non-existant session
URI uri = URIUtil.toURI(URIUtil.newURI("http", "localhost", port, path, ""));
HttpCookie cookie = HttpCookie.build(SessionHandler.__DefaultSessionCookie, "123456789").path("/").domain("localhost").build();
client.getHttpCookieStore().add(uri, cookie);
response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK, response.getStatus());
String content = response.getContentAsString();
assertThat(content, containsString("requestedId=123456789"));
assertThat(content, containsString("valid=false"));

//Get rid of fake cookie
client.getHttpCookieStore().clear();

//Make a real session
response = client.GET(url + "?action=create");
assertEquals(HttpServletResponse.SC_OK, response.getStatus());
assertNotNull(response.getHeaders().get("Set-Cookie"));

//Check the requestedSessionId is valid
response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK, response.getStatus());
content = response.getContentAsString();
assertThat(content, containsString("valid=true"));
}
finally
{
server.stop();
}
}

@Test
public void testRequestedSessionIdFromURL() throws Exception
{
String contextPath = "/";
String servletMapping = "/server";

Server server = new Server();
ServerConnector connector = new ServerConnector(server);
server.addConnector(connector);

DefaultSessionIdManager sessionIdManager = new DefaultSessionIdManager(server);
server.addBean(sessionIdManager, true);
DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
server.addBean(cacheFactory);

SessionDataStoreFactory storeFactory = new NullSessionDataStoreFactory();
server.addBean(storeFactory);

HouseKeeper housekeeper = new HouseKeeper();
housekeeper.setIntervalSec(-1); //turn off scavenging
sessionIdManager.setSessionHouseKeeper(housekeeper);

ServletContextHandler context = new ServletContextHandler();
context.setContextPath(contextPath);
server.setHandler(context);
SessionHandler sessionHandler = new SessionHandler();
sessionHandler.setUsingCookies(false);
sessionHandler.setSessionIdManager(sessionIdManager);
sessionHandler.setMaxInactiveInterval(-1); //immortal session
context.setSessionHandler(sessionHandler);

TestRequestedSessionIdServlet servlet = new TestRequestedSessionIdServlet();
ServletHolder holder = new ServletHolder(servlet);
context.addServlet(holder, servletMapping);

server.start();
int port = connector.getLocalPort();
try (StacklessLogging stackless = new StacklessLogging(SessionHandlerTest.class.getPackage()))
{
HttpClient client = new HttpClient();
client.start();

//test with no session cookie
String path = contextPath + (contextPath.endsWith("/") && servletMapping.startsWith("/") ? servletMapping.substring(1) : servletMapping);
String url = "http://localhost:" + port + path;
ContentResponse response = client.GET(url);
assertEquals(HttpServletResponse.SC_OK, response.getStatus());
assertThat(response.getContentAsString(), containsString("valid=false"));

//test with id for non-existent session
response = client.GET(url + ";" + SessionHandler.__DefaultSessionIdPathParameterName + "=" + "123456789");
assertEquals(HttpServletResponse.SC_OK, response.getStatus());
String content = response.getContentAsString();
assertThat(content, containsString("requestedId=123456789"));
assertThat(content, containsString("valid=false"));

//Make a real session
response = client.GET(url + "?action=create");
assertEquals(HttpServletResponse.SC_OK, response.getStatus());
content = response.getContentAsString();
assertThat(content, containsString("createdId="));
String sessionId = content.substring(content.indexOf("createdId=") + 10);
sessionId = sessionId.trim();

//Check the requestedSessionId is valid
response = client.GET(url + ";" + SessionHandler.__DefaultSessionIdPathParameterName + "=" + sessionId);
assertEquals(HttpServletResponse.SC_OK, response.getStatus());
content = response.getContentAsString();
assertThat(content, containsString("valid=true"));
}
finally
{
server.stop();
}
}

public static class TestServlet extends HttpServlet
{
private static final long serialVersionUID = 1L;
Expand Down Expand Up @@ -386,6 +546,26 @@ else if (action != null && "test".equals(action))
}
}

public static class TestRequestedSessionIdServlet extends HttpServlet
{
private static final long serialVersionUID = 1L;
public String _id = null;

@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
PrintWriter writer = response.getWriter();
writer.println("requestedId=" + request.getRequestedSessionId());
writer.println("valid=" + request.isRequestedSessionIdValid());

if ("create".equals(request.getParameter("action")))
{
HttpSession session = request.getSession(true);
writer.println("createdId=" + session.getId());
}
}
}

public class MockSessionCache extends AbstractSessionCache
{

Expand All @@ -395,8 +575,9 @@ public MockSessionCache(SessionHandler manager)
}

@Override
public void shutdown()
public ManagedSession doDelete(String key)
{
return null;
}

@Override
Expand All @@ -411,12 +592,6 @@ public Session doPutIfAbsent(String key, ManagedSession session)
return null;
}

@Override
public ManagedSession doDelete(String key)
{
return null;
}

@Override
public boolean doReplace(String id, ManagedSession oldValue, ManagedSession newValue)
{
Expand All @@ -429,6 +604,11 @@ public ManagedSession newSession(SessionData data)
return null;
}

@Override
public void shutdown()
{
}

@Override
protected ManagedSession doComputeIfAbsent(String id, Function<String, ManagedSession> mappingFunction)
{
Expand Down
76 changes: 70 additions & 6 deletions ...y-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/SessionHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand Down Expand Up @@ -87,6 +86,7 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
String pathInContext = request.getPathInfo();
String[] split = pathInContext.substring(1).split("/");

String requestedSessionId = request.getRequestedSessionId();
HttpSession session = request.getSession(false);

if (split.length > 0)
Expand Down Expand Up @@ -135,6 +135,10 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
}

StringBuilder out = new StringBuilder();
out.append("requestedSessionId=" + requestedSessionId).append('\n');
out.append("requestedSessionIdValid=" + request.isRequestedSessionIdValid()).append('\n');


if (session == null)
out.append("No Session\n");
else
Expand Down Expand Up @@ -309,8 +313,9 @@ public void testCreateSession() throws Exception
String setCookie = response.get("SET-COOKIE");
assertThat(setCookie, containsString("Path=/"));
String content = response.getContent();
assertThat(content, startsWith("Session="));
String id = content.substring(content.indexOf('=') + 1, content.indexOf('\n'));
assertThat(content, containsString("Session="));
String id = content.substring(content.indexOf("Session=") + 8);
id = id.trim();
assertThat(id, not(equalTo("oldCookieId")));

endPoint.addInput("""
Expand All @@ -326,6 +331,64 @@ public void testCreateSession() throws Exception
assertThat(content, containsString("Session=" + id));
}

@Test
public void testRequestedSessionIdFromCookie() throws Exception
{
_server.stop();
_sessionHandler.setSessionPath(null);
_contextHandler.setContextPath("/");
_server.start();

//test with no session cookie
LocalConnector.LocalEndPoint endPoint = _connector.connect();
endPoint.addInput("""
GET / HTTP/1.1
Host: localhost
""");

HttpTester.Response response = HttpTester.parseResponse(endPoint.getResponse());
assertThat(response.getStatus(), equalTo(200));
assertThat(response.getContent(), containsString("No Session"));
assertThat(response.getContent(), containsString("requestedSessionIdValid=false"));

//test with a cookie for non-existant session
endPoint.addInput("""
GET / HTTP/1.1
Host: localhost
Cookie: JSESSIONID=%s
""".formatted("123456789"));
response = HttpTester.parseResponse(endPoint.getResponse());
assertThat(response.getStatus(), equalTo(200));
assertThat(response.getContent(), containsString("No Session"));
assertThat(response.getContent(), containsString("requestedSessionIdValid=false"));

//Make a real session
endPoint.addInput("""
GET /create HTTP/1.1
Host: localhost
""");

response = HttpTester.parseResponse(endPoint.getResponse());
assertThat(response.getStatus(), equalTo(200));
String content = response.getContent();
assertThat(content, containsString("Session="));
String id = content.substring(content.indexOf("Session=") + 8);
id = id.trim();

//Check the requestedSessionId is valid
endPoint.addInput("""
GET / HTTP/1.1
Host: localhost
Cookie: JSESSIONID=%s
""".formatted(id));
response = HttpTester.parseResponse(endPoint.getResponse());
assertThat(response.getContent(), containsString("requestedSessionIdValid=true"));
}

@Test
public void testSetAttribute() throws Exception
{
Expand All @@ -339,8 +402,9 @@ public void testSetAttribute() throws Exception
HttpTester.Response response = HttpTester.parseResponse(endPoint.getResponse());
assertThat(response.getStatus(), equalTo(200));
String content = response.getContent();
assertThat(content, startsWith("Session="));
String id = content.substring(content.indexOf('=') + 1, content.indexOf('\n'));
assertThat(content, containsString("Session="));
String id = content.substring(content.indexOf("Session=") + 8);
id = id.trim();

endPoint.addInput("""
GET /set/attribute/value HTTP/1.1
Expand Down Expand Up @@ -380,7 +444,7 @@ public void testChangeSessionId() throws Exception
HttpTester.Response response = HttpTester.parseResponse(endPoint.getResponse());
assertThat(response.getStatus(), equalTo(200));
String content = response.getContent();
assertThat(content, startsWith("Session="));
assertThat(content, containsString("Session="));

String setCookie = response.get(HttpHeader.SET_COOKIE);
String id = setCookie.substring(setCookie.indexOf("JSESSIONID=") + 11, setCookie.indexOf("; Path=/"));
Expand Down

0 comments on commit fe18310

Please sign in to comment.