fix: NPE in setInputFiles (#1113)

This commit is contained in:
Yury Semikhatsky 2022-11-04 16:55:01 -07:00 committed by GitHub
parent 071ca8b90c
commit 3cc198ea26
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 60 additions and 30 deletions

View File

@ -26,7 +26,6 @@ import java.nio.file.Path;
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,7 +56,7 @@ class ArtifactImpl extends ChannelOwner {
}
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();
@ -65,7 +64,7 @@ class ArtifactImpl extends ChannelOwner {
}
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);

View File

@ -82,7 +82,6 @@ class BrowserContextImpl extends ChannelOwner implements BrowserContext {
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 @@ class BrowserContextImpl extends ChannelOwner implements BrowserContext {
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");

View File

@ -38,7 +38,6 @@ import static com.microsoft.playwright.impl.Utils.convertType;
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;

View File

@ -97,7 +97,6 @@ class BrowserTypeImpl extends ChannelOwner implements BrowserType {
}
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 @@ class BrowserTypeImpl extends ChannelOwner implements BrowserType {
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();

View File

@ -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 @@ public class Connection {
}
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);
}

View File

@ -151,7 +151,6 @@ public class PageImpl extends ChannelOwner implements Page {
} 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)) {

View File

@ -26,8 +26,6 @@ import java.nio.file.Path;
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 @@ class TracingImpl extends ChannelOwner implements Tracing {
JsonObject params = new JsonObject();
String mode = "doNotSave";
if (path != null) {
if (isRemote) {
if (connection.isRemote) {
mode = "compressTrace";
} else {
mode = "compressTraceAndSources";
@ -48,16 +46,13 @@ class TracingImpl extends ChannelOwner implements Tracing {
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);
}

View File

@ -165,7 +165,7 @@ class Utils {
}
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) {

View File

@ -27,16 +27,13 @@ import static java.util.Arrays.asList;
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 @@ class VideoImpl implements Video {
@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 {

View File

@ -2,14 +2,19 @@ package com.microsoft.playwright;
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 @@ public class TestDefaultBrowserContext2 extends TestBase {
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());
}}