Use appropriate exceptions where possible, turn exceptions into appropriate error codes.

This commit is contained in:
Michael Jumper
2012-03-24 22:03:52 -07:00
parent f42fe962e2
commit 9b36638bf3
7 changed files with 75 additions and 48 deletions

View File

@@ -41,6 +41,7 @@ import java.io.IOException;
import java.io.Reader; import java.io.Reader;
import java.util.LinkedList; import java.util.LinkedList;
import net.sourceforge.guacamole.GuacamoleException; import net.sourceforge.guacamole.GuacamoleException;
import net.sourceforge.guacamole.GuacamoleServerException;
import net.sourceforge.guacamole.protocol.GuacamoleInstruction; import net.sourceforge.guacamole.protocol.GuacamoleInstruction;
import net.sourceforge.guacamole.protocol.GuacamoleInstruction.Operation; import net.sourceforge.guacamole.protocol.GuacamoleInstruction.Operation;
import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.ArrayUtils;
@@ -79,7 +80,7 @@ public class ReaderGuacamoleReader implements GuacamoleReader {
return input.ready() || usedLength != 0; return input.ready() || usedLength != 0;
} }
catch (IOException e) { catch (IOException e) {
throw new GuacamoleException(e); throw new GuacamoleServerException(e);
} }
} }
@@ -144,7 +145,7 @@ public class ReaderGuacamoleReader implements GuacamoleReader {
// Handle invalid terminator characters // Handle invalid terminator characters
else if (terminator != ',') else if (terminator != ',')
throw new GuacamoleException("Element terminator of instruction was not ';' nor ','"); throw new GuacamoleServerException("Element terminator of instruction was not ';' nor ','");
} }
@@ -156,7 +157,7 @@ public class ReaderGuacamoleReader implements GuacamoleReader {
// Otherwise, parse error // Otherwise, parse error
else else
throw new GuacamoleException("Non-numeric character in element length."); throw new GuacamoleServerException("Non-numeric character in element length.");
} }
@@ -179,7 +180,7 @@ public class ReaderGuacamoleReader implements GuacamoleReader {
} }
catch (IOException e) { catch (IOException e) {
throw new GuacamoleException(e); throw new GuacamoleServerException(e);
} }
} }

View File

@@ -40,6 +40,7 @@ package net.sourceforge.guacamole.io;
import java.io.IOException; import java.io.IOException;
import java.io.Writer; import java.io.Writer;
import net.sourceforge.guacamole.GuacamoleException; import net.sourceforge.guacamole.GuacamoleException;
import net.sourceforge.guacamole.GuacamoleServerException;
import net.sourceforge.guacamole.protocol.GuacamoleInstruction; import net.sourceforge.guacamole.protocol.GuacamoleInstruction;
/** /**
@@ -72,7 +73,7 @@ public class WriterGuacamoleWriter implements GuacamoleWriter {
output.flush(); output.flush();
} }
catch (IOException e) { catch (IOException e) {
throw new GuacamoleException(e); throw new GuacamoleServerException(e);
} }
} }

View File

@@ -51,6 +51,7 @@ import java.io.OutputStreamWriter;
import java.net.InetSocketAddress; import java.net.InetSocketAddress;
import java.net.SocketAddress; import java.net.SocketAddress;
import net.sourceforge.guacamole.GuacamoleException; import net.sourceforge.guacamole.GuacamoleException;
import net.sourceforge.guacamole.GuacamoleServerException;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
@@ -105,7 +106,7 @@ public class InetGuacamoleSocket implements GuacamoleSocket {
} }
catch (IOException e) { catch (IOException e) {
throw new GuacamoleException(e); throw new GuacamoleServerException(e);
} }
} }
@@ -117,7 +118,7 @@ public class InetGuacamoleSocket implements GuacamoleSocket {
sock.close(); sock.close();
} }
catch (IOException e) { catch (IOException e) {
throw new GuacamoleException(e); throw new GuacamoleServerException(e);
} }
} }

View File

@@ -41,6 +41,7 @@ import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.util.Properties; import java.util.Properties;
import net.sourceforge.guacamole.GuacamoleException; import net.sourceforge.guacamole.GuacamoleException;
import net.sourceforge.guacamole.GuacamoleServerException;
/** /**
* Simple utility class for reading properties from the guacamole.properties * Simple utility class for reading properties from the guacamole.properties
@@ -90,7 +91,7 @@ public class GuacamoleProperties {
} }
catch (IOException e) { catch (IOException e) {
exception = new GuacamoleException("Error reading guacamole.properties", e); exception = new GuacamoleServerException("Error reading guacamole.properties", e);
} }
} }
@@ -135,7 +136,7 @@ public class GuacamoleProperties {
Type value = getProperty(property); Type value = getProperty(property);
if (value == null) if (value == null)
throw new GuacamoleException("Property " + property.getName() + " is required."); throw new GuacamoleServerException("Property " + property.getName() + " is required.");
return value; return value;

View File

@@ -38,6 +38,7 @@ package net.sourceforge.guacamole.properties;
* ***** END LICENSE BLOCK ***** */ * ***** END LICENSE BLOCK ***** */
import net.sourceforge.guacamole.GuacamoleException; import net.sourceforge.guacamole.GuacamoleException;
import net.sourceforge.guacamole.GuacamoleServerException;
/** /**
* A GuacamoleProperty whose value is an integer. * A GuacamoleProperty whose value is an integer.
@@ -58,7 +59,7 @@ public abstract class IntegerGuacamoleProperty implements GuacamoleProperty<Inte
return integer; return integer;
} }
catch (NumberFormatException e) { catch (NumberFormatException e) {
throw new GuacamoleException("Property \"" + getName() + "\" must be an integer.", e); throw new GuacamoleServerException("Property \"" + getName() + "\" must be an integer.", e);
} }
} }

View File

@@ -45,7 +45,7 @@ import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession; import javax.servlet.http.HttpSession;
import net.sourceforge.guacamole.GuacamoleException; import net.sourceforge.guacamole.*;
import net.sourceforge.guacamole.io.GuacamoleReader; import net.sourceforge.guacamole.io.GuacamoleReader;
import net.sourceforge.guacamole.io.GuacamoleWriter; import net.sourceforge.guacamole.io.GuacamoleWriter;
import org.slf4j.Logger; import org.slf4j.Logger;
@@ -71,6 +71,27 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet {
handleTunnelRequest(request, response); handleTunnelRequest(request, response);
} }
private void sendError(HttpServletResponse response, int code) throws ServletException {
try {
// If response not committed, send error code
if (!response.isCommitted())
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}
catch (IOException ioe) {
// If unable to send error at all due to I/O problems,
// rethrow as servlet exception
throw new ServletException(ioe);
}
}
/** /**
* Dispatches every HTTP GET and POST request to the appropriate handler * Dispatches every HTTP GET and POST request to the appropriate handler
* function based on the query string. * function based on the query string.
@@ -87,7 +108,7 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet {
String query = request.getQueryString(); String query = request.getQueryString();
if (query == null) if (query == null)
throw new GuacamoleException("No query string provided."); throw new GuacamoleClientException("No query string provided.");
// If connect operation, call doConnect() and return tunnel UUID // If connect operation, call doConnect() and return tunnel UUID
// in response. // in response.
@@ -106,16 +127,28 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet {
logger.info("Connection from {} succeeded.", request.getRemoteAddr()); logger.info("Connection from {} succeeded.", request.getRemoteAddr());
try { try {
// Send UUID to client
response.getWriter().print(tunnel.getUUID().toString()); response.getWriter().print(tunnel.getUUID().toString());
} }
catch (IOException e) { catch (IOException e) {
throw new GuacamoleException(e); throw new GuacamoleServerException(e);
} }
} }
else
// Failed to connect
else {
logger.info("Connection from {} failed.", request.getRemoteAddr()); logger.info("Connection from {} failed.", request.getRemoteAddr());
try {
// Send error to client
response.sendError(HttpServletResponse.SC_NOT_FOUND);
}
catch (IOException e) {
throw new GuacamoleServerException(e);
}
}
} }
// If read operation, call doRead() with tunnel UUID // If read operation, call doRead() with tunnel UUID
@@ -128,38 +161,26 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet {
// Otherwise, invalid operation // Otherwise, invalid operation
else else
throw new GuacamoleException("Invalid tunnel operation: " + query); throw new GuacamoleClientException("Invalid tunnel operation: " + query);
} }
// Catch any thrown guacamole exception and attempt to pass within the // Catch any thrown guacamole exception and attempt to pass within the
// HTTP response. // HTTP response, logging each error appropriately.
catch (GuacamoleSecurityException e) {
logger.warn("Authorization failed.", e);
sendError(response, HttpServletResponse.SC_UNAUTHORIZED);
}
catch (GuacamoleResourceNotFoundException e) {
logger.debug("Resource not found.", e);
sendError(response, HttpServletResponse.SC_NOT_FOUND);
}
catch (GuacamoleClientException e) {
logger.warn("Error in client request.", e);
sendError(response, HttpServletResponse.SC_BAD_REQUEST);
}
catch (GuacamoleException e) { catch (GuacamoleException e) {
logger.error("Server error in tunnel", e);
try { sendError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
// If response not committed, send error code along with
// message.
if (!response.isCommitted()) {
response.setHeader("X-Guacamole-Error-Message", e.getMessage());
response.sendError(
HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
e.getMessage()
);
}
// If unable to send error code, rethrow as servlet exception
else
throw new ServletException(e);
}
catch (IOException ioe) {
// If unable to send error at all due to I/O problems,
// rethrow as servlet exception
throw new ServletException(ioe);
}
} }
} }
@@ -206,7 +227,7 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet {
GuacamoleTunnel tunnel = session.getTunnel(tunnelUUID); GuacamoleTunnel tunnel = session.getTunnel(tunnelUUID);
if (tunnel == null) if (tunnel == null)
throw new GuacamoleException("No such tunnel."); throw new GuacamoleResourceNotFoundException("No such tunnel.");
// Obtain exclusive read access // Obtain exclusive read access
GuacamoleReader reader = tunnel.acquireReader(); GuacamoleReader reader = tunnel.acquireReader();
@@ -224,7 +245,7 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet {
// data yet. // data yet.
char[] message = reader.read(); char[] message = reader.read();
if (message == null) if (message == null)
throw new GuacamoleException("Disconnected."); throw new GuacamoleResourceNotFoundException("Tunnel reached end of stream.");
// For all messages, until another stream is ready (we send at least one message) // For all messages, until another stream is ready (we send at least one message)
do { do {
@@ -268,7 +289,7 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet {
session.detachTunnel(tunnel); session.detachTunnel(tunnel);
tunnel.close(); tunnel.close();
throw new GuacamoleException("I/O error writing to servlet output stream.", e); throw new GuacamoleServerException("I/O error writing to servlet output stream.", e);
} }
finally { finally {
tunnel.releaseReader(); tunnel.releaseReader();
@@ -299,7 +320,7 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet {
GuacamoleTunnel tunnel = session.getTunnel(tunnelUUID); GuacamoleTunnel tunnel = session.getTunnel(tunnelUUID);
if (tunnel == null) if (tunnel == null)
throw new GuacamoleException("No such tunnel."); throw new GuacamoleResourceNotFoundException("No such tunnel.");
// We still need to set the content type to avoid the default of // We still need to set the content type to avoid the default of
// text/html, as such a content type would cause some browsers to // text/html, as such a content type would cause some browsers to
@@ -327,7 +348,7 @@ public abstract class GuacamoleHTTPTunnelServlet extends HttpServlet {
session.detachTunnel(tunnel); session.detachTunnel(tunnel);
tunnel.close(); tunnel.close();
throw new GuacamoleException("I/O Error sending data to server: " + e.getMessage(), e); throw new GuacamoleServerException("I/O Error sending data to server: " + e.getMessage(), e);
} }
finally { finally {
tunnel.releaseWriter(); tunnel.releaseWriter();

View File

@@ -41,6 +41,7 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentMap;
import javax.servlet.http.HttpSession; import javax.servlet.http.HttpSession;
import net.sourceforge.guacamole.GuacamoleException; import net.sourceforge.guacamole.GuacamoleException;
import net.sourceforge.guacamole.GuacamoleSecurityException;
import net.sourceforge.guacamole.net.GuacamoleTunnel; import net.sourceforge.guacamole.net.GuacamoleTunnel;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
@@ -71,7 +72,7 @@ public class GuacamoleSession {
public GuacamoleSession(HttpSession session) throws GuacamoleException { public GuacamoleSession(HttpSession session) throws GuacamoleException {
if (session == null) if (session == null)
throw new GuacamoleException("User has no session."); throw new GuacamoleSecurityException("User has no session.");
synchronized (session) { synchronized (session) {