fix: NPE in setInputFiles (#1113)

Author: yury-s

playwright/src/main/java/com/microsoft/playwright/impl/ArtifactImpl.java

@@ -26,7 +26,6 @@
 import static com.microsoft.playwright.impl.Utils.writeToFile;
 
 class ArtifactImpl extends ChannelOwner {
-  boolean isRemote;
   public ArtifactImpl(ChannelOwner parent, String type, String guid, JsonObject initializer) {
     super(parent, type, guid, initializer);
   }
@@ -57,15 +56,15 @@ public String failure() {
   }
 
   public Path pathAfterFinished() {
-    if (isRemote) {
+    if (connection.isRemote) {
       throw new PlaywrightException("Path is not available when using browserType.connect(). Use download.saveAs() to save a local copy.");
     }
     JsonObject json = sendMessage("pathAfterFinished").getAsJsonObject();
     return FileSystems.getDefault().getPath(json.get("value").getAsString());
   }
 
   public void saveAs(Path path) {
-    if (isRemote) {
+    if (connection.isRemote) {
       JsonObject jsonObject = sendMessage("saveAsStream").getAsJsonObject();
       Stream stream = connection.getExistingObject(jsonObject.getAsJsonObject("stream").get("guid").getAsString());
       writeToFile(stream.stream(), path);

playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java

@@ -82,7 +82,6 @@ enum EventType {
       browser = null;
     }
     this.tracing = connection.getExistingObject(initializer.getAsJsonObject("tracing").get("guid").getAsString());
-    tracing.isRemote = browser != null && browser.isRemote;
     this.request = connection.getExistingObject(initializer.getAsJsonObject("requestContext").get("guid").getAsString());
   }
 
@@ -201,12 +200,6 @@ private void closeImpl() {
         params.addProperty("harId", entry.getKey());
         JsonObject json = sendMessage("harExport", params).getAsJsonObject();
         ArtifactImpl artifact = connection.getExistingObject(json.getAsJsonObject("artifact").get("guid").getAsString());
-        // In case of CDP connection browser is null but since the connection is established by
-        // the driver it is safe to consider the artifact local.
-        if (browser() != null && browser().isRemote) {
-          artifact.isRemote = true;
-        }
-
         // Server side will compress artifact if content is attach or if file is .zip.
         HarRecorder harParams = entry.getValue();
         boolean isCompressed = harParams.contentPolicy == HarContentPolicy.ATTACH || harParams.path.toString().endsWith(".zip");

playwright/src/main/java/com/microsoft/playwright/impl/BrowserImpl.java

@@ -38,7 +38,6 @@
 class BrowserImpl extends ChannelOwner implements Browser {
   final Set<BrowserContextImpl> contexts = new HashSet<>();
   private final ListenerCollection<EventType> listeners = new ListenerCollection<>();
-  boolean isRemote;
   boolean isConnectedOverWebSocket;
   private boolean isConnected = true;
   BrowserTypeImpl browserType;

playwright/src/main/java/com/microsoft/playwright/impl/BrowserTypeImpl.java

@@ -97,7 +97,6 @@ private Browser connectImpl(String wsEndpoint, ConnectOptions options) {
     }
     playwright.initSharedSelectors(this.connection.getExistingObject("Playwright"));
     BrowserImpl browser = connection.getExistingObject(playwright.initializer.getAsJsonObject("preLaunchedBrowser").get("guid").getAsString());
-    browser.isRemote = true;
     browser.isConnectedOverWebSocket = true;
     browser.browserType = this;
     Consumer<JsonPipe> connectionCloseListener = t -> browser.notifyRemoteClosed();
@@ -132,7 +131,6 @@ private Browser connectOverCDPImpl(String endpointURL, ConnectOverCDPOptions opt
     JsonObject json = sendMessage("connectOverCDP", params).getAsJsonObject();
 
     BrowserImpl browser = connection.getExistingObject(json.getAsJsonObject("browser").get("guid").getAsString());
-    browser.isRemote = true;
     browser.browserType = this;
     if (json.has("defaultContext")) {
       String contextId = json.getAsJsonObject("defaultContext").get("guid").getAsString();

playwright/src/main/java/com/microsoft/playwright/impl/Connection.java

@@ -55,6 +55,7 @@ public class Connection {
   private final Transport transport;
   private final Map<String, ChannelOwner> objects = new HashMap<>();
   private final Root root;
+  final boolean isRemote;
   private int lastId = 0;
   private final StackTraceCollector stackTraceCollector;
   private final Map<Integer, WaitableResult<JsonElement>> callbacks = new HashMap<>();
@@ -81,12 +82,17 @@ Playwright initialize() {
   }
 
   Connection(Transport pipe, Map<String, String> env, LocalUtils localUtils) {
-    this(pipe, env);
+    this(pipe, env, true);
     this.localUtils = localUtils;
   }
 
   Connection(Transport transport, Map<String, String> env) {
+    this(transport, env, false);
+  }
+
+  private Connection(Transport transport, Map<String, String> env, boolean isRemote) {
     this.env = env;
+    this.isRemote = isRemote;
     if (isLogging) {
       transport = new TransportLogger(transport);
     }

playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java

@@ -151,7 +151,6 @@ protected void handleEvent(String event, JsonObject params) {
     } else if ("download".equals(event)) {
       String artifactGuid = params.getAsJsonObject("artifact").get("guid").getAsString();
       ArtifactImpl artifact = connection.getExistingObject(artifactGuid);
-      artifact.isRemote = browserContext.browser() != null && browserContext.browser().isRemote;
       DownloadImpl download = new DownloadImpl(this, artifact, params);
       listeners.notify(EventType.DOWNLOAD, download);
     } else if ("fileChooser".equals(event)) {

playwright/src/main/java/com/microsoft/playwright/impl/TracingImpl.java

@@ -26,8 +26,6 @@
 import static com.microsoft.playwright.impl.Serialization.gson;
 
 class TracingImpl extends ChannelOwner implements Tracing {
-  boolean isRemote;
-
   TracingImpl(ChannelOwner parent, String type, String guid, JsonObject initializer) {
     super(parent, type, guid, initializer);
   }
@@ -36,7 +34,7 @@ private void stopChunkImpl(Path path) {
     JsonObject params = new JsonObject();
     String mode = "doNotSave";
     if (path != null) {
-      if (isRemote) {
+      if (connection.isRemote) {
         mode = "compressTrace";
       } else {
         mode = "compressTraceAndSources";
@@ -48,16 +46,13 @@ private void stopChunkImpl(Path path) {
       return;
     }
     ArtifactImpl artifact = connection.getExistingObject(json.getAsJsonObject("artifact").get("guid").getAsString());
-    // In case of CDP connection browser is null but since the connection is established by
-    // the driver it is safe to consider the artifact local.
-    if (isRemote) {
-      artifact.isRemote = true;
-    }
     artifact.saveAs(path);
     artifact.delete();
 
     // Add local sources to the remote trace if necessary.
-    if (isRemote && json.has("sourceEntries")) {
+    // In case of CDP connection since the connection is established by
+    // the driver it is safe to consider the artifact local.
+    if (connection.isRemote && json.has("sourceEntries")) {
       JsonArray entries = json.getAsJsonArray("sourceEntries");
       connection.localUtils.zip(path, entries);
     }

playwright/src/main/java/com/microsoft/playwright/impl/Utils.java

@@ -165,7 +165,7 @@ static boolean hasLargeFile(Path[] files) {
   }
 
   static void addLargeFileUploadParams(Path[] files, JsonObject params, BrowserContextImpl context) {
-    if (context.browser().isRemote) {
+    if (context.connection.isRemote) {
       List<WritableStream> streams = new ArrayList<>();
       JsonArray jsonStreams = new JsonArray();
       for (Path path : files) {

playwright/src/main/java/com/microsoft/playwright/impl/VideoImpl.java

@@ -27,16 +27,13 @@
 class VideoImpl implements Video {
   private final PageImpl page;
   private final WaitableResult<ArtifactImpl> waitableArtifact = new WaitableResult<>();
-  private final boolean isRemote;
 
   VideoImpl(PageImpl page) {
     this.page = page;
     BrowserImpl browser = page.context().browser();
-    isRemote = browser != null && browser.isRemote;
   }
 
   void setArtifact(ArtifactImpl artifact) {
-    artifact.isRemote = isRemote;
     waitableArtifact.complete(artifact);
   }
 
@@ -58,7 +55,7 @@ public void delete() {
   @Override
   public Path path() {
     return page.withLogging("Video.path", () -> {
-      if (isRemote) {
+      if (page.connection.isRemote) {
         throw new PlaywrightException("Path is not available when using browserType.connect(). Use saveAs() to save a local copy.");
       }
       try {

playwright/src/test/java/com/microsoft/playwright/TestDefaultBrowserContext2.java

@@ -2,14 +2,19 @@
 
 import com.microsoft.playwright.options.Geolocation;
 import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.DisabledIf;
 import org.junit.jupiter.api.io.TempDir;
 
 import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.stream.Collectors;
@@ -251,4 +256,43 @@ void shouldRespectSelectors() {
     assertEquals("hello", page.innerHTML("defaultContextCSS=div"));
   }
 
-}
+  @Test
+  void shouldUploadLargeFile(@TempDir Path tmpDir) throws IOException, ExecutionException, InterruptedException {
+    Assumptions.assumeTrue(3 <= (Runtime.getRuntime().maxMemory() >> 30), "Fails if max heap size is < 3Gb");
+    Page page = launchPersistent();
+    page.navigate(server.PREFIX + "/input/fileupload.html");
+    Path uploadFile = tmpDir.resolve("200MB.zip");
+    String str = String.join("", Collections.nCopies(4 * 1024, "A"));
+
+    try (Writer stream = new OutputStreamWriter(Files.newOutputStream(uploadFile))) {
+      for (int i = 0; i < 50 * 1024; i++) {
+        stream.write(str);
+      }
+    }
+    Locator input = page.locator("input[type='file']");
+    JSHandle events = input.evaluateHandle("e => {\n" +
+      "    const events = [];\n" +
+      "    e.addEventListener('input', () => events.push('input'));\n" +
+      "    e.addEventListener('change', () => events.push('change'));\n" +
+      "    return events;\n" +
+      "  }");
+    input.setInputFiles(uploadFile);
+    assertEquals("200MB.zip", input.evaluate("e => e.files[0].name"));
+    assertEquals(asList("input", "change"), events.evaluate("e => e"));
+    CompletableFuture<MultipartFormData> formData = new CompletableFuture<>();
+    server.setRoute("/upload", exchange -> {
+      try {
+        MultipartFormData multipartFormData = MultipartFormData.parseRequest(exchange);
+        formData.complete(multipartFormData);
+      } catch (Exception e) {
+        e.printStackTrace();
+        formData.completeExceptionally(e);
+      }
+      exchange.sendResponseHeaders(200, -1);
+    });
+    page.click("input[type=submit]", new Page.ClickOptions().setTimeout(90_000));
+    List<MultipartFormData.Field> fields = formData.get().fields;
+    assertEquals(1, fields.size());
+    assertEquals("200MB.zip", fields.get(0).filename);
+    assertEquals(200 * 1024 * 1024, fields.get(0).content.length());
+  }}