From f57d2c167e687156cc385092589282b6ada21640 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Mon, 3 Feb 2014 11:36:27 -0800 Subject: [PATCH] #518: Send roughly-appropriate codes and close the WebSocket connection when errors are encountered. --- .../GuacamoleWebSocketTunnelServlet.java | 48 +++++++++++---- .../GuacamoleWebSocketTunnelServlet.java | 59 ++++++++++++------- 2 files changed, 73 insertions(+), 34 deletions(-) diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/jetty/GuacamoleWebSocketTunnelServlet.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/jetty/GuacamoleWebSocketTunnelServlet.java index 7a5826f3d..7e5692f49 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/jetty/GuacamoleWebSocketTunnelServlet.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/jetty/GuacamoleWebSocketTunnelServlet.java @@ -31,6 +31,9 @@ import org.glyptodon.guacamole.net.GuacamoleTunnel; import org.eclipse.jetty.websocket.WebSocket; import org.eclipse.jetty.websocket.WebSocket.Connection; import org.eclipse.jetty.websocket.WebSocketServlet; +import org.glyptodon.guacamole.GuacamoleClientException; +import org.glyptodon.guacamole.GuacamoleResourceNotFoundException; +import org.glyptodon.guacamole.GuacamoleSecurityException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,7 +47,7 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet { /** * Logger for this class. */ - private static Logger logger = LoggerFactory.getLogger(GuacamoleWebSocketTunnelServlet.class); + private static final Logger logger = LoggerFactory.getLogger(GuacamoleWebSocketTunnelServlet.class); /** * The default, minimum buffer size for instructions. @@ -96,25 +99,46 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet { char[] readMessage; try { - while ((readMessage = reader.read()) != null) { - // Buffer message - buffer.append(readMessage); + try { + while ((readMessage = reader.read()) != null) { + + // Buffer message + buffer.append(readMessage); + + // Flush if we expect to wait or buffer is getting full + if (!reader.available() || buffer.length() >= BUFFER_SIZE) { + connection.sendMessage(buffer.toString()); + buffer.setLength(0); + } - // Flush if we expect to wait or buffer is getting full - if (!reader.available() || buffer.length() >= BUFFER_SIZE) { - connection.sendMessage(buffer.toString()); - buffer.setLength(0); } - } + + // Catch any thrown guacamole exception and attempt + // to pass within the WebSocket connection, logging + // each error appropriately. + catch (GuacamoleSecurityException e) { + logger.warn("Authorization failed.", e); + connection.close(1008, null); // Policy violation + } + catch (GuacamoleResourceNotFoundException e) { + logger.debug("Resource not found.", e); + connection.close(1002, null); // Protocol error + } + catch (GuacamoleClientException e) { + logger.warn("Error in client request.", e); + connection.close(1002, null); // Protocol error + } + catch (GuacamoleException e) { + logger.error("Server error in tunnel", e); + connection.close(1011, null); // Server error + } + } catch (IOException e) { logger.debug("Tunnel read failed due to I/O error.", e); } - catch (GuacamoleException e) { - logger.debug("Tunnel read failed.", e); - } } diff --git a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/tomcat/GuacamoleWebSocketTunnelServlet.java b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/tomcat/GuacamoleWebSocketTunnelServlet.java index 5023500c5..b774a2a17 100644 --- a/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/tomcat/GuacamoleWebSocketTunnelServlet.java +++ b/guacamole/src/main/java/org/glyptodon/guacamole/net/basic/websocket/tomcat/GuacamoleWebSocketTunnelServlet.java @@ -27,6 +27,7 @@ import java.io.InputStream; import java.io.Reader; import java.nio.CharBuffer; import javax.servlet.http.HttpServletRequest; +import org.apache.catalina.websocket.Constants; import org.glyptodon.guacamole.GuacamoleException; import org.glyptodon.guacamole.io.GuacamoleReader; import org.glyptodon.guacamole.io.GuacamoleWriter; @@ -34,6 +35,9 @@ import org.glyptodon.guacamole.net.GuacamoleTunnel; import org.apache.catalina.websocket.StreamInbound; import org.apache.catalina.websocket.WebSocketServlet; import org.apache.catalina.websocket.WsOutbound; +import org.glyptodon.guacamole.GuacamoleClientException; +import org.glyptodon.guacamole.GuacamoleResourceNotFoundException; +import org.glyptodon.guacamole.GuacamoleSecurityException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -52,7 +56,7 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet { /** * Logger for this class. */ - private Logger logger = LoggerFactory.getLogger(GuacamoleWebSocketTunnelServlet.class); + private final Logger logger = LoggerFactory.getLogger(GuacamoleWebSocketTunnelServlet.class); @Override public StreamInbound createWebSocketInbound(String protocol, HttpServletRequest request) { @@ -101,40 +105,51 @@ public abstract class GuacamoleWebSocketTunnelServlet extends WebSocketServlet { @Override public void run() { - CharBuffer charBuffer = CharBuffer.allocate(BUFFER_SIZE); StringBuilder buffer = new StringBuilder(BUFFER_SIZE); GuacamoleReader reader = tunnel.acquireReader(); char[] readMessage; try { - while ((readMessage = reader.read()) != null) { - // Buffer message - buffer.append(readMessage); + // Attempt to read + try { + while ((readMessage = reader.read()) != null) { - // Flush if we expect to wait or buffer is getting full - if (!reader.available() || buffer.length() >= BUFFER_SIZE) { + // Buffer message + buffer.append(readMessage); - // Reallocate buffer if necessary - if (buffer.length() > charBuffer.length()) - charBuffer = CharBuffer.allocate(buffer.length()); - else - charBuffer.clear(); + // Flush if we expect to wait or buffer is getting full + if (!reader.available() || buffer.length() >= BUFFER_SIZE) { + outbound.writeTextMessage(CharBuffer.wrap(buffer)); + buffer.setLength(0); + } - charBuffer.put(buffer.toString().toCharArray()); - charBuffer.flip(); - - outbound.writeTextMessage(charBuffer); - buffer.setLength(0); } - } + + // Catch any thrown guacamole exception and attempt + // to pass within the WebSocket connection, logging + // each error appropriately. + catch (GuacamoleSecurityException e) { + logger.warn("Authorization failed.", e); + outbound.close(Constants.STATUS_POLICY_VIOLATION, null); + } + catch (GuacamoleResourceNotFoundException e) { + logger.debug("Resource not found.", e); + outbound.close(Constants.STATUS_PROTOCOL_ERROR, null); + } + catch (GuacamoleClientException e) { + logger.warn("Error in client request.", e); + outbound.close(Constants.STATUS_PROTOCOL_ERROR, null); + } + catch (GuacamoleException e) { + logger.error("Server error in tunnel", e); + outbound.close(Constants.STATUS_UNEXPECTED_CONDITION, null); + } + } catch (IOException e) { - logger.debug("Tunnel read failed due to I/O error.", e); - } - catch (GuacamoleException e) { - logger.debug("Tunnel read failed.", e); + logger.debug("I/O error prevents further reads.", e); } }