From b2fde8f0dfe2d60b08724e92f919c1f68223101f Mon Sep 17 00:00:00 2001 From: James Moger Date: Mon, 17 Oct 2011 17:50:09 -0400 Subject: [PATCH] Consistent rpc result codes. --- src/com/gitblit/Constants.java | 2 +- src/com/gitblit/GitBlitException.java | 13 ++++++ src/com/gitblit/JsonServlet.java | 6 +++ src/com/gitblit/RpcFilter.java | 4 ++ src/com/gitblit/RpcServlet.java | 40 ++++++++++++++----- .../client/GitblitManagerLauncher.java | 2 +- src/com/gitblit/utils/JsonUtils.java | 4 ++ 7 files changed, 58 insertions(+), 13 deletions(-) diff --git a/src/com/gitblit/Constants.java b/src/com/gitblit/Constants.java index 6300fa2e..d6495e6f 100644 --- a/src/com/gitblit/Constants.java +++ b/src/com/gitblit/Constants.java @@ -212,7 +212,7 @@ public class Constants { return type; } } - return LIST_REPOSITORIES; + return null; } public boolean exceeds(RpcRequest type) { diff --git a/src/com/gitblit/GitBlitException.java b/src/com/gitblit/GitBlitException.java index af32003a..1111463f 100644 --- a/src/com/gitblit/GitBlitException.java +++ b/src/com/gitblit/GitBlitException.java @@ -56,4 +56,17 @@ public class GitBlitException extends IOException { super(message); } } + + /** + * Exception to indicate that the requested action can not be executed by + * the server because it does not recognize the request type. + */ + public static class UnknownRequestException extends GitBlitException { + + private static final long serialVersionUID = 1L; + + public UnknownRequestException(String message) { + super(message); + } + } } diff --git a/src/com/gitblit/JsonServlet.java b/src/com/gitblit/JsonServlet.java index a7958969..5433a61c 100644 --- a/src/com/gitblit/JsonServlet.java +++ b/src/com/gitblit/JsonServlet.java @@ -41,6 +41,12 @@ public abstract class JsonServlet extends HttpServlet { private static final long serialVersionUID = 1L; + protected final int forbiddenCode = HttpServletResponse.SC_FORBIDDEN; + + protected final int notAllowedCode = HttpServletResponse.SC_METHOD_NOT_ALLOWED; + + protected final int failureCode = HttpServletResponse.SC_INTERNAL_SERVER_ERROR; + protected final Logger logger; public JsonServlet() { diff --git a/src/com/gitblit/RpcFilter.java b/src/com/gitblit/RpcFilter.java index f92dd962..2786f2a4 100644 --- a/src/com/gitblit/RpcFilter.java +++ b/src/com/gitblit/RpcFilter.java @@ -59,6 +59,10 @@ public class RpcFilter extends AuthenticationFilter { String fullUrl = getFullUrl(httpRequest); RpcRequest requestType = RpcRequest.fromName(httpRequest.getParameter("req")); + if (requestType == null) { + httpResponse.sendError(HttpServletResponse.SC_NOT_IMPLEMENTED); + return; + } boolean adminRequest = requestType.exceeds(RpcRequest.LIST_REPOSITORIES); diff --git a/src/com/gitblit/RpcServlet.java b/src/com/gitblit/RpcServlet.java index c366a187..53426dad 100644 --- a/src/com/gitblit/RpcServlet.java +++ b/src/com/gitblit/RpcServlet.java @@ -95,7 +95,11 @@ public class RpcServlet extends JsonServlet { } else if (RpcRequest.CREATE_REPOSITORY.equals(reqType)) { // create repository RepositoryModel model = deserialize(request, response, RepositoryModel.class); - GitBlit.self().updateRepositoryModel(model.name, model, true); + try { + GitBlit.self().updateRepositoryModel(model.name, model, true); + } catch (GitBlitException e) { + response.setStatus(failureCode); + } } else if (RpcRequest.EDIT_REPOSITORY.equals(reqType)) { // edit repository RepositoryModel model = deserialize(request, response, RepositoryModel.class); @@ -104,7 +108,11 @@ public class RpcServlet extends JsonServlet { if (repoName == null) { repoName = model.name; } - GitBlit.self().updateRepositoryModel(repoName, model, false); + try { + GitBlit.self().updateRepositoryModel(repoName, model, false); + } catch (GitBlitException e) { + response.setStatus(failureCode); + } } else if (RpcRequest.DELETE_REPOSITORY.equals(reqType)) { // delete repository RepositoryModel model = deserialize(request, response, RepositoryModel.class); @@ -112,7 +120,11 @@ public class RpcServlet extends JsonServlet { } else if (RpcRequest.CREATE_USER.equals(reqType)) { // create user UserModel model = deserialize(request, response, UserModel.class); - GitBlit.self().updateUserModel(model.username, model, true); + try { + GitBlit.self().updateUserModel(model.username, model, true); + } catch (GitBlitException e) { + response.setStatus(failureCode); + } } else if (RpcRequest.EDIT_USER.equals(reqType)) { // edit user UserModel model = deserialize(request, response, UserModel.class); @@ -121,11 +133,17 @@ public class RpcServlet extends JsonServlet { if (username == null) { username = model.username; } - GitBlit.self().updateUserModel(username, model, false); + try { + GitBlit.self().updateUserModel(username, model, false); + } catch (GitBlitException e) { + response.setStatus(failureCode); + } } else if (RpcRequest.DELETE_USER.equals(reqType)) { // delete user UserModel model = deserialize(request, response, UserModel.class); - GitBlit.self().deleteUser(model.username); + if (!GitBlit.self().deleteUser(model.username)) { + response.setStatus(failureCode); + } } else if (RpcRequest.LIST_REPOSITORY_MEMBERS.equals(reqType)) { // get repository members RepositoryModel model = GitBlit.self().getRepositoryModel(objectName); @@ -136,7 +154,7 @@ public class RpcServlet extends JsonServlet { Collection names = deserialize(request, response, RpcUtils.NAMES_TYPE); List users = new ArrayList(names); if (!GitBlit.self().setRepositoryUsers(model, users)) { - response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + response.setStatus(failureCode); } } else if (RpcRequest.LIST_FEDERATION_REGISTRATIONS.equals(reqType)) { // return the list of federation registrations @@ -146,14 +164,14 @@ public class RpcServlet extends JsonServlet { if (GitBlit.canFederate()) { result = GitBlit.self().getFederationResultRegistrations(); } else { - response.sendError(HttpServletResponse.SC_FORBIDDEN); + response.sendError(notAllowedCode); } } else if (RpcRequest.LIST_FEDERATION_PROPOSALS.equals(reqType)) { // return the list of federation proposals if (GitBlit.canFederate()) { result = GitBlit.self().getPendingFederationProposals(); } else { - response.sendError(HttpServletResponse.SC_FORBIDDEN); + response.sendError(notAllowedCode); } } else if (RpcRequest.LIST_FEDERATION_SETS.equals(reqType)) { // return the list of federation sets @@ -161,13 +179,13 @@ public class RpcServlet extends JsonServlet { String gitblitUrl = HttpUtils.getGitblitURL(request); result = GitBlit.self().getFederationSets(gitblitUrl); } else { - response.sendError(HttpServletResponse.SC_FORBIDDEN); + response.sendError(notAllowedCode); } } else if (RpcRequest.LIST_SETTINGS.equals(reqType)) { // return the server's settings - Properties settings = new Properties(); + Properties settings = new Properties(); List keys = GitBlit.getAllKeys(null); - for (String key:keys) { + for (String key : keys) { String value = GitBlit.getString(key, null); if (value != null) { settings.put(key, value); diff --git a/src/com/gitblit/client/GitblitManagerLauncher.java b/src/com/gitblit/client/GitblitManagerLauncher.java index 1d4670d6..9b6ee96b 100644 --- a/src/com/gitblit/client/GitblitManagerLauncher.java +++ b/src/com/gitblit/client/GitblitManagerLauncher.java @@ -44,7 +44,7 @@ public class GitblitManagerLauncher { DownloadListener downloadListener = new DownloadListener() { @Override public void downloading(String name) { - updateSplash(splash, Translation.get("gb.downloading") + " " + name + "..."); + updateSplash(splash, Translation.get("gb.downloading") + " " + name); } }; diff --git a/src/com/gitblit/utils/JsonUtils.java b/src/com/gitblit/utils/JsonUtils.java index fee79901..0c78df95 100644 --- a/src/com/gitblit/utils/JsonUtils.java +++ b/src/com/gitblit/utils/JsonUtils.java @@ -47,6 +47,7 @@ import org.eclipse.jgit.util.Base64; import com.gitblit.GitBlitException.ForbiddenException; import com.gitblit.GitBlitException.UnauthorizedException; +import com.gitblit.GitBlitException.UnknownRequestException; import com.gitblit.models.RepositoryModel; import com.gitblit.models.UserModel; import com.google.gson.Gson; @@ -277,6 +278,9 @@ public class JsonUtils { } else if (e.getMessage().indexOf("403") > -1) { // requested url is forbidden by the requesting user throw new ForbiddenException(url); + } else if (e.getMessage().indexOf("501") > -1) { + // requested url is not recognized by the server + throw new UnknownRequestException(url); } throw e; } -- 2.39.5