From 587cff12e4469a30db6ead191325a264f75bc9de Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Mon, 31 Oct 2016 13:26:31 +0300 Subject: [PATCH] Use UUID instead of int for rebase editor handler The handler is used to communicate between the rebase editor process and main IDEA process. It is more safe to use UUID to prevent malicious code to access IDEA process through XML RPC (although int was chosen randomly). It is also more consistent with ssh/http handlers, which use UUIDs since 63ad820. --- .../GitInteractiveRebaseEditorHandler.java | 6 ++- .../git4idea/rebase/GitRebaseEditorMain.java | 16 ++----- .../rebase/GitRebaseEditorService.java | 44 ++++++------------- .../src/git4idea/rebase/GitRebaser.java | 4 +- 4 files changed, 24 insertions(+), 46 deletions(-) diff --git a/plugins/git4idea/src/git4idea/rebase/GitInteractiveRebaseEditorHandler.java b/plugins/git4idea/src/git4idea/rebase/GitInteractiveRebaseEditorHandler.java index b7fbe0fcd6ea..954f25d61ad0 100644 --- a/plugins/git4idea/src/git4idea/rebase/GitInteractiveRebaseEditorHandler.java +++ b/plugins/git4idea/src/git4idea/rebase/GitInteractiveRebaseEditorHandler.java @@ -28,6 +28,7 @@ import org.jetbrains.annotations.NotNull; import java.io.Closeable; import java.util.List; +import java.util.UUID; /** * The handler for rebase editor request. The handler shows {@link git4idea.rebase.GitRebaseEditor} @@ -58,7 +59,7 @@ public class GitInteractiveRebaseEditorHandler implements Closeable, GitRebaseEd /** * The handler number */ - private final int myHandlerNo; + @NotNull private final UUID myHandlerNo; /** * If true, the handler has been closed */ @@ -197,7 +198,8 @@ public class GitInteractiveRebaseEditorHandler implements Closeable, GitRebaseEd /** * @return the handler number */ - public int getHandlerNo() { + @NotNull + public UUID getHandlerNo() { return myHandlerNo; } diff --git a/plugins/git4idea/src/git4idea/rebase/GitRebaseEditorMain.java b/plugins/git4idea/src/git4idea/rebase/GitRebaseEditorMain.java index 1a84937afc59..0a37edd97c88 100644 --- a/plugins/git4idea/src/git4idea/rebase/GitRebaseEditorMain.java +++ b/plugins/git4idea/src/git4idea/rebase/GitRebaseEditorMain.java @@ -74,25 +74,17 @@ public class GitRebaseEditorMain { System.exit(ERROR_EXIT_CODE); return; } - final String handlerValue = System.getenv(IDEA_REBASE_HANDER_NO); - if (handlerValue == null) { + String handlerId = System.getenv(IDEA_REBASE_HANDER_NO); + if (handlerId == null) { System.err.println("Handler no is not specified"); System.exit(ERROR_EXIT_CODE); } - int handler; - try { - handler = Integer.parseInt(handlerValue); - } - catch (NumberFormatException ex) { - System.err.println("Invalid handler number: " + handlerValue); - System.exit(ERROR_EXIT_CODE); - return; - } + String file = args[1]; try { XmlRpcClientLite client = new XmlRpcClientLite("127.0.0.1", port); Vector params = new Vector<>(); - params.add(handler); + params.add(handlerId); if (System.getProperty("os.name").toLowerCase().startsWith("windows") && file.startsWith(CYGDRIVE_PREFIX)) { int p = CYGDRIVE_PREFIX.length(); file = file.substring(p, p + 1) + ":" + file.substring(p + 1); diff --git a/plugins/git4idea/src/git4idea/rebase/GitRebaseEditorService.java b/plugins/git4idea/src/git4idea/rebase/GitRebaseEditorService.java index fb2007a7be49..9764bc5f8457 100644 --- a/plugins/git4idea/src/git4idea/rebase/GitRebaseEditorService.java +++ b/plugins/git4idea/src/git4idea/rebase/GitRebaseEditorService.java @@ -17,9 +17,9 @@ package git4idea.rebase; import com.intellij.ide.XmlRpcServer; import com.intellij.openapi.components.ServiceManager; +import com.intellij.util.containers.ContainerUtil; import git4idea.commands.GitCommand; import git4idea.commands.GitLineHandler; -import gnu.trove.THashMap; import org.apache.commons.codec.DecoderException; import org.apache.xmlrpc.XmlRpcClientLite; import org.jetbrains.annotations.NonNls; @@ -27,9 +27,8 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.git4idea.util.ScriptGenerator; import org.jetbrains.ide.BuiltInServerManager; -import java.security.SecureRandom; import java.util.Map; -import java.util.Random; +import java.util.UUID; /** * The service that generates editor script for @@ -46,15 +45,11 @@ public class GitRebaseEditorService { /** * The handlers to use */ - private final Map myHandlers = new THashMap<>(); + private final Map myHandlers = ContainerUtil.newHashMap(); /** * The lock for the handlers */ private final Object myHandlersLock = new Object(); - /** - * Random number generator - */ - private static final Random oursRandom = new SecureRandom(); /** * The prefix for rebase editors */ @@ -103,33 +98,22 @@ public class GitRebaseEditorService { * @param handler the handler to register * @return the handler identifier */ - public int registerHandler(GitRebaseEditorHandler handler) { + @NotNull + public UUID registerHandler(@NotNull GitRebaseEditorHandler handler) { addInternalHandler(); - Integer rc = null; synchronized (myHandlersLock) { - for (int i = Integer.MAX_VALUE; i > 0; i--) { - int code = Math.abs(oursRandom.nextInt()); - // note that code might still be negative at this point if it is Integer.MIN_VALUE. - if (code > 0 && !myHandlers.containsKey(code)) { - rc = code; - break; - } - } - if (rc == null) { - throw new IllegalStateException("There is a problem with random number allocation"); - } - myHandlers.put(rc, handler); + UUID key = UUID.randomUUID(); + myHandlers.put(key, handler); + return key; } - return rc; } - /** * Unregister handler * * @param handlerNo the handler number. */ - public void unregisterHandler(final int handlerNo) { + public void unregisterHandler(@NotNull UUID handlerNo) { synchronized (myHandlersLock) { if (myHandlers.remove(handlerNo) == null) { throw new IllegalStateException("The handler " + handlerNo + " has been already removed"); @@ -143,7 +127,7 @@ public class GitRebaseEditorService { * @param handlerNo the handler number. */ @NotNull - GitRebaseEditorHandler getHandler(final int handlerNo) { + GitRebaseEditorHandler getHandler(@NotNull UUID handlerNo) { synchronized (myHandlersLock) { GitRebaseEditorHandler h = myHandlers.get(handlerNo); if (h == null) { @@ -159,9 +143,9 @@ public class GitRebaseEditorService { * @param h the handler to configure * @param editorNo the editor number */ - public void configureHandler(GitLineHandler h, int editorNo) { + public void configureHandler(GitLineHandler h, @NotNull UUID editorNo) { h.setEnvironment(GitCommand.GIT_EDITOR_ENV, getEditorCommand()); - h.setEnvironment(GitRebaseEditorMain.IDEA_REBASE_HANDER_NO, Integer.toString(editorNo)); + h.setEnvironment(GitRebaseEditorMain.IDEA_REBASE_HANDER_NO, editorNo.toString()); } @@ -177,8 +161,8 @@ public class GitRebaseEditorService { * @return exit code */ @SuppressWarnings({"UnusedDeclaration"}) - public int editCommits(int handlerNo, String path) { - GitRebaseEditorHandler editor = getHandler(handlerNo); + public int editCommits(@NotNull String handlerNo, String path) { + GitRebaseEditorHandler editor = getHandler(UUID.fromString(handlerNo)); return editor.editCommits(path); } } diff --git a/plugins/git4idea/src/git4idea/rebase/GitRebaser.java b/plugins/git4idea/src/git4idea/rebase/GitRebaser.java index 27cc42fe0d65..34c28e2dc251 100644 --- a/plugins/git4idea/src/git4idea/rebase/GitRebaser.java +++ b/plugins/git4idea/src/git4idea/rebase/GitRebaser.java @@ -163,7 +163,7 @@ public class GitRebaser { GitRebaseEditorService rebaseEditorService = GitRebaseEditorService.getInstance(); // TODO If interactive rebase with commit rewording was invoked, this should take the reworded message GitRebaser.TrivialEditor editor = new GitRebaser.TrivialEditor(rebaseEditorService, myProject, root, rh); - Integer rebaseEditorNo = editor.getHandlerNo(); + UUID rebaseEditorNo = editor.getHandlerNo(); rebaseEditorService.configureHandler(rh, rebaseEditorNo); } @@ -196,7 +196,7 @@ public class GitRebaser { final GitLineHandler h = new GitLineHandler(myProject, root, GitCommand.REBASE); h.setStdoutSuppressed(false); - Integer rebaseEditorNo = null; + UUID rebaseEditorNo = null; GitRebaseEditorService rebaseEditorService = GitRebaseEditorService.getInstance(); try { h.addParameters("-i", "-m", "-v");