Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions py/selenium/webdriver/common/api_request_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import urllib3
from urllib3.util.retry import Retry

from selenium.webdriver.common.cookie import Cookie

if TYPE_CHECKING:
from selenium.webdriver.remote.webdriver import WebDriver

Expand Down Expand Up @@ -98,7 +100,7 @@ def dispose(self) -> None:
self._body = b""


def _cookie_matches(cookie: dict, url: str, default_domain: str = "") -> bool:
def _cookie_matches(cookie: Cookie, url: str, default_domain: str = "") -> bool:
"""Check if a browser cookie should be sent with a request to the given URL.

Evaluates expiry, domain, path, and secure attribute matching per RFC 6265.
Expand Down Expand Up @@ -151,7 +153,7 @@ def _cookie_matches(cookie: dict, url: str, default_domain: str = "") -> bool:
return True


def _parse_set_cookie(header_value: str) -> dict:
def _parse_set_cookie(header_value: str) -> Cookie:
"""Parse a single Set-Cookie header value into a cookie dict.

Uses manual parsing instead of http.cookies.SimpleCookie which is too
Expand All @@ -171,7 +173,7 @@ def _parse_set_cookie(header_value: str) -> dict:
name = name_value[:eq_idx].strip()
value = name_value[eq_idx + 1 :].strip()

cookie: dict[str, Any] = {"name": name, "value": value}
cookie: Cookie = {"name": name, "value": value}
has_max_age = False

for part in parts[1:]:
Expand Down Expand Up @@ -455,7 +457,7 @@ def _build_response(self, resp: urllib3.BaseHTTPResponse, url: str) -> APIRespon
body=resp.data,
)

def _get_cookies_for_request(self, url: str) -> list[dict]:
def _get_cookies_for_request(self, url: str) -> list[Cookie]:
"""Get cookies that should be sent with the request. Overridden by subclasses."""
return []

Expand Down Expand Up @@ -558,7 +560,7 @@ def new_context(
Returns:
An _IsolatedAPIRequestContext instance.
"""
cookies: list[dict] = []
cookies: list[Cookie] = []
if storage_state is not None:
if isinstance(storage_state, (str, pathlib.Path)):
file_path = pathlib.Path(storage_state)
Expand Down Expand Up @@ -604,7 +606,7 @@ def get_storage_state(self, path: str | pathlib.Path | None = None) -> dict[str,
raise OSError(f"Cannot write storage state to {file_path}: {e}") from e
return state

def _get_cookies_for_request(self, url: str) -> list[dict]:
def _get_cookies_for_request(self, url: str) -> list[Cookie]:
"""Get matching browser cookies for the request URL."""
try:
browser_cookies = self._driver.get_cookies()
Expand Down Expand Up @@ -657,7 +659,7 @@ def __init__(
self,
base_url: str = "",
extra_headers: dict[str, str] | None = None,
cookies: list[dict] | None = None,
cookies: list[Cookie] | None = None,
timeout: float = 30.0,
max_redirects: int = 10,
fail_on_status_code: bool = False,
Expand All @@ -669,13 +671,13 @@ def __init__(
max_redirects=max_redirects,
fail_on_status_code=fail_on_status_code,
)
self._cookies: list[dict] = cookies or []
self._cookies: list[Cookie] = cookies or []

def get_storage_state(self) -> dict[str, Any]:
"""Return the current cookies as a storage state dict."""
return {"cookies": list(self._cookies)}

def _get_cookies_for_request(self, url: str) -> list[dict]:
def _get_cookies_for_request(self, url: str) -> list[Cookie]:
"""Get matching cookies from the internal jar."""
# For isolated contexts, use the request hostname as default domain
default_domain = urllib.parse.urlparse(url).hostname or ""
Expand Down
31 changes: 31 additions & 0 deletions py/selenium/webdriver/common/cookie.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Licensed to the Software Freedom Conservancy (SFC) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The SFC licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

from typing_extensions import NotRequired, TypedDict


class Cookie(TypedDict, total=False):
"""A WebDriver cookie dictionary."""

name: str
value: str
path: NotRequired[str]
domain: NotRequired[str]
secure: NotRequired[bool]
httpOnly: NotRequired[bool]
expiry: NotRequired[int]
sameSite: NotRequired[str]
Comment on lines +21 to +31
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Cookie missing required keys 🐞 Bug ≡ Correctness

Cookie is declared with TypedDict(total=False), making name and value optional and allowing
{} to type-check as a Cookie. This defeats the goal of cookie typing and can let invalid cookies
flow into WebDriver.add_cookie(), which requires name/value and will fail at runtime when
given incomplete data.
Agent Prompt
## Issue description
`Cookie` is currently defined as `TypedDict(total=False)`, which makes **all** keys optional (including `name` and `value`). This allows invalid cookies (including `{}`) to type-check, undermining the purpose of introducing the shared cookie type.

## Issue Context
- `WebDriver.add_cookie()` documents that `name` and `value` are required.
- `_parse_set_cookie()` currently returns `{}` on parse failure while being annotated to return `Cookie`, which only type-checks because `Cookie` has no required keys.

## Fix Focus Areas
- py/selenium/webdriver/common/cookie.py[21-31]
- py/selenium/webdriver/common/api_request_context.py[156-216]
- py/selenium/webdriver/remote/webdriver.py[741-760]

## Proposed fix
1. Make `Cookie` require `name` and `value` by removing `total=False` (default `total=True`) and keeping the other keys as `NotRequired[...]`.
2. Update `_parse_set_cookie()` to return `Cookie | None` (or introduce a separate permissive type like `ParsedCookie` / `PartialCookie`) and adjust callers to handle the non-cookie case without relying on `{}` being a `Cookie`.
3. Ensure any code path calling `add_cookie()` only passes a `Cookie` that includes `name` and `value`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

7 changes: 4 additions & 3 deletions py/selenium/webdriver/remote/webdriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
from selenium.webdriver.common.bidi.storage import Storage
from selenium.webdriver.common.bidi.webextension import WebExtension
from selenium.webdriver.common.by import By
from selenium.webdriver.common.cookie import Cookie
from selenium.webdriver.common.fedcm.dialog import Dialog
from selenium.webdriver.common.options import ArgOptions, BaseOptions
from selenium.webdriver.common.print_page_options import PrintOptions
Expand Down Expand Up @@ -689,7 +690,7 @@ def refresh(self) -> None:
"""Refreshes the current page."""
self.execute(Command.REFRESH)

def get_cookies(self) -> list[dict]:
def get_cookies(self) -> list[Cookie]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. get_cookies() return type narrowed 📘 Rule violation ⚙ Maintainability

The public WebDriver cookie APIs now use Cookie/list[Cookie] instead of dict/list[dict],
which is not type-compatible for existing typed callers (e.g., list invariance makes
list[Cookie] not assignable to list[dict]). This can force downstream users to change type
annotations/code to satisfy type checkers, violating compatibility expectations for user-facing
APIs.
Agent Prompt
## Issue description
Public cookie-related WebDriver APIs were re-annotated from `dict`/`list[dict]` to `Cookie`/`list[Cookie]`. This is a user-facing interface change that can break downstream static type checking (notably because `list` is invariant), requiring callers to update their types.

## Issue Context
This PR’s goal is better cookie typing, but the compliance requirement is to keep user-facing APIs backward-compatible. Consider using a more compatible annotation strategy (e.g., keeping return/param types as `dict[str, Any]` / `Mapping[str, Any]` in the public API while using `Cookie` internally or via helper casts/types), so existing typed call sites don’t require changes.

## Fix Focus Areas
- py/selenium/webdriver/remote/webdriver.py[693-703]
- py/selenium/webdriver/remote/webdriver.py[741-742]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

"""Get all cookies visible to the current WebDriver instance.
Returns:
Expand All @@ -698,7 +699,7 @@ def get_cookies(self) -> list[dict]:
"""
return self.execute(Command.GET_ALL_COOKIES)["value"]

def get_cookie(self, name) -> dict | None:
def get_cookie(self, name: str) -> Cookie | None:
"""Get a single cookie by name (case-sensitive,).
Returns:
Expand Down Expand Up @@ -737,7 +738,7 @@ def delete_all_cookies(self) -> None:
"""Delete all cookies in the scope of the session."""
self.execute(Command.DELETE_ALL_COOKIES)

def add_cookie(self, cookie_dict) -> None:
def add_cookie(self, cookie_dict: Cookie) -> None:
"""Adds a cookie to your current session.
Args:
Expand Down
Loading