follow-ups

Author: mxschmitt

playwright/_impl/_helper.py

@@ -294,11 +294,12 @@ async def handle(self, route: "Route") -> bool:
             if self._ignore_exception:
                 return False
             if is_target_closed_error(e):
-                # We are failing in the handler because the target close closed.
+                # We are failing in the handler because the target has closed.
                 # Give user a hint!
+                optional_async_prefix = "await " if not self._is_sync else ""
                 raise rewrite_error(
                     e,
-                    f"\"{str(e)}\" while running route callback.\nConsider awaiting `await page.unroute_all(behavior='ignoreErrors')`\nbefore the end of the test to ignore remaining routes in flight.",
+                    f"\"{str(e)}\" while running route callback.\nConsider awaiting `{optional_async_prefix}page.unroute_all(behavior='ignoreErrors')`\nbefore the end of the test to ignore remaining routes in flight.",
                 )
             raise e
         finally:

playwright/_impl/_network.py

@@ -111,11 +111,6 @@ def __init__(
         self._fallback_overrides: SerializedFallbackOverrides = (
             SerializedFallbackOverrides()
         )
-        base64_post_data = initializer.get("postData")
-        if base64_post_data is not None:
-            self._fallback_overrides.post_data_buffer = base64.b64decode(
-                base64_post_data
-            )
 
     def __repr__(self) -> str:
         return f"<Request url={self.url!r} method={self.method!r}>"
@@ -159,9 +154,12 @@ async def sizes(self) -> RequestSizes:
     @property
     def post_data(self) -> Optional[str]:
         data = self._fallback_overrides.post_data_buffer
-        if not data:
-            return None
-        return data.decode()
+        if data:
+            return data.decode()
+        base64_post_data = self._initializer.get("postData")
+        if base64_post_data is not None:
+            return base64.b64decode(base64_post_data).decode()
+        return None
 
     @property
     def post_data_json(self) -> Optional[Any]:

tests/async/test_browsertype_connect.py

@@ -428,11 +428,15 @@ async def test_should_upload_a_folder(
     (dir / "sub-dir").mkdir()
     (dir / "sub-dir" / "really.txt").write_text("sub-dir file content")
     await input.set_input_files(dir)
-    assert await input.evaluate("e => [...e.files].map(f => f.webkitRelativePath)") == [
-        "file-upload-test/file1.txt",
-        "file-upload-test/file2",
-        "file-upload-test/sub-dir/really.txt",
-    ]
+    assert set(
+        await input.evaluate("e => [...e.files].map(f => f.webkitRelativePath)")
+    ) == set(
+        [
+            "file-upload-test/file1.txt",
+            "file-upload-test/file2",
+            "file-upload-test/sub-dir/really.txt",
+        ]
+    )
     webkit_relative_paths = await input.evaluate(
         "e => [...e.files].map(f => f.webkitRelativePath)"
     )

tests/async/test_defaultbrowsercontext.py

@@ -83,7 +83,14 @@ async def test_context_add_cookies_should_work(
     (page, context) = await launch_persistent()
     await page.goto(server.EMPTY_PAGE)
     await page.context.add_cookies(
-        [{"url": server.EMPTY_PAGE, "name": "username", "value": "John Doe"}]
+        [
+            {
+                "url": server.EMPTY_PAGE,
+                "name": "username",
+                "value": "John Doe",
+                "sameSite": "Lax",
+            }
+        ]
     )
     assert await page.evaluate("() => document.cookie") == "username=John Doe"
     assert await page.context.cookies() == [

tests/async/test_input.py

@@ -427,11 +427,15 @@ async def test_should_upload_a_folder(
     (dir / "sub-dir").mkdir()
     (dir / "sub-dir" / "really.txt").write_text("sub-dir file content")
     await input.set_input_files(dir)
-    assert await input.evaluate("e => [...e.files].map(f => f.webkitRelativePath)") == [
-        "file-upload-test/file1.txt",
-        "file-upload-test/file2",
-        "file-upload-test/sub-dir/really.txt",
-    ]
+    assert set(
+        await input.evaluate("e => [...e.files].map(f => f.webkitRelativePath)")
+    ) == (
+        [
+            "file-upload-test/file1.txt",
+            "file-upload-test/file2",
+            "file-upload-test/sub-dir/really.txt",
+        ]
+    )
     webkit_relative_paths = await input.evaluate(
         "e => [...e.files].map(f => f.webkitRelativePath)"
     )

tests/async/test_request_continue.py

@@ -16,7 +16,7 @@
 from typing import Optional
 
 from playwright.async_api import Page, Route
-from tests.server import Server
+from tests.server import Server, TestServerRequest
 
 
 async def test_request_continue_should_work(page: Page, server: Server) -> None:
@@ -145,3 +145,100 @@ async def test_should_amend_binary_post_data(page: Page, server: Server) -> None
     )
     assert server_request.method == b"POST"
     assert server_request.post_body == b"\x00\x01\x02\x03\x04"
+
+
+# it('continue should not change multipart/form-data body', async ({ page, server, browserName }) => {
+#   it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/19158' });
+#   await page.goto(server.EMPTY_PAGE);
+#   server.setRoute('/upload', (request, response) => {
+#     response.writeHead(200, { 'Content-Type': 'text/plain' });
+#     response.end('done');
+#   });
+#   async function sendFormData() {
+#     const reqPromise = server.waitForRequest('/upload');
+#     const status = await page.evaluate(async () => {
+#       const newFile = new File(['file content'], 'file.txt');
+#       const formData = new FormData();
+#       formData.append('file', newFile);
+#       const response = await fetch('/upload', {
+#         method: 'POST',
+#         credentials: 'include',
+#         body: formData,
+#       });
+#       return response.status;
+#     });
+#     const req = await reqPromise;
+#     expect(status).toBe(200);
+#     return req;
+#   }
+#   const reqBefore = await sendFormData();
+#   await page.route('**/*', async route => {
+#     await route.continue();
+#   });
+#   const reqAfter = await sendFormData();
+#   const fileContent = [
+#     'Content-Disposition: form-data; name=\"file\"; filename=\"file.txt\"',
+#     'Content-Type: application/octet-stream',
+#     '',
+#     'file content',
+#     '------'].join('\r\n');
+#   expect.soft((await reqBefore.postBody).toString('utf8')).toContain(fileContent);
+#   expect.soft((await reqAfter.postBody).toString('utf8')).toContain(fileContent);
+#   // Firefox sends a bit longer boundary.
+#   const expectedLength = browserName === 'firefox' ? '246' : '208';
+#   expect.soft(reqBefore.headers['content-length']).toBe(expectedLength);
+#   expect.soft(reqAfter.headers['content-length']).toBe(expectedLength);
+# });
+
+
+async def test_continue_should_not_change_multipart_form_data_body(
+    page: Page, server: Server, browser_name: str
+) -> None:
+    await page.goto(server.EMPTY_PAGE)
+    server.set_route(
+        "/upload",
+        lambda context: (
+            context.write(b"done"),
+            context.setHeader("Content-Type", "text/plain"),
+            context.finish(),
+        ),
+    )
+
+    async def send_form_data() -> TestServerRequest:
+        req_task = asyncio.create_task(server.wait_for_request("/upload"))
+        status = await page.evaluate(
+            """async () => {
+            const newFile = new File(['file content'], 'file.txt');
+            const formData = new FormData();
+            formData.append('file', newFile);
+            const response = await fetch('/upload', {
+                method: 'POST',
+                credentials: 'include',
+                body: formData,
+            });
+            return response.status;
+        }"""
+        )
+        req = await req_task
+        assert status == 200
+        return req
+
+    req_before = await send_form_data()
+    await page.route("**/*", lambda route: route.continue_())
+    req_after = await send_form_data()
+
+    file_content = (
+        'Content-Disposition: form-data; name="file"; filename="file.txt"\r\n'
+        "Content-Type: application/octet-stream\r\n"
+        "\r\n"
+        "file content\r\n"
+        "------"
+    )
+    assert req_before.post_body
+    assert req_after.post_body
+    assert file_content in req_before.post_body.decode()
+    assert file_content in req_after.post_body.decode()
+    # Firefox sends a bit longer boundary.
+    expected_length = "246" if browser_name == "firefox" else "208"
+    assert req_before.getHeader("content-length") == expected_length
+    assert req_after.getHeader("content-length") == expected_length