From 8c3e10187d687df44d0ecc7001ba22d1820804a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 29 Jun 2026 11:38:12 +0000 Subject: [PATCH 1/5] fix(py): skip malformed Set-Cookie instead of panicking across FFI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `set_cookies` runs inside reqwest's cookie-store callback, a Rust callback driven by the HTTP stack. The `.unwrap()`s on the cookie constructor and `set_cookie` call meant a hostile/buggy server's malformed `Set-Cookie` (bad domain/expiry that `http.cookiejar.Cookie` rejects), or a custom cookie jar whose `set_cookie` raises, would panic and unwind across the FFI boundary — aborting the host process instead of raising a catchable exception. Mirror the Node wrapper's behavior of skipping cookies the jar rejects, but emit a Python `UserWarning` (falling back to stderr) so the dropped cookie is surfaced rather than silently ignored. Closes #478 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015p2biLkkRLNEs9xWyqdpby --- impit-python/src/cookies.rs | 42 ++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/impit-python/src/cookies.rs b/impit-python/src/cookies.rs index a7dbb079..8249c6b7 100644 --- a/impit-python/src/cookies.rs +++ b/impit-python/src/cookies.rs @@ -91,13 +91,30 @@ impl CookieStore for PythonCookieJar { kwargs.set_item("rest", rest).unwrap_or_default(); - let py_cookie = self.cookie_constructor.call(py, (), Some(&kwargs)).unwrap(); + // A hostile or buggy server can send a `Set-Cookie` that + // `http.cookiejar.Cookie` rejects (e.g. a bad domain/expiry), and a custom + // cookie jar's `set_cookie` may raise too. Skip the offending cookie and warn + // instead of `.unwrap()`ing: a panic here would unwind across the FFI boundary + // and abort the host process rather than raise a catchable Python exception. + let py_cookie = match self.cookie_constructor.call(py, (), Some(&kwargs)) { + Ok(py_cookie) => py_cookie, + Err(err) => { + warn_skipped_cookie(py, cookie.name(), &err); + continue; + } + }; - let args = PyTuple::new(py, vec![py_cookie]).unwrap(); + let args = match PyTuple::new(py, vec![py_cookie]) { + Ok(args) => args, + Err(err) => { + warn_skipped_cookie(py, cookie.name(), &err); + continue; + } + }; - self.cookie_jar - .call_method1(py, "set_cookie", args) - .unwrap(); + if let Err(err) = self.cookie_jar.call_method1(py, "set_cookie", args) { + warn_skipped_cookie(py, cookie.name(), &err); + } } }); } @@ -175,6 +192,21 @@ impl CookieStore for PythonCookieJar { } } +/// Emits a Python `UserWarning` that a `Set-Cookie` was skipped because the cookie jar +/// rejected it, instead of letting the error panic across the FFI boundary. +fn warn_skipped_cookie(py: Python<'_>, cookie_name: &str, err: &PyErr) { + let message = format!("Skipping malformed cookie {cookie_name:?}: {err}"); + + let warned = PyModule::import(py, "warnings") + .and_then(|warnings| warnings.call_method1("warn", (message.as_str(),))); + + // Fall back to stderr if even emitting the warning fails, so the cookie is never + // silently dropped and we still avoid aborting the process. + if warned.is_err() { + eprintln!("{message}"); + } +} + /// Checks whether a request `host` may receive a cookie scoped to `cookie_domain`, /// following the domain matching rules of /// [RFC 6265, §5.1.3](https://www.rfc-editor.org/rfc/rfc6265#section-5.1.3). From d662da9e5e94a4e0a0522986b5bcc61d24ad54d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 29 Jun 2026 12:26:20 +0000 Subject: [PATCH 2/5] fix(py): skip malformed Set-Cookie instead of panicking across FFI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `set_cookies` runs inside reqwest's cookie-store callback, a Rust callback driven by the HTTP stack. The `.unwrap()`s on the cookie constructor and `set_cookie` call meant a hostile/buggy server's malformed `Set-Cookie` (bad domain/expiry that `http.cookiejar.Cookie` rejects), or a custom cookie jar whose `set_cookie` raises, would panic and unwind across the FFI boundary — aborting the host process instead of raising a catchable exception. Skip the offending cookie and continue, ignoring parsing errors silently to match the Node binding's `setCookie` handling in `index.wrapper.js`. Closes #478 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015p2biLkkRLNEs9xWyqdpby --- impit-python/src/cookies.rs | 37 ++++++++----------------------------- 1 file changed, 8 insertions(+), 29 deletions(-) diff --git a/impit-python/src/cookies.rs b/impit-python/src/cookies.rs index 8249c6b7..886a9c0f 100644 --- a/impit-python/src/cookies.rs +++ b/impit-python/src/cookies.rs @@ -93,28 +93,22 @@ impl CookieStore for PythonCookieJar { // A hostile or buggy server can send a `Set-Cookie` that // `http.cookiejar.Cookie` rejects (e.g. a bad domain/expiry), and a custom - // cookie jar's `set_cookie` may raise too. Skip the offending cookie and warn - // instead of `.unwrap()`ing: a panic here would unwind across the FFI boundary - // and abort the host process rather than raise a catchable Python exception. + // cookie jar's `set_cookie` may raise too. Skip the offending cookie instead + // of `.unwrap()`ing: a panic here would unwind across the FFI boundary and + // abort the host process rather than raise a catchable Python exception. + // Cookie parsing errors are ignored silently, matching the Node binding's + // `setCookie` handling in `index.wrapper.js`. let py_cookie = match self.cookie_constructor.call(py, (), Some(&kwargs)) { Ok(py_cookie) => py_cookie, - Err(err) => { - warn_skipped_cookie(py, cookie.name(), &err); - continue; - } + Err(_) => continue, }; let args = match PyTuple::new(py, vec![py_cookie]) { Ok(args) => args, - Err(err) => { - warn_skipped_cookie(py, cookie.name(), &err); - continue; - } + Err(_) => continue, }; - if let Err(err) = self.cookie_jar.call_method1(py, "set_cookie", args) { - warn_skipped_cookie(py, cookie.name(), &err); - } + let _ = self.cookie_jar.call_method1(py, "set_cookie", args); } }); } @@ -192,21 +186,6 @@ impl CookieStore for PythonCookieJar { } } -/// Emits a Python `UserWarning` that a `Set-Cookie` was skipped because the cookie jar -/// rejected it, instead of letting the error panic across the FFI boundary. -fn warn_skipped_cookie(py: Python<'_>, cookie_name: &str, err: &PyErr) { - let message = format!("Skipping malformed cookie {cookie_name:?}: {err}"); - - let warned = PyModule::import(py, "warnings") - .and_then(|warnings| warnings.call_method1("warn", (message.as_str(),))); - - // Fall back to stderr if even emitting the warning fails, so the cookie is never - // silently dropped and we still avoid aborting the process. - if warned.is_err() { - eprintln!("{message}"); - } -} - /// Checks whether a request `host` may receive a cookie scoped to `cookie_domain`, /// following the domain matching rules of /// [RFC 6265, §5.1.3](https://www.rfc-editor.org/rfc/rfc6265#section-5.1.3). From ed2c2daf55191b1c4943c6fbe464e706fa6a168f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 29 Jun 2026 13:04:49 +0000 Subject: [PATCH 3/5] test(py): malformed cookie is skipped, not a process abort Add a regression test (sync and async) for #478: a custom cookie jar whose `set_cookie` raises must not abort the interpreter. The request completes, the offending cookie is skipped, and a valid cookie in the same response is still stored. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015p2biLkkRLNEs9xWyqdpby --- impit-python/test/async_client_test.py | 41 +++++++++++++++++++++++++- impit-python/test/basic_client_test.py | 40 ++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/impit-python/test/async_client_test.py b/impit-python/test/async_client_test.py index 8a0b11b5..0dbd44ef 100644 --- a/impit-python/test/async_client_test.py +++ b/impit-python/test/async_client_test.py @@ -2,7 +2,7 @@ import json import socket import threading -from http.cookiejar import CookieJar +from http.cookiejar import Cookie, CookieJar from typing import Literal import pytest @@ -192,6 +192,45 @@ async def test_complex_cookies(self, browser: Browser) -> None: # but it's ok - https://www.rfc-editor.org/rfc/rfc6265#section-4.1.2.3 assert cookie.domain == '127.0.0.1' + @pytest.mark.asyncio + async def test_malformed_cookie_is_skipped_not_crashing(self, browser: Browser) -> None: + """A cookie the jar rejects is skipped instead of aborting the process. + + Regression test for https://github.com/apify/impit/issues/478: `set_cookies` + runs inside reqwest's cookie-store callback, so a raising `set_cookie` used to + `.unwrap()` into a Rust panic that unwound across the FFI boundary and aborted + the host process. It must now be caught and the offending cookie skipped, while + valid cookies in the same response are still stored. + """ + + class RejectingCookieJar(CookieJar): + def set_cookie(self, cookie: Cookie) -> None: + if cookie.name == 'bad': + raise ValueError('cookie jar rejects this cookie') + super().set_cookie(cookie) + + cookies_jar = RejectingCookieJar() + + impit = AsyncClient(browser=browser, cookie_jar=cookies_jar, follow_redirects=True) + + url = get_httpbin_url( + '/response-headers', + query={ + 'set-cookie': [ + 'bad=1; Path=/', + 'good=2; Path=/', + ] + }, + ) + + # The request must return normally rather than aborting the interpreter. + response = await impit.get(url) + assert response.status_code == 200 + + names = {cookie.name for cookie in cookies_jar} + assert 'bad' not in names # the rejected cookie was skipped + assert 'good' in names # a valid cookie in the same response is still stored + @pytest.mark.asyncio async def test_cookie_jar_works(self, browser: Browser) -> None: cookies = Cookies({'preset-cookie': '123'}) diff --git a/impit-python/test/basic_client_test.py b/impit-python/test/basic_client_test.py index 7da84e07..ea8a5c83 100644 --- a/impit-python/test/basic_client_test.py +++ b/impit-python/test/basic_client_test.py @@ -2,7 +2,7 @@ import socket import threading import time -from http.cookiejar import CookieJar +from http.cookiejar import Cookie, CookieJar from typing import Literal import pytest @@ -203,6 +203,44 @@ def test_complex_cookies(self, browser: Browser) -> None: # but it's ok - https://www.rfc-editor.org/rfc/rfc6265#section-4.1.2.3 assert cookie.domain == '127.0.0.1' + def test_malformed_cookie_is_skipped_not_crashing(self, browser: Browser) -> None: + """A cookie the jar rejects is skipped instead of aborting the process. + + Regression test for https://github.com/apify/impit/issues/478: `set_cookies` + runs inside reqwest's cookie-store callback, so a raising `set_cookie` used to + `.unwrap()` into a Rust panic that unwound across the FFI boundary and aborted + the host process. It must now be caught and the offending cookie skipped, while + valid cookies in the same response are still stored. + """ + + class RejectingCookieJar(CookieJar): + def set_cookie(self, cookie: Cookie) -> None: + if cookie.name == 'bad': + raise ValueError('cookie jar rejects this cookie') + super().set_cookie(cookie) + + cookies_jar = RejectingCookieJar() + + impit = Client(browser=browser, cookie_jar=cookies_jar, follow_redirects=True) + + url = get_httpbin_url( + '/response-headers', + query={ + 'set-cookie': [ + 'bad=1; Path=/', + 'good=2; Path=/', + ] + }, + ) + + # The request must return normally rather than aborting the interpreter. + response = impit.get(url) + assert response.status_code == 200 + + names = {cookie.name for cookie in cookies_jar} + assert 'bad' not in names # the rejected cookie was skipped + assert 'good' in names # a valid cookie in the same response is still stored + def test_cookie_jar_works(self, browser: Browser) -> None: cookies = Cookies({'preset-cookie': '123'}) From fbb575f84557fa582f8357408022d3d3f5e6489e Mon Sep 17 00:00:00 2001 From: Josef Prochazka Date: Mon, 29 Jun 2026 15:23:10 +0200 Subject: [PATCH 4/5] Polish --- impit-python/src/cookies.rs | 8 +------- impit-python/test/async_client_test.py | 18 ++++-------------- impit-python/test/basic_client_test.py | 18 ++++-------------- impit-python/uv.lock | 2 +- 4 files changed, 10 insertions(+), 36 deletions(-) diff --git a/impit-python/src/cookies.rs b/impit-python/src/cookies.rs index 886a9c0f..7bf94a16 100644 --- a/impit-python/src/cookies.rs +++ b/impit-python/src/cookies.rs @@ -91,13 +91,7 @@ impl CookieStore for PythonCookieJar { kwargs.set_item("rest", rest).unwrap_or_default(); - // A hostile or buggy server can send a `Set-Cookie` that - // `http.cookiejar.Cookie` rejects (e.g. a bad domain/expiry), and a custom - // cookie jar's `set_cookie` may raise too. Skip the offending cookie instead - // of `.unwrap()`ing: a panic here would unwind across the FFI boundary and - // abort the host process rather than raise a catchable Python exception. - // Cookie parsing errors are ignored silently, matching the Node binding's - // `setCookie` handling in `index.wrapper.js`. + // Cookie parsing errors are ignored silently. let py_cookie = match self.cookie_constructor.call(py, (), Some(&kwargs)) { Ok(py_cookie) => py_cookie, Err(_) => continue, diff --git a/impit-python/test/async_client_test.py b/impit-python/test/async_client_test.py index 0dbd44ef..919e3d6d 100644 --- a/impit-python/test/async_client_test.py +++ b/impit-python/test/async_client_test.py @@ -193,20 +193,13 @@ async def test_complex_cookies(self, browser: Browser) -> None: assert cookie.domain == '127.0.0.1' @pytest.mark.asyncio - async def test_malformed_cookie_is_skipped_not_crashing(self, browser: Browser) -> None: - """A cookie the jar rejects is skipped instead of aborting the process. - - Regression test for https://github.com/apify/impit/issues/478: `set_cookies` - runs inside reqwest's cookie-store callback, so a raising `set_cookie` used to - `.unwrap()` into a Rust panic that unwound across the FFI boundary and aborted - the host process. It must now be caught and the offending cookie skipped, while - valid cookies in the same response are still stored. - """ + async def test_rejected_cookie_is_skipped_not_crashing(self, browser: Browser) -> None: + """A cookie jar rejects are skipped instead of aborting the process.""" class RejectingCookieJar(CookieJar): def set_cookie(self, cookie: Cookie) -> None: if cookie.name == 'bad': - raise ValueError('cookie jar rejects this cookie') + raise ValueError('simulate parsing error') super().set_cookie(cookie) cookies_jar = RejectingCookieJar() @@ -223,13 +216,10 @@ def set_cookie(self, cookie: Cookie) -> None: }, ) - # The request must return normally rather than aborting the interpreter. response = await impit.get(url) assert response.status_code == 200 - names = {cookie.name for cookie in cookies_jar} - assert 'bad' not in names # the rejected cookie was skipped - assert 'good' in names # a valid cookie in the same response is still stored + assert {'good'} == {cookie.name for cookie in cookies_jar} @pytest.mark.asyncio async def test_cookie_jar_works(self, browser: Browser) -> None: diff --git a/impit-python/test/basic_client_test.py b/impit-python/test/basic_client_test.py index ea8a5c83..6a42e794 100644 --- a/impit-python/test/basic_client_test.py +++ b/impit-python/test/basic_client_test.py @@ -203,20 +203,13 @@ def test_complex_cookies(self, browser: Browser) -> None: # but it's ok - https://www.rfc-editor.org/rfc/rfc6265#section-4.1.2.3 assert cookie.domain == '127.0.0.1' - def test_malformed_cookie_is_skipped_not_crashing(self, browser: Browser) -> None: - """A cookie the jar rejects is skipped instead of aborting the process. - - Regression test for https://github.com/apify/impit/issues/478: `set_cookies` - runs inside reqwest's cookie-store callback, so a raising `set_cookie` used to - `.unwrap()` into a Rust panic that unwound across the FFI boundary and aborted - the host process. It must now be caught and the offending cookie skipped, while - valid cookies in the same response are still stored. - """ + def test_rejected_cookie_is_skipped_not_crashing(self, browser: Browser) -> None: + """A cookie jar rejects are skipped instead of aborting the process.""" class RejectingCookieJar(CookieJar): def set_cookie(self, cookie: Cookie) -> None: if cookie.name == 'bad': - raise ValueError('cookie jar rejects this cookie') + raise ValueError('simulate parsing error') super().set_cookie(cookie) cookies_jar = RejectingCookieJar() @@ -233,13 +226,10 @@ def set_cookie(self, cookie: Cookie) -> None: }, ) - # The request must return normally rather than aborting the interpreter. response = impit.get(url) assert response.status_code == 200 - names = {cookie.name for cookie in cookies_jar} - assert 'bad' not in names # the rejected cookie was skipped - assert 'good' in names # a valid cookie in the same response is still stored + assert {'good'} == {cookie.name for cookie in cookies_jar} def test_cookie_jar_works(self, browser: Browser) -> None: cookies = Cookies({'preset-cookie': '123'}) diff --git a/impit-python/uv.lock b/impit-python/uv.lock index 2ab19201..85b06d06 100644 --- a/impit-python/uv.lock +++ b/impit-python/uv.lock @@ -388,7 +388,7 @@ wheels = [ [[package]] name = "impit" -version = "0.12.0" +version = "0.13.1" source = { editable = "." } [package.dev-dependencies] From c8486ac4347a0ed6a966294ac11522defa32cf41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20Proch=C3=A1zka?= Date: Tue, 30 Jun 2026 13:45:30 +0200 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- impit-python/src/cookies.rs | 2 +- impit-python/test/async_client_test.py | 2 +- impit-python/test/basic_client_test.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/impit-python/src/cookies.rs b/impit-python/src/cookies.rs index 7bf94a16..04002c9a 100644 --- a/impit-python/src/cookies.rs +++ b/impit-python/src/cookies.rs @@ -91,7 +91,7 @@ impl CookieStore for PythonCookieJar { kwargs.set_item("rest", rest).unwrap_or_default(); - // Cookie parsing errors are ignored silently. + // Malformed cookies and cookie-jar insertion errors are ignored silently. let py_cookie = match self.cookie_constructor.call(py, (), Some(&kwargs)) { Ok(py_cookie) => py_cookie, Err(_) => continue, diff --git a/impit-python/test/async_client_test.py b/impit-python/test/async_client_test.py index 919e3d6d..cb5c837a 100644 --- a/impit-python/test/async_client_test.py +++ b/impit-python/test/async_client_test.py @@ -194,7 +194,7 @@ async def test_complex_cookies(self, browser: Browser) -> None: @pytest.mark.asyncio async def test_rejected_cookie_is_skipped_not_crashing(self, browser: Browser) -> None: - """A cookie jar rejects are skipped instead of aborting the process.""" + """Cookies rejected by the cookie jar are skipped instead of aborting the process.""" class RejectingCookieJar(CookieJar): def set_cookie(self, cookie: Cookie) -> None: diff --git a/impit-python/test/basic_client_test.py b/impit-python/test/basic_client_test.py index 6a42e794..f78bffe6 100644 --- a/impit-python/test/basic_client_test.py +++ b/impit-python/test/basic_client_test.py @@ -204,7 +204,7 @@ def test_complex_cookies(self, browser: Browser) -> None: assert cookie.domain == '127.0.0.1' def test_rejected_cookie_is_skipped_not_crashing(self, browser: Browser) -> None: - """A cookie jar rejects are skipped instead of aborting the process.""" + """Cookies rejected by the cookie jar are skipped instead of aborting the process.""" class RejectingCookieJar(CookieJar): def set_cookie(self, cookie: Cookie) -> None: