Improve error handling with universal, proper HTTP codes.

guacamole/src/main/java/net/sourceforge/guacamole/net/basic/AuthenticatingHttpServlet.java

@@ -26,7 +26,10 @@ import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
+import net.sourceforge.guacamole.GuacamoleClientException;
 import net.sourceforge.guacamole.GuacamoleException;
+import net.sourceforge.guacamole.GuacamoleResourceNotFoundException;
+import net.sourceforge.guacamole.GuacamoleSecurityException;
 import net.sourceforge.guacamole.net.auth.AuthenticationProvider;
 import net.sourceforge.guacamole.net.auth.Credentials;
 import net.sourceforge.guacamole.net.auth.UserContext;
@@ -166,6 +169,40 @@ public abstract class AuthenticatingHttpServlet extends HttpServlet {
         response.sendError(HttpServletResponse.SC_FORBIDDEN);
     }
 
+    /**
+     * Sends an error on the given HTTP response with the given integer error
+     * code.
+     *
+     * @param response The HTTP response to use to send the error.
+     * @param code The HTTP status code of the error.
+     * @param message A human-readable message that can be presented to the
+     *                user.
+     * @throws ServletException If an error prevents sending of the error
+     *                          code.
+     */
+    private void sendError(HttpServletResponse response, int code,
+            String message) throws ServletException {
+
+        try {
+
+            // If response not committed, send error code
+            if (!response.isCommitted()) {
+                response.sendError(code);
+                response.addHeader("Guacamole-Error-Message", message);
+            }
+
+        }
+        catch (IOException ioe) {
+
+            // If unable to send error at all due to I/O problems,
+            // rethrow as servlet exception
+            throw new ServletException(ioe);
+
+        }
+
+    }
+
+
     /**
      * Returns the credentials associated with the given session.
      *
@@ -281,11 +318,38 @@ public abstract class AuthenticatingHttpServlet extends HttpServlet {
 
         }
 
+        try {
+
             // Allow servlet to run now that authentication has been validated
             authenticatedService(context, request, response);
 
         }
 
+        // Catch any thrown guacamole exception and attempt to pass within the
+        // HTTP response, logging each error appropriately.
+        catch (GuacamoleSecurityException e) {
+            logger.warn("Permission denied: {}", e.getMessage());
+            sendError(response, HttpServletResponse.SC_FORBIDDEN,
+                    "Permission denied.");
+        }
+        catch (GuacamoleResourceNotFoundException e) {
+            logger.debug("Resource not found: {}", e.getMessage());
+            sendError(response, HttpServletResponse.SC_NOT_FOUND,
+                    e.getMessage());
+        }
+        catch (GuacamoleClientException e) {
+            logger.warn("Error in client request: {}", e.getMessage());
+            sendError(response, HttpServletResponse.SC_BAD_REQUEST,
+                    e.getMessage());
+        }
+        catch (GuacamoleException e) {
+            logger.error("Internal server error.", e);
+            sendError(response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+                      "Internal server error.");
+        }
+
+    }
+
     /**
      * Function called after the credentials given in the request (if any)
      * are authenticated. If the current session is not associated with
@@ -296,14 +360,12 @@ public abstract class AuthenticatingHttpServlet extends HttpServlet {
      * @param response An HttpServletResponse which controls the HTTP response
      *                 of this servlet.
      *
-     * @throws ServletException If an error occurs that interferes with the
+     * @throws GuacamoleException If an error occurs that interferes with the
      *                            normal operation of this servlet.
-     * @throws IOException If an error occurs that prevents this servlet from
-     *                     communicating.
      */
     protected abstract void authenticatedService(
             UserContext context,
             HttpServletRequest request, HttpServletResponse response)
-            throws ServletException, IOException;
+            throws GuacamoleException;
 
 }

guacamole/src/main/java/net/sourceforge/guacamole/net/basic/BasicGuacamoleTunnelServlet.java

@@ -60,10 +60,23 @@ public class BasicGuacamoleTunnelServlet extends AuthenticatingHttpServlet {
     protected void authenticatedService(
             UserContext context,
             HttpServletRequest request, HttpServletResponse response)
-    throws IOException, ServletException {
+    throws GuacamoleException {
+
+        try {
 
             // If authenticated, respond as tunnel
             tunnelServlet.service(request, response);
+        }
+
+        catch (ServletException e) {
+            logger.info("Error from tunnel (see previous log messages): {}",
+                    e.getMessage());
+        }
+
+        catch (IOException e) {
+            logger.info("I/O error from tunnel (see previous log messages): {}",
+                    e.getMessage());
+        }
 
     }
 

guacamole/src/main/java/net/sourceforge/guacamole/net/basic/BasicLogin.java

@@ -18,7 +18,6 @@ package net.sourceforge.guacamole.net.basic;
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import java.io.IOException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import net.sourceforge.guacamole.net.auth.UserContext;
@@ -41,8 +40,7 @@ public class BasicLogin extends AuthenticatingHttpServlet {
     @Override
     protected void authenticatedService(
             UserContext context,
-            HttpServletRequest request, HttpServletResponse response)
+            HttpServletRequest request, HttpServletResponse response) {
-    throws IOException {
         logger.info("Login was successful.");
     }
 

guacamole/src/main/java/net/sourceforge/guacamole/net/basic/crud/connections/Create.java

@@ -18,9 +18,7 @@ package net.sourceforge.guacamole.net.basic.crud.connections;
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import java.io.IOException;
 import java.util.Enumeration;
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import net.sourceforge.guacamole.GuacamoleException;
@@ -47,14 +45,12 @@ public class Create extends AuthenticatingHttpServlet {
     protected void authenticatedService(
             UserContext context,
             HttpServletRequest request, HttpServletResponse response)
-    throws IOException, ServletException {
+    throws GuacamoleException {
 
         // Get ID and protocol
         String identifier = request.getParameter("id");
         String protocol = request.getParameter("protocol");
 
-        try {
-
         // Attempt to get connection directory
         Directory<String, Connection> directory =
                 context.getConnectionDirectory();
@@ -86,11 +82,6 @@ public class Create extends AuthenticatingHttpServlet {
         directory.add(connection);
 
     }
-        catch (GuacamoleException e) {
-            throw new ServletException("Unable to create connection.", e);
-        }
-
-    }
 
 }
 

guacamole/src/main/java/net/sourceforge/guacamole/net/basic/crud/connections/Delete.java

@@ -18,8 +18,6 @@ package net.sourceforge.guacamole.net.basic.crud.connections;
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import java.io.IOException;
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import net.sourceforge.guacamole.GuacamoleException;
@@ -39,13 +37,11 @@ public class Delete extends AuthenticatingHttpServlet {
     protected void authenticatedService(
             UserContext context,
             HttpServletRequest request, HttpServletResponse response)
-    throws IOException, ServletException {
+    throws GuacamoleException {
 
         // Get ID
         String identifier = request.getParameter("id");
 
-        try {
-
         // Attempt to get connection directory
         Directory<String, Connection> directory =
                 context.getConnectionDirectory();
@@ -53,10 +49,6 @@ public class Delete extends AuthenticatingHttpServlet {
         // Remove connection
         directory.remove(identifier);
 
-        }
-        catch (GuacamoleException e) {
-            throw new ServletException("Unable to remove connection.", e);
-        }
 
     }
 

guacamole/src/main/java/net/sourceforge/guacamole/net/basic/crud/connections/List.java

@@ -19,7 +19,6 @@ package net.sourceforge.guacamole.net.basic.crud.connections;
  */
 
 import java.io.IOException;
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.xml.stream.XMLOutputFactory;
@@ -27,6 +26,7 @@ import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.XMLStreamWriter;
 import net.sourceforge.guacamole.GuacamoleException;
 import net.sourceforge.guacamole.GuacamoleSecurityException;
+import net.sourceforge.guacamole.GuacamoleServerException;
 import net.sourceforge.guacamole.net.auth.Connection;
 import net.sourceforge.guacamole.net.auth.ConnectionRecord;
 import net.sourceforge.guacamole.net.auth.Directory;
@@ -84,7 +84,7 @@ public class List extends AuthenticatingHttpServlet {
     protected void authenticatedService(
             UserContext context,
             HttpServletRequest request, HttpServletResponse response)
-    throws IOException, ServletException {
+    throws GuacamoleException {
 
         // Do not cache
         response.setHeader("Cache-Control", "no-cache");
@@ -92,17 +92,8 @@ public class List extends AuthenticatingHttpServlet {
         // Write XML content type
         response.setHeader("Content-Type", "text/xml");
 
-        // Attempt to get connections
-        Directory<String, Connection> directory;
-        try {
-
         // Get connection directory
-            directory = context.getConnectionDirectory();
+        Directory<String, Connection> directory = context.getConnectionDirectory();
-
-        }
-        catch (GuacamoleException e) {
-            throw new ServletException("Unable to retrieve connections.", e);
-        }
 
         // Write actual XML
         try {
@@ -180,10 +171,12 @@ public class List extends AuthenticatingHttpServlet {
 
         }
         catch (XMLStreamException e) {
-            throw new IOException("Unable to write configuration list XML.", e);
+            throw new GuacamoleServerException(
+                    "Unable to write configuration list XML.", e);
         }
-        catch (GuacamoleException e) {
+        catch (IOException e) {
-            throw new ServletException("Unable to read configurations.", e);
+            throw new GuacamoleServerException(
+                    "I/O error writing configuration list XML.", e);
         }
 
     }

guacamole/src/main/java/net/sourceforge/guacamole/net/basic/crud/connections/Update.java

@@ -47,14 +47,12 @@ public class Update extends AuthenticatingHttpServlet {
     protected void authenticatedService(
             UserContext context,
             HttpServletRequest request, HttpServletResponse response)
-    throws IOException, ServletException {
+    throws GuacamoleException {
 
         // Get ID and protocol
         String identifier = request.getParameter("id");
         String protocol = request.getParameter("protocol");
 
-        try {
-
         // Attempt to get connection directory
         Directory<String, Connection> directory =
                 context.getConnectionDirectory();
@@ -85,11 +83,6 @@ public class Update extends AuthenticatingHttpServlet {
         directory.update(connection);
 
     }
-        catch (GuacamoleException e) {
-            throw new ServletException("Unable to update connection.", e);
-        }
-
-    }
 
 }
 

guacamole/src/main/java/net/sourceforge/guacamole/net/basic/crud/permissions/List.java

@@ -19,14 +19,15 @@ package net.sourceforge.guacamole.net.basic.crud.permissions;
  */
 
 import java.io.IOException;
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.xml.stream.XMLOutputFactory;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.XMLStreamWriter;
+import net.sourceforge.guacamole.GuacamoleClientException;
 import net.sourceforge.guacamole.GuacamoleException;
 import net.sourceforge.guacamole.GuacamoleSecurityException;
+import net.sourceforge.guacamole.GuacamoleServerException;
 import net.sourceforge.guacamole.net.auth.Directory;
 import net.sourceforge.guacamole.net.auth.User;
 import net.sourceforge.guacamole.net.auth.UserContext;
@@ -96,7 +97,7 @@ public class List extends AuthenticatingHttpServlet {
     protected void authenticatedService(
             UserContext context,
             HttpServletRequest request, HttpServletResponse response)
-    throws IOException, ServletException {
+    throws GuacamoleException {
 
         // Do not cache
         response.setHeader("Cache-Control", "no-cache");
@@ -176,7 +177,8 @@ public class List extends AuthenticatingHttpServlet {
                 }
 
                 else
-                    throw new ServletException("Unsupported permission type.");
+                    throw new GuacamoleClientException(
+                            "Unsupported permission type.");
 
             }
 
@@ -186,17 +188,12 @@ public class List extends AuthenticatingHttpServlet {
 
         }
         catch (XMLStreamException e) {
-            throw new IOException("Unable to write permission list XML.", e);
+            throw new GuacamoleServerException(
+                    "Unable to write permission list XML.", e);
         }
-        catch (GuacamoleSecurityException e) {
+        catch (IOException e) {
-
+            throw new GuacamoleServerException(
-            // If cannot read permissions, return error
+                    "I/O error writing permission list XML.", e);
-            response.sendError(HttpServletResponse.SC_FORBIDDEN,
-                    "Permission denied.");
-
-        }
-        catch (GuacamoleException e) {
-            throw new ServletException("Unable to read permissions.", e);
         }
 
     }

guacamole/src/main/java/net/sourceforge/guacamole/net/basic/crud/users/Create.java

@@ -18,9 +18,7 @@ package net.sourceforge.guacamole.net.basic.crud.users;
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import java.io.IOException;
 import java.util.UUID;
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import net.sourceforge.guacamole.GuacamoleException;
@@ -40,13 +38,11 @@ public class Create extends AuthenticatingHttpServlet {
     protected void authenticatedService(
             UserContext context,
             HttpServletRequest request, HttpServletResponse response)
-    throws IOException, ServletException {
+    throws GuacamoleException {
 
         // Create user as specified
         String username = request.getParameter("name");
 
-        try {
-
         // Attempt to get user directory
         Directory<String, User> directory =
                 context.getUserDirectory();
@@ -60,12 +56,6 @@ public class Create extends AuthenticatingHttpServlet {
         directory.add(user);
 
     }
-        catch (GuacamoleException e) {
-            throw new ServletException("Unable to create user.", e);
-        }
-
-
-    }
 
 }
 

guacamole/src/main/java/net/sourceforge/guacamole/net/basic/crud/users/Delete.java

@@ -18,8 +18,6 @@ package net.sourceforge.guacamole.net.basic.crud.users;
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import java.io.IOException;
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import net.sourceforge.guacamole.GuacamoleException;
@@ -39,13 +37,11 @@ public class Delete extends AuthenticatingHttpServlet {
     protected void authenticatedService(
             UserContext context,
             HttpServletRequest request, HttpServletResponse response)
-    throws IOException, ServletException {
+    throws GuacamoleException {
 
         // Get username
         String username = request.getParameter("name");
 
-        try {
-
         // Attempt to get user directory
         Directory<String, User> directory = context.getUserDirectory();
 
@@ -53,11 +49,6 @@ public class Delete extends AuthenticatingHttpServlet {
         directory.remove(username);
 
     }
-        catch (GuacamoleException e) {
-            throw new ServletException("Unable to remove user.", e);
-        }
-
-    }
 
 }
 

guacamole/src/main/java/net/sourceforge/guacamole/net/basic/crud/users/List.java

@@ -20,14 +20,13 @@ package net.sourceforge.guacamole.net.basic.crud.users;
 
 import java.io.IOException;
 import java.util.Set;
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.xml.stream.XMLOutputFactory;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.XMLStreamWriter;
 import net.sourceforge.guacamole.GuacamoleException;
-import net.sourceforge.guacamole.GuacamoleSecurityException;
+import net.sourceforge.guacamole.GuacamoleServerException;
 import net.sourceforge.guacamole.net.auth.Directory;
 import net.sourceforge.guacamole.net.auth.User;
 import net.sourceforge.guacamole.net.auth.UserContext;
@@ -44,7 +43,7 @@ public class List extends AuthenticatingHttpServlet {
     protected void authenticatedService(
             UserContext context,
             HttpServletRequest request, HttpServletResponse response)
-    throws IOException, ServletException {
+    throws GuacamoleException {
 
         // Do not cache
         response.setHeader("Cache-Control", "no-cache");
@@ -86,17 +85,12 @@ public class List extends AuthenticatingHttpServlet {
 
         }
         catch (XMLStreamException e) {
-            throw new IOException("Unable to write user list XML.", e);
+            throw new GuacamoleServerException(
+                    "Unable to write configuration list XML.", e);
         }
-        catch (GuacamoleSecurityException e) {
+        catch (IOException e) {
-
+            throw new GuacamoleServerException(
-            // If cannot read permissions, return error
+                    "I/O error writing configuration list XML.", e);
-            response.sendError(HttpServletResponse.SC_FORBIDDEN,
-                    "Permission denied.");
-
-        }
-        catch (GuacamoleException e) {
-            throw new ServletException("Unable to read users.", e);
         }
 
     }

guacamole/src/main/java/net/sourceforge/guacamole/net/basic/crud/users/Update.java

@@ -18,10 +18,9 @@ package net.sourceforge.guacamole.net.basic.crud.users;
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import java.io.IOException;
-import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import net.sourceforge.guacamole.GuacamoleClientException;
 import net.sourceforge.guacamole.GuacamoleException;
 import net.sourceforge.guacamole.net.auth.Directory;
 import net.sourceforge.guacamole.net.auth.User;
@@ -166,7 +165,7 @@ public class Update extends AuthenticatingHttpServlet {
             return new ConnectionPermission(ObjectPermission.Type.ADMINISTER,
                     str.substring(ADMIN_PREFIX.length()));
 
-        throw new GuacamoleException("Invalid permission string.");
+        throw new GuacamoleClientException("Invalid permission string.");
 
     }
 
@@ -174,14 +173,12 @@ public class Update extends AuthenticatingHttpServlet {
     protected void authenticatedService(
             UserContext context,
             HttpServletRequest request, HttpServletResponse response)
-    throws IOException, ServletException {
+    throws GuacamoleException {
 
         // Create user as specified
         String username = request.getParameter("name");
         String password = request.getParameter("password");
 
-        try {
-
         // Attempt to get user directory
         Directory<String, User> directory =
                 context.getUserDirectory();
@@ -246,11 +243,6 @@ public class Update extends AuthenticatingHttpServlet {
         directory.update(user);
 
     }
-        catch (GuacamoleException e) {
-            throw new ServletException("Unable to update user.", e);
-        }
-
-    }
 
 }