Skip to content

Commit 61761e6

Browse files
slominskir-coding-agent[bot]claude
authored andcommitted
Create HTTP sessions only for WebSocket handshakes
RequestListener created an HTTP session for every request, including /caget, /healthcheck and static files, only to pass the remote address to the WebSocket handshake. A client without cookies, such as a script or a health probe, got a new session on every request, each kept for the 30 minute session timeout. Enough requests exhausted the heap. RequestListener now creates the session only when the Upgrade header asks for a WebSocket. The JSP pages don't use sessions and no longer create them (session="false"). AuditServerEndpointConfigurator copes with a handshake that has no session. Fixes #44 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent a74e686 commit 61761e6

8 files changed

Lines changed: 209 additions & 11 deletions

File tree

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package org.jlab.epics2web;
2+
3+
import static org.junit.Assert.assertEquals;
4+
import static org.junit.Assert.assertNotEquals;
5+
import static org.junit.Assert.assertNotNull;
6+
7+
import java.net.URI;
8+
import java.net.http.HttpClient;
9+
import java.net.http.HttpRequest;
10+
import java.net.http.HttpResponse;
11+
import java.net.http.WebSocket;
12+
import java.util.List;
13+
import java.util.UUID;
14+
import java.util.regex.Matcher;
15+
import java.util.regex.Pattern;
16+
import org.junit.Test;
17+
18+
/**
19+
* Tests that only WebSocket handshakes create HTTP sessions (#44): a session per request kept
20+
* clients without cookies, such as scripts, from being cheap to serve.
21+
*/
22+
public class HttpSessionTest {
23+
24+
private static final HttpClient HTTP = HttpClient.newHttpClient();
25+
26+
@Test
27+
public void requestsDoNotCreateSessions() throws Exception {
28+
for (String path :
29+
List.of(
30+
"caget?pv=channel1",
31+
"healthcheck",
32+
"console",
33+
"test-camonitor",
34+
"resources/js/epics2web.js")) {
35+
HttpResponse<String> response = get(path);
36+
37+
assertEquals(path, 200, response.statusCode());
38+
assertEquals(path + " set a cookie", List.of(), response.headers().allValues("Set-Cookie"));
39+
}
40+
}
41+
42+
/** The handshake's session carries the client's address to the endpoint, for the console. */
43+
@Test
44+
public void consoleShowsWebSocketClientAddress() throws Exception {
45+
String name = "session-test-" + UUID.randomUUID();
46+
WebSocket socket =
47+
HTTP.newWebSocketBuilder()
48+
.buildAsync(
49+
URI.create("ws://localhost:8080/epics2web/monitor?clientName=" + name),
50+
new WebSocket.Listener() {})
51+
.join();
52+
53+
try {
54+
String address = consoleAddressOf(name);
55+
assertNotNull("Client not on the console", address);
56+
assertNotEquals("Client address unknown", "Unknown", address);
57+
} finally {
58+
socket.abort();
59+
}
60+
}
61+
62+
/**
63+
* Waits for the console to list the client, and returns its address, or null if it never does.
64+
*/
65+
private static String consoleAddressOf(String name) throws Exception {
66+
// Row cells: ID, IP, agent, name. The server may list the client just after the handshake.
67+
Pattern row =
68+
Pattern.compile("<td>[^<]*</td>\\s*<td>([^<]*)</td>\\s*<td>[^<]*</td>\\s*<td>" + name);
69+
long deadline = System.currentTimeMillis() + 5_000;
70+
while (true) {
71+
Matcher matcher = row.matcher(get("console").body());
72+
if (matcher.find()) {
73+
return matcher.group(1);
74+
}
75+
if (System.currentTimeMillis() > deadline) {
76+
return null;
77+
}
78+
Thread.sleep(100);
79+
}
80+
}
81+
82+
private static HttpResponse<String> get(String path) throws Exception {
83+
return HTTP.send(
84+
HttpRequest.newBuilder().uri(URI.create("http://localhost:8080/epics2web/" + path)).build(),
85+
HttpResponse.BodyHandlers.ofString());
86+
}
87+
}

‎src/main/java/org/jlab/epics2web/websocket/AuditServerEndpointConfigurator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ public void modifyHandshake(
2020
ServerEndpointConfig config, HandshakeRequest request, HandshakeResponse response) {
2121

2222
Map<String, List<String>> headers = request.getHeaders();
23-
String remoteAddr =
24-
(String) ((HttpSession) request.getHttpSession()).getAttribute("remoteAddr");
23+
HttpSession session = (HttpSession) request.getHttpSession();
24+
String remoteAddr = session == null ? null : (String) session.getAttribute("remoteAddr");
2525

2626
// We don't use config.getUserProperties.add because it isn't one-to-one with a web socket
2727
// connection

‎src/main/java/org/jlab/epics2web/websocket/RequestListener.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,15 @@
55
import jakarta.servlet.annotation.WebListener;
66
import jakarta.servlet.http.HttpServletRequest;
77
import jakarta.servlet.http.HttpSession;
8+
import java.util.Locale;
89

910
/**
10-
* Stores the remote address in the user's session. This is necessary since the Java web socket API
11-
* does not expose the remote address. Note: some implementations do (Tomcat does not, GlassFish
12-
* does).
11+
* Stores the remote address in the user's session for WebSocket handshakes. This is necessary since
12+
* the Java web socket API does not expose the remote address. Note: some implementations do (Tomcat
13+
* does not, GlassFish does).
14+
*
15+
* <p>Only handshakes get a session: a session for every request would keep one per request, for the
16+
* session timeout, for clients without cookies such as scripts and health probes.
1317
*
1418
* @author slominskir
1519
*/
@@ -23,8 +27,15 @@ public void requestDestroyed(ServletRequestEvent sre) {}
2327
public void requestInitialized(ServletRequestEvent sre) {
2428
HttpServletRequest request = (HttpServletRequest) sre.getServletRequest();
2529

26-
HttpSession session = request.getSession();
30+
if (isWebSocketHandshake(request)) {
31+
HttpSession session = request.getSession();
32+
33+
session.setAttribute("remoteAddr", request.getRemoteAddr());
34+
}
35+
}
2736

28-
session.setAttribute("remoteAddr", request.getRemoteAddr());
37+
static boolean isWebSocketHandshake(HttpServletRequest request) {
38+
String upgrade = request.getHeader("Upgrade");
39+
return upgrade != null && upgrade.toLowerCase(Locale.ROOT).contains("websocket");
2940
}
3041
}

‎src/main/webapp/WEB-INF/views/console.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<%@page contentType="text/html" pageEncoding="UTF-8"%>
1+
<%@page contentType="text/html" pageEncoding="UTF-8" session="false"%>
22
<%@taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core"%>
33
<%@taglib prefix="fmt" uri="http://java.sun.com/jsp/jstl/fmt"%>
44
<!DOCTYPE html>

‎src/main/webapp/WEB-INF/views/overview.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<%@page contentType="text/html" pageEncoding="UTF-8"%>
1+
<%@page contentType="text/html" pageEncoding="UTF-8" session="false"%>
22
<!DOCTYPE html>
33
<html>
44
<head>

‎src/main/webapp/WEB-INF/views/test-caget.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<%@page contentType="text/html" pageEncoding="UTF-8"%>
1+
<%@page contentType="text/html" pageEncoding="UTF-8" session="false"%>
22
<!DOCTYPE html>
33
<html>
44
<head>

‎src/main/webapp/WEB-INF/views/test-camonitor.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<%@page contentType="text/html" pageEncoding="UTF-8"%>
1+
<%@page contentType="text/html" pageEncoding="UTF-8" session="false"%>
22
<%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %>
33
<%@taglib prefix="app" uri="http://jlab.org/app/functions"%>
44
<!DOCTYPE html>
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
package org.jlab.epics2web.websocket;
2+
3+
import static org.junit.Assert.assertEquals;
4+
import static org.junit.Assert.assertNull;
5+
6+
import jakarta.servlet.ServletContext;
7+
import jakarta.servlet.ServletRequestEvent;
8+
import jakarta.servlet.http.HttpServletRequest;
9+
import jakarta.servlet.http.HttpSession;
10+
import java.lang.reflect.Proxy;
11+
import java.util.HashMap;
12+
import java.util.Map;
13+
import org.junit.Test;
14+
15+
public class RequestListenerTest {
16+
17+
/** Session attributes, or null if no session was created. */
18+
private Map<String, Object> session;
19+
20+
@Test
21+
public void webSocketHandshakeGetsSessionWithRemoteAddress() {
22+
requestInitialized("websocket");
23+
24+
assertEquals(Map.of("remoteAddr", "10.0.0.1"), session);
25+
}
26+
27+
@Test
28+
public void upgradeHeaderIsCaseInsensitive() {
29+
requestInitialized("WebSocket");
30+
31+
assertEquals("10.0.0.1", session.get("remoteAddr"));
32+
}
33+
34+
/** Other requests, such as /caget from a script without cookies, must not create sessions. */
35+
@Test
36+
public void otherRequestsGetNoSession() {
37+
requestInitialized(null);
38+
assertNull(session);
39+
40+
requestInitialized("h2c");
41+
assertNull(session);
42+
}
43+
44+
private void requestInitialized(String upgradeHeader) {
45+
session = null;
46+
HttpSession httpSession =
47+
proxy(
48+
HttpSession.class,
49+
(method, args) -> {
50+
if (method.equals("setAttribute")) {
51+
session.put((String) args[0], args[1]);
52+
return null;
53+
}
54+
throw new UnsupportedOperationException(method);
55+
});
56+
HttpServletRequest request =
57+
proxy(
58+
HttpServletRequest.class,
59+
(method, args) ->
60+
switch (method) {
61+
case "getHeader" ->
62+
"Upgrade".equalsIgnoreCase((String) args[0]) ? upgradeHeader : null;
63+
case "getRemoteAddr" -> "10.0.0.1";
64+
case "getSession" -> {
65+
if (session == null) {
66+
session = new HashMap<>();
67+
}
68+
yield httpSession;
69+
}
70+
default -> throw new UnsupportedOperationException(method);
71+
});
72+
ServletContext context =
73+
proxy(
74+
ServletContext.class,
75+
(method, args) -> {
76+
throw new UnsupportedOperationException(method);
77+
});
78+
79+
new RequestListener().requestInitialized(new ServletRequestEvent(context, request));
80+
}
81+
82+
private interface Handler {
83+
Object handle(String method, Object[] args);
84+
}
85+
86+
@SuppressWarnings("unchecked")
87+
private static <T> T proxy(Class<T> type, Handler handler) {
88+
return (T)
89+
Proxy.newProxyInstance(
90+
type.getClassLoader(),
91+
new Class<?>[] {type},
92+
(p, method, args) ->
93+
switch (method.getName()) {
94+
case "hashCode" -> System.identityHashCode(p);
95+
case "equals" -> p == args[0];
96+
case "toString" -> type.getSimpleName();
97+
default -> handler.handle(method.getName(), args);
98+
});
99+
}
100+
}

0 commit comments

Comments
 (0)