From 8a716685085d476ba32698e691ece94c6ea6f3a7 Mon Sep 17 00:00:00 2001 From: 0xb00bface <0xboobface@gmail.com> Date: Sat, 11 Nov 2023 12:39:18 +0100 Subject: [PATCH] Fix NPE in ExternalBrowser --- .../main/java/ctbrec/ui/ExternalBrowser.java | 165 +++++++++--------- 1 file changed, 80 insertions(+), 85 deletions(-) diff --git a/client/src/main/java/ctbrec/ui/ExternalBrowser.java b/client/src/main/java/ctbrec/ui/ExternalBrowser.java index a4801e06..e4de6000 100644 --- a/client/src/main/java/ctbrec/ui/ExternalBrowser.java +++ b/client/src/main/java/ctbrec/ui/ExternalBrowser.java @@ -1,47 +1,37 @@ package ctbrec.ui; -import static java.nio.charset.StandardCharsets.*; +import ctbrec.Config; +import ctbrec.OS; +import ctbrec.io.ProcessOutputLogger; +import lombok.extern.slf4j.Slf4j; +import org.json.JSONObject; -import java.io.BufferedReader; -import java.io.File; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.OutputStream; +import java.io.*; import java.net.Socket; -import java.util.Arrays; -import java.util.HashMap; -import java.util.Map; -import java.util.Optional; -import java.util.UUID; +import java.util.*; import java.util.concurrent.CompletableFuture; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; -import org.json.JSONObject; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import static java.nio.charset.StandardCharsets.UTF_8; -import ctbrec.Config; -import ctbrec.OS; -import ctbrec.io.ProcessOutputLogger; - -public class ExternalBrowser implements AutoCloseable { - private static final Logger LOG = LoggerFactory.getLogger(ExternalBrowser.class); +@Slf4j +public class ExternalBrowser implements AutoCloseable { // NOSONAR singleton is wanted private static final ExternalBrowser INSTANCE = new ExternalBrowser(); - private Lock lock = new ReentrantLock(); + private static final String MSGID = "msgid"; + private final Lock lock = new ReentrantLock(); + private Socket socket; private Consumer messageListener; private InputStream in; private OutputStream out; - private Socket socket; // NOSONAR private Thread reader; private volatile boolean stopped = true; private volatile boolean browserReady = false; - private Object browserReadyLock = new Object(); + private final Object browserReadyLock = new Object(); - private Map> responseFutures = new HashMap<>(); + private final Map> responseFutures = new HashMap<>(); private Runnable onReadyCallback; public static ExternalBrowser getInstance() { @@ -49,7 +39,7 @@ public class ExternalBrowser implements AutoCloseable { } public void run(JSONObject jsonConfig, Consumer messageListener) throws InterruptedException, IOException { - LOG.debug("Running browser with config {}", jsonConfig); + log.debug("Running browser with config {}", jsonConfig); lock.lock(); try { stopped = false; @@ -62,7 +52,7 @@ public class ExternalBrowser implements AutoCloseable { Process p = new ProcessBuilder(cmdline).start(); new Thread(new ProcessOutputLogger(p.getInputStream(), "ExternalBrowser stdout")).start(); new Thread(new ProcessOutputLogger(p.getErrorStream(), "ExternalBrowser stderr")).start(); - LOG.debug("Browser started: {}", Arrays.toString(cmdline)); + log.debug("Browser started: {}", Arrays.toString(cmdline)); connectToRemoteControlSocket(); while (!browserReady) { @@ -70,10 +60,10 @@ public class ExternalBrowser implements AutoCloseable { browserReadyLock.wait(100); } } - if(LOG.isTraceEnabled()) { - LOG.debug("Connected to remote control server. Sending config {}", jsonConfig); + if (log.isTraceEnabled()) { + log.debug("Connected to remote control server. Sending config {}", jsonConfig); } else { - LOG.debug("Connected to remote control server. Sending config"); + log.debug("Connected to remote control server. Sending config"); } out.write(jsonConfig.toString().getBytes(UTF_8)); out.write('\n'); @@ -81,14 +71,14 @@ public class ExternalBrowser implements AutoCloseable { Optional.ofNullable(onReadyCallback).ifPresent(Runnable::run); - LOG.debug("Waiting for browser to terminate"); + log.debug("Waiting for browser to terminate"); p.waitFor(); int exitValue = p.exitValue(); reader = null; in = null; out = null; this.messageListener = null; - LOG.debug("Browser Process terminated with {}", exitValue); + log.debug("Browser Process terminated with {}", exitValue); } finally { lock.unlock(); } @@ -102,11 +92,11 @@ public class ExternalBrowser implements AutoCloseable { out = socket.getOutputStream(); reader = new Thread(this::readBrowserOutput); reader.start(); - LOG.debug("Connected to control socket"); + log.debug("Connected to control socket"); return; } catch (IOException e) { - if(i == 19) { - LOG.error("Connection to remote control socket failed", e); + if (i == 19) { + log.error("Connection to remote control socket failed", e); throw e; } } @@ -122,9 +112,9 @@ public class ExternalBrowser implements AutoCloseable { public CompletableFuture executeJavaScript(String javaScript) throws IOException { String id = UUID.randomUUID().toString(); - var future = new CompletableFuture(); + var future = new CompletableFuture<>(); var script = new JSONObject(); - script.put("msgid", id); + script.put(MSGID, id); script.put("execute", javaScript); if (out != null) { out.write(script.toString().getBytes(UTF_8)); @@ -140,7 +130,7 @@ public class ExternalBrowser implements AutoCloseable { @Override public void close() throws IOException { - if(stopped) { + if (stopped) { return; } stopped = true; @@ -148,27 +138,29 @@ public class ExternalBrowser implements AutoCloseable { } private void readBrowserOutput() { - LOG.debug("Browser output reader started"); + log.debug("Browser output reader started"); try (var br = new BufferedReader(new InputStreamReader(in))) { String line; synchronized (browserReadyLock) { browserReady = true; browserReadyLock.notifyAll(); } - while( !Thread.interrupted() && (line = br.readLine()) != null ) { - LOG.debug("Browser output: {}", line); + while (!Thread.interrupted() && (line = br.readLine()) != null) { + log.debug("Browser output: {}", line); if (line.startsWith("{")) { JSONObject json = new JSONObject(line); - if (json.has("msgid")) { + if (json.has(MSGID)) { handleExecuteScriptResponse(json); } else { - messageListener.accept(line); + if (messageListener != null) { + messageListener.accept(line); + } } } } } catch (IOException e) { - if(!stopped) { - LOG.error("Couldn't read browser output", e); + if (!stopped) { + log.error("Couldn't read browser output", e); } } finally { stopped = true; @@ -180,59 +172,62 @@ public class ExternalBrowser implements AutoCloseable { } private void handleExecuteScriptResponse(JSONObject json) { - var msgid = json.getString("msgid"); - LOG.debug("Future {}", msgid); + var msgid = json.getString(MSGID); + log.debug("Future {}", msgid); CompletableFuture future = responseFutures.get(msgid); if (future != null) { responseFutures.remove(msgid); - if (json.has("result")) { - LOG.debug("Future {} done. Result: {}", msgid, json.get("result")); - future.complete(json.get("result")); + final String RESULT = "result"; + if (json.has(RESULT)) { + log.debug("Future {} done. Result: {}", msgid, json.get(RESULT)); + future.complete(json.get(RESULT)); } else if (json.has("error")) { - LOG.debug("Future {} failed", msgid); + log.debug("Future {} failed", msgid); future.completeExceptionally(new Exception(json.getJSONObject("error").toString())); } } else { - LOG.warn("No future for previous request {}", msgid); + log.warn("No future for previous request {}", msgid); } } private void addProxyConfig(JSONObject jsonConfig) { var proxyType = Config.getInstance().getSettings().proxyType; + var proxy = new JSONObject(); + final String KEY_ADDRESS = "address"; + final String KEY_PROXY = "proxy"; switch (proxyType) { - case HTTP: - var proxy = new JSONObject(); - proxy.put("address", - "http=" + Config.getInstance().getSettings().proxyHost + ':' + Config.getInstance().getSettings().proxyPort - + ";https=" + Config.getInstance().getSettings().proxyHost + ':' + Config.getInstance().getSettings().proxyPort); - if(Config.getInstance().getSettings().proxyUser != null && !Config.getInstance().getSettings().proxyUser.isEmpty()) { - String username = Config.getInstance().getSettings().proxyUser; - String password = Config.getInstance().getSettings().proxyPassword; - proxy.put("user", username); - proxy.put("password", password); - } - jsonConfig.put("proxy", proxy); - break; - case SOCKS4: - proxy = new JSONObject(); - proxy.put("address", "socks4://" + Config.getInstance().getSettings().proxyHost + ':' + Config.getInstance().getSettings().proxyPort); - jsonConfig.put("proxy", proxy); - break; - case SOCKS5: - proxy = new JSONObject(); - proxy.put("address", "socks5://" + Config.getInstance().getSettings().proxyHost + ':' + Config.getInstance().getSettings().proxyPort); - if(Config.getInstance().getSettings().proxyUser != null && !Config.getInstance().getSettings().proxyUser.isEmpty()) { - String username = Config.getInstance().getSettings().proxyUser; - String password = Config.getInstance().getSettings().proxyPassword; - proxy.put("user", username); - proxy.put("password", password); - } - jsonConfig.put("proxy", proxy); - break; - case DIRECT: - default: - // nothing to do here - break; + case HTTP: + proxy.put(KEY_ADDRESS, + "http=" + Config.getInstance().getSettings().proxyHost + ':' + Config.getInstance().getSettings().proxyPort + + ";https=" + Config.getInstance().getSettings().proxyHost + ':' + Config.getInstance().getSettings().proxyPort); + if (Config.getInstance().getSettings().proxyUser != null && !Config.getInstance().getSettings().proxyUser.isEmpty()) { + String username = Config.getInstance().getSettings().proxyUser; + String password = Config.getInstance().getSettings().proxyPassword; + proxy.put("user", username); + proxy.put("password", password); + } + jsonConfig.put(KEY_PROXY, proxy); + break; + case SOCKS4: + proxy = new JSONObject(); + proxy.put(KEY_ADDRESS, "socks4://" + Config.getInstance().getSettings().proxyHost + ':' + Config.getInstance().getSettings().proxyPort); + jsonConfig.put(KEY_PROXY, proxy); + break; + case SOCKS5: + proxy = new JSONObject(); + proxy.put(KEY_ADDRESS, "socks5://" + Config.getInstance().getSettings().proxyHost + ':' + Config.getInstance().getSettings().proxyPort); + if (Config.getInstance().getSettings().proxyUser != null && !Config.getInstance().getSettings().proxyUser.isEmpty()) { + String username = Config.getInstance().getSettings().proxyUser; + String password = Config.getInstance().getSettings().proxyPassword; + proxy.put("user", username); + proxy.put("password", password); + } + jsonConfig.put(KEY_PROXY, proxy); + break; + case DIRECT: + default: + // nothing to do here + break; } }