Minor improvements

gradle.properties

@@ -1 +1,2 @@
 org.gradle.warning.mode=all
+org.gradle.parallel=true

jadx-core/src/main/java/jadx/core/codegen/NameGen.java

@@ -268,13 +268,17 @@ private String makeNameFromInsn(InsnNode insn) {
 
 	private String makeNameFromInvoke(MethodInfo callMth) {
 		String name = callMth.getName();
+		ArgType declType = callMth.getDeclClass().getType();
+		if ("getInstance".equals(name)) {
+			// e.g. Cipher.getInstance
+			return makeNameForType(declType);
+		}
 		if (name.startsWith("get") || name.startsWith("set")) {
 			return fromName(name.substring(3));
 		}
 		if ("iterator".equals(name)) {
 			return "it";
 		}
-		ArgType declType = callMth.getDeclClass().getType();
 		if ("toString".equals(name)) {
 			return makeNameForType(declType);
 		}

jadx-core/src/main/java/jadx/core/utils/log/LogUtils.java

@@ -0,0 +1,30 @@
+package jadx.core.utils.log;
+
+import java.nio.charset.StandardCharsets;
+import java.util.regex.Pattern;
+
+/**
+ * Escape input from untrusted source before pass to logger.
+ * Suggested by CodeQL: https://codeql.github.com/codeql-query-help/java/java-log-injection/
+ */
+public class LogUtils {
+
+	private static final Pattern ALFA_NUMERIC = Pattern.compile("\\w*");
+
+	public static String escape(String input) {
+		if (input == null) {
+			return "null";
+		}
+		if (ALFA_NUMERIC.matcher(input).matches()) {
+			return input;
+		}
+		return input.replaceAll("\\W", ".");
+	}
+
+	public static String escape(byte[] input) {
+		if (input == null) {
+			return "null";
+		}
+		return escape(new String(input, StandardCharsets.UTF_8));
+	}
+}

jadx-core/src/test/java/jadx/core/utils/log/LogUtilsTest.java

@@ -0,0 +1,13 @@
+package jadx.core.utils.log;
+
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class LogUtilsTest {
+
+	@Test
+	void escape() {
+		assertThat(LogUtils.escape("Guest'%0AUser:'Admin")).isEqualTo("Guest..0AUser..Admin");
+	}
+}

jadx-gui/src/main/java/jadx/gui/device/debugger/DebugController.java

@@ -11,6 +11,7 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 
+import javax.swing.JOptionPane;
 import javax.swing.tree.DefaultMutableTreeNode;
 
 import org.slf4j.Logger;
@@ -40,6 +41,7 @@
 import jadx.gui.ui.panel.JDebuggerPanel;
 import jadx.gui.ui.panel.JDebuggerPanel.IListElement;
 import jadx.gui.ui.panel.JDebuggerPanel.ValueTreeNode;
+import jadx.gui.utils.NLS;
 
 public final class DebugController implements SmaliDebugger.SuspendListener, IDebugController {
 	private static final Logger LOG = LoggerFactory.getLogger(DebugController.class);
@@ -69,23 +71,22 @@ public final class DebugController implements SmaliDebugger.SuspendListener, IDe
 	private final ExecutorService updateQueue = Executors.newSingleThreadExecutor();
 	private final ExecutorService lazyQueue = Executors.newSingleThreadExecutor();
 
-	/**
-	 * @param args at least 3 elements, host, port and android release version respectively.
-	 */
 	@Override
-	public boolean startDebugger(JDebuggerPanel debuggerPanel, String[] args) {
+	public boolean startDebugger(JDebuggerPanel debuggerPanel, String adbHost, int adbPort, int androidVer) {
 		if (TYPE_MAP.isEmpty()) {
 			initTypeMap();
 		}
 		this.debuggerPanel = debuggerPanel;
 		debuggerPanel.resetUI();
 		try {
-			debugger = SmaliDebugger.attach(args[0], Integer.parseInt(args[1]), this);
+			debugger = SmaliDebugger.attach(adbHost, adbPort, this);
 		} catch (SmaliDebuggerException e) {
+			JOptionPane.showMessageDialog(debuggerPanel.getMainWindow(), e.getMessage(),
+					NLS.str("error_dialog.title"), JOptionPane.ERROR_MESSAGE);
 			logErr(e);
 			return false;
 		}
-		art = ArtAdapter.getAdapter(Integer.parseInt(args[2]));
+		art = ArtAdapter.getAdapter(androidVer);
 		resetAllInfo();
 		hasResumed = false;
 		run = debugger::resume;

jadx-gui/src/main/java/jadx/gui/device/protocol/ADB.java

@@ -5,6 +5,7 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.Socket;
+import java.net.SocketException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -14,10 +15,12 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import jadx.core.utils.log.LogUtils;
 import jadx.gui.utils.IOUtils;
 
 public class ADB {
@@ -29,22 +32,36 @@ public class ADB {
 	private static final String CMD_FEATURES = "000dhost:features";
 	private static final String CMD_TRACK_DEVICES = "0014host:track-devices-l";
 	private static final byte[] OKAY = "OKAY".getBytes();
+	private static final byte[] FAIL = "FAIL".getBytes();
 
 	static boolean isOkay(InputStream stream) throws IOException {
 		byte[] buf = IOUtils.readNBytes(stream, 4);
-		return Arrays.equals(buf, OKAY);
+		if (Arrays.equals(buf, OKAY)) {
+			return true;
+		}
+		if (Arrays.equals(buf, FAIL)) {
+			// Observed that after FAIL the length in hex follows and afterwards an error message,
+			// but it is unclear if this is true for all cases where isOkay is used.
+			// int msgLen = Integer.parseInt(new String(IOUtils.readNBytes(stream, 4)), 16);
+			// byte[] errorMsg = IOUtils.readNBytes(stream, msgLen);
+			// LOG.error("isOkay failed: received error message: {}", new String(errorMsg));
+			LOG.error("isOkay failed");
+			return false;
+		}
+		if (buf == null) {
+			throw new IOException("isOkay failed - steam ended");
+		}
+		throw new IOException("isOkay failed - unexpected response " + new String(buf));
 	}
 
 	public static byte[] exec(String cmd, OutputStream outputStream, InputStream inputStream) throws IOException {
 		return execCommandSync(outputStream, inputStream, cmd);
 	}
 
 	public static byte[] exec(String cmd) throws IOException {
-		byte[] res;
 		try (Socket socket = connect()) {
-			res = exec(cmd, socket.getOutputStream(), socket.getInputStream());
+			return exec(cmd, socket.getOutputStream(), socket.getInputStream());
 		}
-		return res;
 	}
 
 	public static Socket connect() throws IOException {
@@ -55,14 +72,12 @@ public static Socket connect(String host, int port) throws IOException {
 		return new Socket(host, port);
 	}
 
-	static boolean execCommandAsync(OutputStream outputStream,
-			InputStream inputStream, String cmd) throws IOException {
+	static boolean execCommandAsync(OutputStream outputStream, InputStream inputStream, String cmd) throws IOException {
 		outputStream.write(cmd.getBytes());
 		return isOkay(inputStream);
 	}
 
-	private static byte[] execCommandSync(OutputStream outputStream,
-			InputStream inputStream, String cmd) throws IOException {
+	private static byte[] execCommandSync(OutputStream outputStream, InputStream inputStream, String cmd) throws IOException {
 		outputStream.write(cmd.getBytes());
 		if (isOkay(inputStream)) {
 			return readServiceProtocol(inputStream);
@@ -73,15 +88,26 @@ private static byte[] execCommandSync(OutputStream outputStream,
 	static byte[] readServiceProtocol(InputStream stream) {
 		try {
 			byte[] buf = IOUtils.readNBytes(stream, 4);
+			if (buf == null) {
+				return null;
+			}
 			int len = unhex(buf);
+			byte[] result;
 			if (len == 0) {
-				return new byte[0];
+				result = new byte[0];
+			} else {
+				result = IOUtils.readNBytes(stream, len);
+			}
+			if (LOG.isTraceEnabled()) {
+				LOG.trace("readServiceProtocol result: {}", LogUtils.escape(result));
 			}
-			return IOUtils.readNBytes(stream, len);
+			return result;
+		} catch (SocketException e) {
+			LOG.error("Aborting readServiceProtocol: socket closed");
 		} catch (IOException e) {
-			LOG.error("Failed to read readServiceProtocol: {}", e.toString());
-			return null;
+			LOG.error("Failed to read readServiceProtocol", e);
 		}
+		return null;
 	}
 
 	static boolean setSerial(String serial, OutputStream outputStream, InputStream inputStream) throws IOException {
@@ -91,7 +117,9 @@ static boolean setSerial(String serial, OutputStream outputStream, InputStream i
 		boolean ok = isOkay(inputStream);
 		if (ok) {
 			// skip the shell-state-id returned by ADB server, it's not important for the following actions.
-			ok = inputStream.skip(8) == 8;
+			IOUtils.readNBytes(inputStream, 8);
+		} else {
+			LOG.error("setSerial command {} failed", LogUtils.escape(setSerialCmd));
 		}
 		return ok;
 	}
@@ -106,8 +134,7 @@ private static byte[] execShellCommandRaw(String cmd, OutputStream outputStream,
 		return null;
 	}
 
-	static byte[] execShellCommandRaw(String serial, String cmd,
-			OutputStream outputStream, InputStream inputStream) throws IOException {
+	static byte[] execShellCommandRaw(String serial, String cmd, OutputStream outputStream, InputStream inputStream) throws IOException {
 		if (setSerial(serial, outputStream, inputStream)) {
 			return execShellCommandRaw(cmd, outputStream, inputStream);
 		}
@@ -124,14 +151,16 @@ public static List<String> getFeatures() throws IOException {
 
 	public static boolean startServer(String adbPath, int port) throws IOException {
 		String tcpPort = String.format("tcp:%d", port);
-		java.lang.Process proc = new ProcessBuilder(adbPath, "-L", tcpPort, "start-server")
+		List<String> command = Arrays.asList(adbPath, "-L", tcpPort, "start-server");
+		java.lang.Process proc = new ProcessBuilder(command)
 				.redirectErrorStream(true)
 				.start();
 		try {
-			proc.waitFor(3, TimeUnit.SECONDS); // for listening to a port, 3 sec should be more than enough.
+			// Wait for the adb server to start. On Windows even on a fast system 6 seconds are not unusual.
+			proc.waitFor(10, TimeUnit.SECONDS);
 			proc.exitValue();
 		} catch (Exception e) {
-			LOG.error("Start server error", e);
+			LOG.error("ADB start server failed with command: {}", String.join(" ", command), e);
 			proc.destroyForcibly();
 			return false;
 		}
@@ -143,7 +172,7 @@ public static boolean startServer(String adbPath, int port) throws IOException {
 				out.write(buf, 0, read);
 			}
 		}
-		return new String(out.toByteArray()).contains(tcpPort);
+		return out.toString().contains(tcpPort);
 	}
 
 	public static boolean isServerRunning(String host, int port) {
@@ -167,22 +196,21 @@ public static Socket listenForDeviceState(DeviceStateListener listener, String h
 		}
 		ExecutorService listenThread = Executors.newFixedThreadPool(1);
 		listenThread.execute(() -> {
-			for (;;) {
+			while (true) {
 				byte[] res = readServiceProtocol(inputStream);
-				if (res != null) {
-					if (listener != null) {
-						String payload = new String(res);
-						String[] deviceLines = payload.split("\n");
-						List<ADBDeviceInfo> deviceInfoList = new ArrayList<>(deviceLines.length);
-						for (String deviceLine : deviceLines) {
-							if (!deviceLine.trim().isEmpty()) {
-								deviceInfoList.add(ADBDeviceInfo.make(deviceLine, host, port));
-							}
+				if (res == null) {
+					break; // socket disconnected
+				}
+				if (listener != null) {
+					String payload = new String(res);
+					String[] deviceLines = payload.split("\n");
+					List<ADBDeviceInfo> deviceInfoList = new ArrayList<>(deviceLines.length);
+					for (String deviceLine : deviceLines) {
+						if (!deviceLine.trim().isEmpty()) {
+							deviceInfoList.add(ADBDeviceInfo.make(deviceLine, host, port));
 						}
-						listener.onDeviceStatusChange(deviceInfoList);
 					}
-				} else { // socket disconnected
-					break;
+					listener.onDeviceStatusChange(deviceInfoList);
 				}
 			}
 			if (listener != null) {
@@ -193,43 +221,39 @@ public static Socket listenForDeviceState(DeviceStateListener listener, String h
 	}
 
 	public static List<String> listForward(String host, int port) throws IOException {
-		Socket socket = connect(host, port);
-		String cmd = "0011host:list-forward";
-		InputStream inputStream = socket.getInputStream();
-		OutputStream outputStream = socket.getOutputStream();
-		outputStream.write(cmd.getBytes());
-		if (isOkay(inputStream)) {
-			byte[] bytes = readServiceProtocol(inputStream);
-			if (bytes != null) {
-				String[] forwards = new String(bytes).split("\n");
-				List<String> forwardList = Arrays.stream(forwards).map(s -> s.trim()).collect(Collectors.toList());
-				socket.close();
-				return forwardList;
+		try (Socket socket = connect(host, port)) {
+			String cmd = "0011host:list-forward";
+			InputStream inputStream = socket.getInputStream();
+			OutputStream outputStream = socket.getOutputStream();
+			outputStream.write(cmd.getBytes());
+			if (isOkay(inputStream)) {
+				byte[] bytes = readServiceProtocol(inputStream);
+				if (bytes != null) {
+					String[] forwards = new String(bytes).split("\n");
+					return Stream.of(forwards).map(String::trim).collect(Collectors.toList());
+				}
 			}
 		}
-		socket.close();
 		return Collections.emptyList();
 	}
 
 	public static boolean removeForward(String host, int port, String serial, String localPort) throws IOException {
-		Socket socket = connect(host, port);
-		String cmd = String.format("host:killforward:tcp:%s", localPort);
-		cmd = String.format("%04x%s", cmd.length(), cmd);
-		InputStream inputStream = socket.getInputStream();
-		OutputStream outputStream = socket.getOutputStream();
-		boolean ok = false;
-		if (setSerial(serial, outputStream, inputStream)) {
-			outputStream.write(cmd.getBytes());
-			ok = isOkay(inputStream) && isOkay(inputStream);
+		try (Socket socket = connect(host, port)) {
+			String cmd = String.format("host:killforward:tcp:%s", localPort);
+			cmd = String.format("%04x%s", cmd.length(), cmd);
+			InputStream inputStream = socket.getInputStream();
+			OutputStream outputStream = socket.getOutputStream();
+			if (setSerial(serial, outputStream, inputStream)) {
+				outputStream.write(cmd.getBytes());
+				return isOkay(inputStream) && isOkay(inputStream);
+			}
 		}
-		socket.close();
-		return ok;
+		return false;
 	}
 
 	// Little endian
 	private static int readInt(byte[] bytes, int start) {
-		int result = 0;
-		result = (bytes[start] & 0xff);
+		int result = (bytes[start] & 0xff);
 		result += ((bytes[start + 1] & 0xff) << 8);
 		result += ((bytes[start + 2] & 0xff) << 16);
 		result += (bytes[start + 3] & 0xff) << 24;