-
Notifications
You must be signed in to change notification settings - Fork 2
release: 0.22.0 #623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
release: 0.22.0 #623
Changes from all commits
3679fc3
d359d17
7e02151
dc9d512
0f01051
0ecf1e6
757aae0
e96f24b
95c72a9
e23a0d8
16d4263
e942e18
6f06faf
5c4116d
c8199ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| .prism.log | ||
| .stdy.log | ||
| _dev | ||
|
|
||
| __pycache__ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| { | ||
| ".": "0.21.0" | ||
| ".": "0.22.0" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import re | ||
| from typing import ( | ||
| Any, | ||
| Mapping, | ||
| Callable, | ||
| ) | ||
| from urllib.parse import quote | ||
|
|
||
| # Matches '.' or '..' where each dot is either literal or percent-encoded (%2e / %2E). | ||
| _DOT_SEGMENT_RE = re.compile(r"^(?:\.|%2[eE]){1,2}$") | ||
|
|
||
| _PLACEHOLDER_RE = re.compile(r"\{(\w+)\}") | ||
|
|
||
|
|
||
| def _quote_path_segment_part(value: str) -> str: | ||
| """Percent-encode `value` for use in a URI path segment. | ||
|
|
||
| Considers characters not in `pchar` set from RFC 3986 §3.3 to be unsafe. | ||
| https://datatracker.ietf.org/doc/html/rfc3986#section-3.3 | ||
| """ | ||
| # quote() already treats unreserved characters (letters, digits, and -._~) | ||
| # as safe, so we only need to add sub-delims, ':', and '@'. | ||
| # Notably, unlike the default `safe` for quote(), / is unsafe and must be quoted. | ||
| return quote(value, safe="!$&'()*+,;=:@") | ||
|
|
||
|
|
||
| def _quote_query_part(value: str) -> str: | ||
| """Percent-encode `value` for use in a URI query string. | ||
|
|
||
| Considers &, = and characters not in `query` set from RFC 3986 §3.4 to be unsafe. | ||
| https://datatracker.ietf.org/doc/html/rfc3986#section-3.4 | ||
| """ | ||
| return quote(value, safe="!$'()*+,;:@/?") | ||
|
|
||
|
|
||
| def _quote_fragment_part(value: str) -> str: | ||
| """Percent-encode `value` for use in a URI fragment. | ||
|
|
||
| Considers characters not in `fragment` set from RFC 3986 §3.5 to be unsafe. | ||
| https://datatracker.ietf.org/doc/html/rfc3986#section-3.5 | ||
| """ | ||
| return quote(value, safe="!$&'()*+,;=:@/?") | ||
|
|
||
|
|
||
| def _interpolate( | ||
| template: str, | ||
| values: Mapping[str, Any], | ||
| quoter: Callable[[str], str], | ||
| ) -> str: | ||
| """Replace {name} placeholders in `template`, quoting each value with `quoter`. | ||
|
|
||
| Placeholder names are looked up in `values`. | ||
|
|
||
| Raises: | ||
| KeyError: If a placeholder is not found in `values`. | ||
| """ | ||
| # re.split with a capturing group returns alternating | ||
| # [text, name, text, name, ..., text] elements. | ||
| parts = _PLACEHOLDER_RE.split(template) | ||
|
|
||
| for i in range(1, len(parts), 2): | ||
| name = parts[i] | ||
| if name not in values: | ||
| raise KeyError(f"a value for placeholder {{{name}}} was not provided") | ||
| val = values[name] | ||
| if val is None: | ||
| parts[i] = "null" | ||
| elif isinstance(val, bool): | ||
| parts[i] = "true" if val else "false" | ||
| else: | ||
| parts[i] = quoter(str(values[name])) | ||
|
|
||
| return "".join(parts) | ||
|
|
||
|
|
||
| def path_template(template: str, /, **kwargs: Any) -> str: | ||
| """Interpolate {name} placeholders in `template` from keyword arguments. | ||
|
|
||
| Args: | ||
| template: The template string containing {name} placeholders. | ||
| **kwargs: Keyword arguments to interpolate into the template. | ||
|
|
||
| Returns: | ||
| The template with placeholders interpolated and percent-encoded. | ||
|
|
||
| Safe characters for percent-encoding are dependent on the URI component. | ||
| Placeholders in path and fragment portions are percent-encoded where the `segment` | ||
| and `fragment` sets from RFC 3986 respectively are considered safe. | ||
| Placeholders in the query portion are percent-encoded where the `query` set from | ||
| RFC 3986 §3.3 is considered safe except for = and & characters. | ||
|
|
||
| Raises: | ||
| KeyError: If a placeholder is not found in `kwargs`. | ||
| ValueError: If resulting path contains /./ or /../ segments (including percent-encoded dot-segments). | ||
| """ | ||
| # Split the template into path, query, and fragment portions. | ||
| fragment_template: str | None = None | ||
| query_template: str | None = None | ||
|
|
||
|
Comment on lines
+99
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Inside Extended reasoning...In path_template = restThis assigns a local string variable with the exact same name as the enclosing function. In Python, a name is determined to be local to a function at compile time - if Python sees any assignment to The specific code path is: the function splits the input template into path, query, and fragment portions ( The existing code is safe only because: (1) the function is not recursive and does not need to call itself, and (2) the local variable However, this is an error-prone pattern with real maintenance risks. If a developer later adds code inside this function body and writes Step-by-step proof:
Fix: rename the local variable to path_str = rest
path_result = _interpolate(path_str, kwargs, _quote_path_segment_part) |
||
| rest = template | ||
| if "#" in rest: | ||
| rest, fragment_template = rest.split("#", 1) | ||
| if "?" in rest: | ||
| rest, query_template = rest.split("?", 1) | ||
| path_template = rest | ||
|
|
||
| # Interpolate each portion with the appropriate quoting rules. | ||
| path_result = _interpolate(path_template, kwargs, _quote_path_segment_part) | ||
|
|
||
| # Reject dot-segments (. and ..) in the final assembled path. The check | ||
| # runs after interpolation so that adjacent placeholders or a mix of static | ||
| # text and placeholders that together form a dot-segment are caught. | ||
| # Also reject percent-encoded dot-segments to protect against incorrectly | ||
| # implemented normalization in servers/proxies. | ||
| for segment in path_result.split("/"): | ||
| if _DOT_SEGMENT_RE.match(segment): | ||
| raise ValueError(f"Constructed path {path_result!r} contains dot-segment {segment!r} which is not allowed") | ||
|
|
||
| result = path_result | ||
| if query_template is not None: | ||
| result += "?" + _interpolate(query_template, kwargs, _quote_query_part) | ||
| if fragment_template is not None: | ||
| result += "#" + _interpolate(fragment_template, kwargs, _quote_fragment_part) | ||
|
|
||
| return result | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| # File generated from our OpenAPI spec by Stainless. See CONTRIBUTING.md for details. | ||
|
|
||
| __title__ = "openlayer" | ||
| __version__ = "0.21.0" # x-release-please-version | ||
| __version__ = "0.22.0" # x-release-please-version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 The
_ModelDumpKwargsdict built inmodel_dumpis never spread intomodel.model_dump()— the explicitby_alias=bool(by_alias) if by_alias is not None else Falseon line 159 remains, soby_alias=Falseis unconditionally passed to pydantic when the caller omits the argument. This completely defeats the stated fix ('pydantic: do not pass by_alias unless set') and silently overrides model-level alias_generator defaults in pydantic v2/v3; the fix is to replace the explicit by_alias kwarg with **kwargs.Extended reasoning...
What the bug is
In
src/openlayer/_compat.py, the PR introduces a_ModelDumpKwargsTypedDict and, inside the pydantic-v2 branch ofmodel_dump, conditionally populates it:However, the subsequent
model.model_dump()call still contains the old explicit argument:The
kwargsdict is built but never spread —**kwargsis absent. It is completely dead code.The specific code path that triggers it
Every call to
model_dump(model)(the default case,by_alias=None) hits the pydantic v2 branch, builds an emptykwargs, and then callsmodel.model_dump(..., by_alias=False)becausebool(by_alias) if by_alias is not None else Falseevaluates toFalse.Why existing code does not prevent it
The
kwargsTypedDict construction looks correct in isolation, which makes the bug easy to miss. The explicitby_alias=...keyword was never removed from themodel.model_dump()call site when**kwargswas supposed to replace it. There are no tests verifying thatby_aliasis absent from forwarded kwargs when not explicitly set, so the regression went undetected.Impact
Pydantic v2 models that rely on model-level alias configuration (e.g.,
model_config = ConfigDict(alias_generator=..., populate_by_name=True)) will have their alias behavior silently overridden whenevermodel_dumpis called without an explicitby_aliasargument. Fields will be serialized using their Python attribute names instead of the configured aliases, potentially breaking serialization for any SDK model that uses aliased field names.How to fix it
Remove the explicit
by_alias=bool(by_alias) if by_alias is not None else Falsekwarg and replace with**kwargs:Step-by-step proof
model_dump(my_model)with noby_aliasargument — defaults toNone.if (not PYDANTIC_V1) or hasattr(model, "model_dump"):branch.kwargs: _ModelDumpKwargs = {}is created; theif by_alias is not None:guard isFalse, sokwargsremains{}.model.model_dump(..., by_alias=bool(None) if None is not None else False)simplifies tomodel.model_dump(..., by_alias=False).by_alias=False, overriding any model-levelalias_generatordefault.kwargsdict is never referenced again and is garbage-collected at end of scope.