From 5743db87e113a4cd95d7f2c3e0376babb11d193e Mon Sep 17 00:00:00 2001 From: alena Date: Tue, 4 Jan 2011 14:52:33 -0800 Subject: [PATCH] bug 5482: build Error api response based on responseType(xml, json) specified in the request instead of using default HttpServlet error response format status 5482: resolved fixed --- server/src/com/cloud/api/ApiServer.java | 59 ++++++++++++-------- server/src/com/cloud/api/ApiServlet.java | 70 ++++++++++++------------ 2 files changed, 71 insertions(+), 58 deletions(-) diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java index 41c3c5674c3..aeccf9c86ba 100755 --- a/server/src/com/cloud/api/ApiServer.java +++ b/server/src/com/cloud/api/ApiServer.java @@ -155,6 +155,10 @@ public class ApiServer implements HttpRequestHandler { return s_instance; } + public Properties get_apiCommands() { + return _apiCommands; + } + public void init(String[] apiConfig) { try { BaseCmd.setComponents(new ApiResponseHelper()); @@ -273,30 +277,9 @@ public class ApiServer implements HttpRequestHandler { writeResponse(response, responseText, HttpStatus.SC_OK, responseType, null); } catch (ServerApiException se) { - String responseName = null; - String cmdClassName = null; - - try { - if (se.getErrorCode() == BaseCmd.UNSUPPORTED_ACTION_ERROR) { - responseName = "errorresponse"; - } else { - String cmdName = ((String[])parameterMap.get("command"))[0]; - cmdClassName = _apiCommands.getProperty(cmdName); - Class claz = Class.forName(cmdClassName); - responseName = ((BaseCmd)claz.newInstance()).getCommandName(); - } - - ExceptionResponse apiResponse = new ExceptionResponse(); - apiResponse.setErrorCode(se.getErrorCode()); - apiResponse.setErrorText(se.getDescription()); - apiResponse.setResponseName(responseName); - String responseText = ApiResponseSerializer.toSerializedString(apiResponse, responseType); - - writeResponse(response, responseText, se.getErrorCode(), responseType, se.getDescription()); - sb.append(" " + se.getErrorCode() + " " + responseText.length()); - } catch (Exception e) { - s_logger.error("IO Exception responding to http request", e); - } + String responseText = getSerializedApiError(se.getErrorCode(), se.getDescription(), parameterMap, responseType); + writeResponse(response, responseText, se.getErrorCode(), responseType, se.getDescription()); + sb.append(" " +se.getErrorCode() + " " + se.getDescription()); } catch(RuntimeException e) { // log runtime exception like NullPointerException to help identify the source easier s_logger.error("Unhandled exception, ", e); @@ -887,4 +870,32 @@ public class ApiServer implements HttpRequestHandler { } } } + + public String getSerializedApiError(int errorCode, String errorText, Map apiCommandParams, String responseType) { + String responseName = null; + String cmdClassName = null; + + String responseText = null; + + try { + if (errorCode == BaseCmd.UNSUPPORTED_ACTION_ERROR || apiCommandParams == null || apiCommandParams.isEmpty()) { + responseName = "errorresponse"; + } else { + String cmdName = ((String[])apiCommandParams.get("command"))[0]; + cmdClassName = _apiCommands.getProperty(cmdName); + Class claz = Class.forName(cmdClassName); + responseName = ((BaseCmd)claz.newInstance()).getCommandName(); + } + + ExceptionResponse apiResponse = new ExceptionResponse(); + apiResponse.setErrorCode(errorCode); + apiResponse.setErrorText(errorText); + apiResponse.setResponseName(responseName); + responseText = ApiResponseSerializer.toSerializedString(apiResponse, responseType); + + }catch (Exception e) { + s_logger.error("Exception responding to http request", e); + } + return responseText; + } } diff --git a/server/src/com/cloud/api/ApiServlet.java b/server/src/com/cloud/api/ApiServlet.java index f7ab3b658de..b8f19b78d2e 100755 --- a/server/src/com/cloud/api/ApiServlet.java +++ b/server/src/com/cloud/api/ApiServlet.java @@ -19,7 +19,6 @@ package com.cloud.api; import java.io.IOException; -import java.io.OutputStream; import java.security.InvalidParameterException; import java.util.Enumeration; import java.util.HashMap; @@ -81,14 +80,14 @@ public class ApiServlet extends HttpServlet { private void processRequest(HttpServletRequest req, HttpServletResponse resp) { StringBuffer auditTrailSb = new StringBuffer(); auditTrailSb.append(" " +req.getRemoteAddr()); - auditTrailSb.append(" -- " + req.getMethod() + " " ); + auditTrailSb.append(" -- " + req.getMethod() + " " ); + // get the response format since we'll need it in a couple of places + String responseType = BaseCmd.RESPONSE_TYPE_XML; + Map params = new HashMap(); + params.putAll(req.getParameterMap()); + try { - Map params = new HashMap(); - params.putAll(req.getParameterMap()); HttpSession session = req.getSession(false); - - // get the response format since we'll need it in a couple of places - String responseType = BaseCmd.RESPONSE_TYPE_XML; Object[] responseTypeParam = params.get("response"); if (responseTypeParam != null) { responseType = (String)responseTypeParam[0]; @@ -117,8 +116,8 @@ public class ApiServlet extends HttpServlet { }catch (IllegalStateException ise) {} } auditTrailSb.append("command=logout"); - auditTrailSb.append(" " +HttpServletResponse.SC_OK); - writeResponse(resp, getLogoutSuccessResponse(responseType), false, responseType); + auditTrailSb.append(" " + HttpServletResponse.SC_OK); + writeResponse(resp, getLogoutSuccessResponse(responseType), HttpServletResponse.SC_OK, responseType); return; } else if ("login".equalsIgnoreCase(command)) { auditTrailSb.append("command=login"); @@ -147,7 +146,8 @@ public class ApiServlet extends HttpServlet { { s_logger.warn("Invalid domain id entered by user"); auditTrailSb.append(" " + HttpServletResponse.SC_UNAUTHORIZED + " " + "Invalid domain id entered, please enter a valid one"); - resp.sendError(HttpServletResponse.SC_UNAUTHORIZED,"Invalid domain id entered, please enter a valid one"); + String serializedResponse = _apiServer.getSerializedApiError(HttpServletResponse.SC_UNAUTHORIZED, "Invalid domain id entered, please enter a valid one", params, responseType); + writeResponse(resp, serializedResponse, HttpServletResponse.SC_UNAUTHORIZED, responseType); } } String domain = null; @@ -173,15 +173,17 @@ public class ApiServlet extends HttpServlet { " accountId="+ ((Account)session.getAttribute("accountobj")).getId()+ " sessionId="+session.getId()+ ")" ); String loginResponse = getLoginSuccessResponse(session, responseType); - writeResponse(resp, loginResponse, false, responseType); + writeResponse(resp, loginResponse, HttpServletResponse.SC_OK, responseType); return; } catch (CloudAuthenticationException ex) { // TODO: fall through to API key, or just fail here w/ auth error? (HTTP 401) try { session.invalidate(); }catch (IllegalStateException ise) {} + auditTrailSb.append(" " + BaseCmd.ACCOUNT_ERROR + " " + ex.getMessage() != null ? ex.getMessage() : "failed to authenticate user, check if username/password are correct"); - resp.sendError(BaseCmd.ACCOUNT_ERROR, ex.getMessage() != null ? ex.getMessage() : "failed to authenticate user, check if username/password are correct"); + String serializedResponse = _apiServer.getSerializedApiError(BaseCmd.ACCOUNT_ERROR, ex.getMessage() != null ? ex.getMessage() : "failed to authenticate user, check if username/password are correct", params, responseType); + writeResponse(resp, serializedResponse, BaseCmd.ACCOUNT_ERROR, responseType); return; } } @@ -208,7 +210,8 @@ public class ApiServlet extends HttpServlet { session.invalidate(); }catch (IllegalStateException ise) {} auditTrailSb.append(" " + HttpServletResponse.SC_UNAUTHORIZED + " " + "unable to verify user credentials"); - resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "unable to verify user credentials"); + String serializedResponse = _apiServer.getSerializedApiError(HttpServletResponse.SC_UNAUTHORIZED, "unable to verify user credentials", params, responseType); + writeResponse(resp, serializedResponse, HttpServletResponse.SC_UNAUTHORIZED, responseType); return; } @@ -217,8 +220,9 @@ public class ApiServlet extends HttpServlet { String[] command = (String[])params.get("command"); if (command == null) { s_logger.info("missing command, ignoring request..."); - auditTrailSb.append(" " + HttpServletResponse.SC_BAD_REQUEST + " " + "no command specified"); - resp.sendError(HttpServletResponse.SC_BAD_REQUEST, "no command specified"); + auditTrailSb.append(" " + HttpServletResponse.SC_BAD_REQUEST + " " + "no command specified"); + String serializedResponse = _apiServer.getSerializedApiError(HttpServletResponse.SC_BAD_REQUEST, "no command specified", params, responseType); + writeResponse(resp, serializedResponse, HttpServletResponse.SC_BAD_REQUEST, responseType); return; } UserContext.updateContext(userId, (Account)accountObj, session.getId()); @@ -228,8 +232,10 @@ public class ApiServlet extends HttpServlet { try { session.invalidate(); }catch (IllegalStateException ise) {} + auditTrailSb.append(" " + HttpServletResponse.SC_UNAUTHORIZED + " " + "unable to verify user credentials"); - resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "unable to verify user credentials"); + String serializedResponse = _apiServer.getSerializedApiError(HttpServletResponse.SC_UNAUTHORIZED, "unable to verify user credentials", params, responseType); + writeResponse(resp, serializedResponse, HttpServletResponse.SC_UNAUTHORIZED, responseType); return; } } @@ -258,10 +264,11 @@ public class ApiServlet extends HttpServlet { try { String response = _apiServer.handleRequest(params, true, responseType, auditTrailSb); - writeResponse(resp, response != null ? response : "", false, responseType); + writeResponse(resp, response != null ? response : "", HttpServletResponse.SC_OK, responseType); } catch (ServerApiException se) { + String serializedResponseText = _apiServer.getSerializedApiError(se.getErrorCode(), se.getDescription(), params, responseType); + writeResponse(resp, serializedResponseText, se.getErrorCode(), responseType); auditTrailSb.append(" " +se.getErrorCode() + " " + se.getDescription()); - resp.sendError(se.getErrorCode(), se.getDescription()); } } else { if (session != null) { @@ -269,22 +276,16 @@ public class ApiServlet extends HttpServlet { session.invalidate(); }catch (IllegalStateException ise) {} } + auditTrailSb.append(" " + HttpServletResponse.SC_UNAUTHORIZED + " " + "unable to verify user credentials and/or request signature"); - resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "unable to verify user credentials and/or request signature"); + String serializedResponse = _apiServer.getSerializedApiError(HttpServletResponse.SC_UNAUTHORIZED, "unable to verify user credentials and/or request signature", params, responseType); + writeResponse(resp, serializedResponse, HttpServletResponse.SC_UNAUTHORIZED, responseType); + } - } catch (IOException ioex) { - if (s_logger.isTraceEnabled()) { - s_logger.trace("exception processing request: " + ioex); - } - auditTrailSb.append(" exception processing request" ); }catch (InvalidParameterException ipe){ auditTrailSb.append(" " + HttpServletResponse.SC_NOT_FOUND + " " + ipe.getMessage()); - try { - resp.sendError(HttpServletResponse.SC_NOT_FOUND, ipe.getMessage()); - } catch (IOException e) { - s_logger.error("Unable to send back error response for invalid command"); - auditTrailSb.append(" " + HttpServletResponse.SC_NOT_FOUND + " " + "Unable to send back error response for "+ipe.getMessage()); - } + String serializedResponse = _apiServer.getSerializedApiError(HttpServletResponse.SC_NOT_FOUND, ipe.getMessage(), params, responseType); + writeResponse(resp, serializedResponse, HttpServletResponse.SC_NOT_FOUND, responseType); }catch (Exception ex) { s_logger.error("unknown exception writing api response", ex); auditTrailSb.append(" unknown exception writing api response"); @@ -312,16 +313,16 @@ public class ApiServlet extends HttpServlet { */ // FIXME: rather than isError, we might was to pass in the status code to give more flexibility - private void writeResponse(HttpServletResponse resp, String response, boolean isError, String responseType) { + private void writeResponse(HttpServletResponse resp, String response, int responseCode, String responseType) { try { // is text/plain sufficient for XML and JSON? if (BaseCmd.RESPONSE_TYPE_JSON.equalsIgnoreCase(responseType)) { resp.setContentType("text/javascript"); } else { resp.setContentType("text/xml"); - } - resp.setStatus(isError? HttpServletResponse.SC_INTERNAL_SERVER_ERROR : HttpServletResponse.SC_OK); + } + resp.setStatus(responseCode); // use getWriter() instead of manually manipulate encoding to have better localization support resp.getWriter().print(response); } catch (IOException ioex) { @@ -379,5 +380,6 @@ public class ApiServlet extends HttpServlet { sb.append("success"); } return sb.toString(); - } + } + }