From 82a3bfc209a7f8ec55c4a26d4a4b5fba8c87cd19 Mon Sep 17 00:00:00 2001 From: Ashraf Ali Date: Fri, 2 Oct 2026 16:26:44 +0600 Subject: [PATCH] fix(fetch): use identity check for data/form/multipart mutual exclusion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mutual-exclusivity validation in APIRequestContext._inner_fetch() used truthiness ((1 if data else 0)), so any falsy but supplied data value ("", b"", 0, [], {}, False) bypassed the check when combined with form or multipart. Body selection then used `data is not None`, which treated the falsy data as supplied and silently dropped the form/multipart payload — the request was sent with an empty/JSON body and no error. Mirror the Node.js client semantics (options.data === undefined ? 0 : 1, packages/playwright-core/src/client/fetch.ts) by counting an option as specified when it is not None. This also rejects data together with an empty form/multipart, matching Node. Empty form/multipart alone remain allowed and send no fields, and data="" alone still sends an empty body. Fixes https://github.com/microsoft/playwright/issues/43059 --- playwright/_impl/_fetch.py | 4 ++- tests/async/test_fetch_global.py | 55 +++++++++++++++++++++++++++++++ tests/sync/test_fetch_global.py | 56 ++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 1 deletion(-) diff --git a/playwright/_impl/_fetch.py b/playwright/_impl/_fetch.py index 6181f22b2..38c7205a0 100644 --- a/playwright/_impl/_fetch.py +++ b/playwright/_impl/_fetch.py @@ -369,7 +369,9 @@ async def _inner_fetch( if self._close_reason: raise TargetClosedError(self._close_reason) assert ( - (1 if data else 0) + (1 if form else 0) + (1 if multipart else 0) + (0 if data is None else 1) + + (0 if form is None else 1) + + (0 if multipart is None else 1) ) <= 1, "Only one of 'data', 'form' or 'multipart' can be specified" assert ( maxRedirects is None or maxRedirects >= 0 diff --git a/tests/async/test_fetch_global.py b/tests/async/test_fetch_global.py index 10f82583c..d20e18b36 100644 --- a/tests/async/test_fetch_global.py +++ b/tests/async/test_fetch_global.py @@ -474,6 +474,61 @@ async def test_should_serialize_request_data( await request.dispose() +@pytest.mark.parametrize( + "data", + ["", b"", 0, [], {}, False], +) +async def test_should_disallow_falsy_data_together_with_form_or_multipart( + playwright: Playwright, server: Server, data: Any +) -> None: + server.set_route("/echo", lambda req: (req.write(req.post_body), req.finish())) + request = await playwright.request.new_context() + try: + for body_option in ["form", "multipart"]: + with pytest.raises(AssertionError) as exc_info: + if body_option == "form": + await request.post( + server.PREFIX + "/echo", data=data, form={"name": "value"} + ) + else: + await request.post( + server.PREFIX + "/echo", + data=data, + multipart={"name": "value"}, + ) + assert "Only one of 'data', 'form' or 'multipart' can be specified" in str( + exc_info + ) + finally: + await request.dispose() + + +async def test_should_count_empty_form_and_multipart_as_specified( + playwright: Playwright, server: Server +) -> None: + # Mirrors Node.js semantics: an option counts as specified when it is + # not None, even if it is empty, so it still conflicts with data. + server.set_route("/echo", lambda req: (req.write(req.post_body), req.finish())) + request = await playwright.request.new_context() + try: + with pytest.raises(AssertionError) as exc_info: + await request.post(server.PREFIX + "/echo", data="payload", form={}) + assert "Only one of 'data', 'form' or 'multipart' can be specified" in str( + exc_info + ) + with pytest.raises(AssertionError) as exc_info: + await request.post(server.PREFIX + "/echo", data="payload", multipart={}) + assert "Only one of 'data', 'form' or 'multipart' can be specified" in str( + exc_info + ) + # Empty form/multipart on their own are allowed and send no fields. + response = await request.post(server.PREFIX + "/echo", form={}) + assert response.status == 200 + assert await response.text() == "" + finally: + await request.dispose() + + async def test_should_retry_ECONNRESET(playwright: Playwright, server: Server) -> None: request_count = 0 diff --git a/tests/sync/test_fetch_global.py b/tests/sync/test_fetch_global.py index 15a11fca8..3e38604ea 100644 --- a/tests/sync/test_fetch_global.py +++ b/tests/sync/test_fetch_global.py @@ -14,6 +14,7 @@ import json from pathlib import Path +from typing import Any from urllib.parse import urlparse import pytest @@ -334,6 +335,61 @@ def test_should_serialize_null_values_in_json( request.dispose() +@pytest.mark.parametrize( + "data", + ["", b"", 0, [], {}, False], +) +def test_should_disallow_falsy_data_together_with_form_or_multipart( + playwright: Playwright, server: Server, data: Any +) -> None: + server.set_route("/echo", lambda req: (req.write(req.post_body), req.finish())) + request = playwright.request.new_context() + try: + for body_option in ["form", "multipart"]: + with pytest.raises(AssertionError) as exc_info: + if body_option == "form": + request.post( + server.PREFIX + "/echo", data=data, form={"name": "value"} + ) + else: + request.post( + server.PREFIX + "/echo", + data=data, + multipart={"name": "value"}, + ) + assert "Only one of 'data', 'form' or 'multipart' can be specified" in str( + exc_info + ) + finally: + request.dispose() + + +def test_should_count_empty_form_and_multipart_as_specified( + playwright: Playwright, server: Server +) -> None: + # Mirrors Node.js semantics: an option counts as specified when it is + # not None, even if it is empty, so it still conflicts with data. + server.set_route("/echo", lambda req: (req.write(req.post_body), req.finish())) + request = playwright.request.new_context() + try: + with pytest.raises(AssertionError) as exc_info: + request.post(server.PREFIX + "/echo", data="payload", form={}) + assert "Only one of 'data', 'form' or 'multipart' can be specified" in str( + exc_info + ) + with pytest.raises(AssertionError) as exc_info: + request.post(server.PREFIX + "/echo", data="payload", multipart={}) + assert "Only one of 'data', 'form' or 'multipart' can be specified" in str( + exc_info + ) + # Empty form/multipart on their own are allowed and send no fields. + response = request.post(server.PREFIX + "/echo", form={}) + assert response.status == 200 + assert response.text() == "" + finally: + request.dispose() + + def test_should_throw_when_fail_on_status_code_is_true( playwright: Playwright, server: Server ) -> None: