From 18df40527ea490769df38537077c0168888f757d Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 13 Oct 2026 05:00:58 +0000 Subject: [PATCH 1/3] fix: instance endpoints need an obj_owner and are bounded in size (#30) An instance endpoint (an object exposed through attr_names) is one object every client shares. Like class endpoints (#29), it now needs obj_owner: a (request) -> owner hook that refuses unidentified requests, or the named opt-in obj_owner=shared_objects. Module endpoints are unchanged. max_obj_size / obj_sizeof bound it: checked when the service is built and after every call; once over, the object is kept and calls are refused until it is back under. AuthorizationError now maps to 401 (its subclasses ForbiddenError and OverwritesNotAllowed to 403) in the Bottle and aiohttp error handlers, which share one status table. The aiohttp handler resets current_request. Co-Authored-By: Claude Opus 5.5 --- py2http/default_configs.py | 62 ++++++--- py2http/examples/object_dispatch.py | 5 + py2http/service.py | 124 ++++++++++++++--- py2http/tests/test_attr_dispatch.py | 4 +- py2http/tests/test_instance_endpoints.py | 161 +++++++++++++++++++++++ 5 files changed, 314 insertions(+), 42 deletions(-) create mode 100644 py2http/tests/test_instance_endpoints.py diff --git a/py2http/default_configs.py b/py2http/default_configs.py index 6a03236..8f5139c 100644 --- a/py2http/default_configs.py +++ b/py2http/default_configs.py @@ -23,6 +23,7 @@ InputError, NotFoundError, DuplicateRecordError, + OverwritesNotAllowed, ) from py2http.decorators import handle_json_req, send_json_resp, JsonRespEncoder @@ -59,34 +60,53 @@ def _raise_http_client_error(error, message, reason=None): ) +# HTTP status of each client error, most specific class first. ``AuthorizationError`` +# itself means the request did not say who it is (401); its subclasses mean it did and +# may not do this (403). +_CLIENT_ERROR_STATUSES = ( + ((ForbiddenError, OverwritesNotAllowed), 403), + (AuthorizationError, 401), + ((InputError, DuplicateRecordError), 400), + (NotFoundError, 404), +) +_AIOHTTP_ERRORS = { + 400: web.HTTPBadRequest, + 401: web.HTTPUnauthorized, + 403: web.HTTPForbidden, + 404: web.HTTPNotFound, +} + + +def error_status(error: Exception) -> int: + """The HTTP status for an error a handler raised (500 when it is not a client's). + + >>> error_status(AuthorizationError()), error_status(ForbiddenError()) + (401, 403) + >>> error_status(InputError()), error_status(KeyError()) + (400, 500) + """ + for error_types, status in _CLIENT_ERROR_STATUSES: + if isinstance(error, error_types): + return status + return 500 + + def aiohttp_error_handler(error: Exception): - message = str(error) - if isinstance(error, (AuthorizationError, InputError, DuplicateRecordError)): + status = error_status(error) + if status == 500: _raise_http_client_error( - web.HTTPBadRequest, message, reason=type(error).__name__ + web.HTTPInternalServerError, "Internal server error" ) - elif isinstance(error, ForbiddenError): - _raise_http_client_error(web.HTTPForbidden, message) - elif isinstance(error, NotFoundError): - _raise_http_client_error(web.HTTPNotFound, message) - else: - message = "Internal server error" - _raise_http_client_error(web.HTTPInternalServerError, message) + reason = type(error).__name__ if status == 400 else None + _raise_http_client_error(_AIOHTTP_ERRORS[status], str(error), reason=reason) def bottle_error_handler(error: Exception): message = str(error) - if isinstance(error, (AuthorizationError, InputError, DuplicateRecordError)): - response.status = f"400 {type(error).__name__}" - # response.reason = type(error).__name__ - elif isinstance(error, ForbiddenError): - response.status = 403 - elif isinstance(error, NotFoundError): - response.status = 404 - else: - response.status = 500 - if os.getenv("OPAQUE_ERRORS", None): - message = "Internal server error" + status = error_status(error) + response.status = f"400 {type(error).__name__}" if status == 400 else status + if status == 500 and os.getenv("OPAQUE_ERRORS", None): + message = "Internal server error" return {"error": message} diff --git a/py2http/examples/object_dispatch.py b/py2http/examples/object_dispatch.py index b60ac24..77230da 100644 --- a/py2http/examples/object_dispatch.py +++ b/py2http/examples/object_dispatch.py @@ -11,6 +11,8 @@ from typing import MutableMapping +from py2http.service import shared_objects + class BackendStore(MutableMapping): """MutableMapping wrapper that assumes that values are given as strings, @@ -47,6 +49,9 @@ def __delitem__(self, key: str): endpoint=backend_store, name="backend_store", attr_names=["__iter__", "__getitem__", "__setitem__"], + # One store every client shares and may write to: fine for a demo. A real + # service passes obj_owner=<(request) -> authenticated user id> instead. + obj_owner=shared_objects, ), ] diff --git a/py2http/service.py b/py2http/service.py index 558f258..fa3750e 100644 --- a/py2http/service.py +++ b/py2http/service.py @@ -59,6 +59,10 @@ from py2http.util import TypeAsserter from py2http.constants import JSON_CONTENT_TYPE +# Instance endpoints (``handler['endpoint']`` is an object) are one object every client +# shares; they too need an ``obj_owner`` and are bounded by ``max_obj_size`` (see +# ``_instance_endpoint_dispatcher``). +# # Class endpoints (``handler['endpoint']`` is a class) keep the objects clients create. # Who may use an object is the handler's ``obj_owner`` (see ``ObjOwner``); how many # objects an owner may keep is ``obj_quota``; how many bytes each object, each owner and @@ -475,7 +479,13 @@ def sync_handle_request(req): @handle_error async def aiohttp_handle_request(req): - current_request.set(req) # each aiohttp request runs in its own task context + token = current_request.set(req) + try: + return await _aiohttp_handle(req) + finally: + current_request.reset(token) + + async def _aiohttp_handle(req): inputs = input_mapper(req) if isawaitable(inputs): # Pattern: pass-on async property inputs = await inputs @@ -928,11 +938,14 @@ def _get_func_to_dispatch_handler(handler): if inspect.isclass(endpoint): func = _class_endpoint_dispatcher(endpoint, handler, name, is_dispatchable) - else: + elif isinstance(endpoint, ModuleType): # a module's functions: no shared object def func(_attr_name=None, **kwargs): return _get_attr_value(endpoint, _attr_name, is_dispatchable, **kwargs) + else: + func = _instance_endpoint_dispatcher(endpoint, handler, name, is_dispatchable) + func.__name__ = name return func @@ -968,6 +981,96 @@ def owner_bytes(self, owner): _SHARED_OWNER = "" +def _request_owner(owner_of: ObjOwner, name: str) -> Callable[[], Any]: + """A zero-argument function giving the owner of the request being handled, as + ``owner_of(current_request)`` says, refusing requests it does not identify.""" + if not callable(owner_of): + raise TypeError(f"obj_owner must be callable or shared_objects: {owner_of!r}") + + def owner(): + who = owner_of(current_request.get()) + if who is None or who == "": + raise AuthorizationError(f"{name}: no owner for this request") + try: + hash(who) + except TypeError: + raise TypeError( + f"obj_owner of {name!r} must return a hashable, got {type(who)}" + ) from None + return who + + return owner + + +def _instance_endpoint_dispatcher(obj, handler, name, is_dispatchable): + """The function behind an instance endpoint (``handler['endpoint']`` is an object, + not a class or module): it dispatches ``_attr_name`` on that one object, which every + client allowed in shares. A method that mutates it (``__setitem__``, ``write``...) + lets each of them change what the others see. + + Handler options: + + - ``obj_owner`` (required): ``(request) -> owner`` (see ``ObjOwner``); a request it + does not identify is refused, the others share the object. Or ``shared_objects`` + to let any client use it. An instance endpoint without one is refused at build + time. + - ``max_obj_size`` (``DFLT_MAX_OBJ_SIZE``) and ``obj_sizeof`` + (``bounded_deep_sizeof``): the bytes the object may hold, measured when the + service is built (an object already over it is refused then) and after every + dispatched call, failed ones included. The call that takes it over the limit has + already run: its result is withheld, the object is kept, and every later call is + refused until it is back under (shrunk outside the service). So memory is bounded + by the limit plus what the calls running at that moment allocate. ``None`` + disables the check. Raise ``max_obj_size``, or pass an ``obj_sizeof`` that + skips what is not request-driven (a loaded model, a database connection), for a + large object. Data the object keeps outside the process (files, a database) is + not measured. + """ + owner_of = handler.get("obj_owner") + if owner_of is None: + raise ValueError( + f"Instance endpoint {name!r} is one object shared by every client: give it " + "an obj_owner, a callable (request) -> owner (e.g. the authenticated user " + "id) that refuses requests it does not identify, or obj_owner=shared_objects " + "(from py2http.service) to let any client use it, as before." + ) + owner = (lambda: None) if owner_of is shared_objects else _request_owner(owner_of, name) + max_obj_size = handler.get("max_obj_size", DFLT_MAX_OBJ_SIZE) + sizeof = handler.get("obj_sizeof", bounded_deep_sizeof) + + def is_too_big(): + return max_obj_size is not None and sizeof(obj, max_obj_size) > max_obj_size + + if is_too_big(): + raise ValueError( + f"Instance endpoint {name!r} already holds more than max_obj_size=" + f"{max_obj_size} bytes (as measured by obj_sizeof): raise max_obj_size, pass " + "an obj_sizeof that skips what clients cannot grow, or max_obj_size=None." + ) + over_limit = threading.Event() # set while the object is known to be too big + + def refuse(): + over_limit.set() + raise InputError( + f"{name} is over its size limit ({max_obj_size} bytes): calls are refused " + "until it is back under" + ) + + def func(_attr_name=None, **kwargs): + owner() + if over_limit.is_set(): + if is_too_big(): + refuse() + over_limit.clear() + try: + return _get_attr_value(obj, _attr_name, is_dispatchable, **kwargs) + finally: # a call that grows the object and then raises is measured too + if is_too_big(): + refuse() + + return func + + def _class_endpoint_dispatcher(cls, handler, name, is_dispatchable): """The function behind a class endpoint: called without ``_obj_id`` it creates an object and returns its id; with one, it dispatches ``_attr_name`` on that object. @@ -1019,10 +1122,7 @@ def owner(): return _SHARED_OWNER else: - if not callable(owner_of): - raise TypeError( - f"obj_owner must be callable or shared_objects: {owner_of!r}" - ) + owner = _request_owner(owner_of, name) if handler.get("obj_store") is not None: raise ValueError( "obj_store applies to obj_owner=shared_objects only; with an obj_owner " @@ -1033,18 +1133,6 @@ def owner(): max_owners=handler.get("max_owners", DFLT_MAX_OWNERS), ) - def owner(): - who = owner_of(current_request.get()) - if who is None or who == "": - raise AuthorizationError(f"{name}: no owner for this request") - try: - hash(who) - except TypeError: - raise TypeError( - f"obj_owner of {name!r} must return a hashable, got {type(who)}" - ) from None - return who - def over_budget(who, obj_id, obj): """Why the object must go, or None. Records its size when it may stay.""" limit = max_obj_size diff --git a/py2http/tests/test_attr_dispatch.py b/py2http/tests/test_attr_dispatch.py index 9df41e8..394b9d9 100644 --- a/py2http/tests/test_attr_dispatch.py +++ b/py2http/tests/test_attr_dispatch.py @@ -28,9 +28,7 @@ def _reset(self): def _dispatcher(endpoint, attr_names, **handler_extras): - if inspect.isclass( - endpoint - ): # class endpoints must say who owns their objects (#22) + if not inspect.ismodule(endpoint): # objects must say who may use them (#22, #30) handler_extras.setdefault("obj_owner", shared_objects) return _get_func_to_dispatch_handler( dict(endpoint=endpoint, name="ep", attr_names=attr_names, **handler_extras) diff --git a/py2http/tests/test_instance_endpoints.py b/py2http/tests/test_instance_endpoints.py new file mode 100644 index 0000000..c0059d4 --- /dev/null +++ b/py2http/tests/test_instance_endpoints.py @@ -0,0 +1,161 @@ +"""Instance endpoints (one object every client shares) need an owner hook and are +bounded in size (#30).""" + +import io +import json +from wsgiref.util import setup_testing_defaults + +import pytest +from i2.errors import AuthorizationError, InputError + +from py2http import mk_app +from py2http.service import ( + _get_func_to_dispatch_handler, + current_request, + shared_objects, +) + + +class Store(dict): + """A mapping clients may write to.""" + + def write(self, key, value): + self[key] = value + return len(self) + + def read(self, key): + return self[key] + + def write_then_fail(self, key, value): + self[key] = value + raise ValueError("invalid") + + +def _instance(endpoint=None, **extras): + extras.setdefault("obj_owner", lambda req: req) # the "request" is the owner here + return _get_func_to_dispatch_handler( + dict(endpoint=Store() if endpoint is None else endpoint, name="store", attr_names="*", **extras) + ) + + +def _as(owner, func, **kwargs): + token = current_request.set(owner) + try: + return func(**kwargs) + finally: + current_request.reset(token) + + +def test_instance_endpoint_without_owner_is_refused_at_build(): + with pytest.raises(ValueError, match="obj_owner"): + _get_func_to_dispatch_handler(dict(endpoint=Store(), name="s", attr_names="*")) + + +def test_bad_owner_hook_is_refused_at_build(): + with pytest.raises(TypeError, match="obj_owner"): + _instance(obj_owner="ann") + + +def test_module_endpoint_needs_no_owner(): + import os.path as module + + func = _get_func_to_dispatch_handler( + dict(endpoint=module, name="m", attr_names=["basename"]) + ) + assert func(_attr_name="basename", p="a/b") == "b" + + +@pytest.mark.parametrize("who", [None, ""]) +def test_unidentified_request_is_refused(who): + store = Store() + func = _instance(store) + with pytest.raises(AuthorizationError): + _as(who, func, _attr_name="write", key="k", value=1) + assert store == {} + + +def test_identified_requests_share_the_object(): + func = _instance() + assert _as("ann", func, _attr_name="write", key="k", value=1) == 1 + assert _as("bob", func, _attr_name="read", key="k") == 1 + + +def test_shared_objects_lets_anyone_in(): + func = _instance(obj_owner=shared_objects) + assert _as(None, func, _attr_name="write", key="k", value=1) == 1 + + +def test_object_already_too_big_is_refused_at_build(): + with pytest.raises(ValueError, match="max_obj_size"): + _instance(Store(big="y" * 20_000), max_obj_size=10_000) + _instance(Store(big="y" * 20_000), max_obj_size=None) # check disabled + + +def test_growing_past_the_limit_refuses_until_back_under(): + store = Store() + func = _instance(store, max_obj_size=10_000) + _as("ann", func, _attr_name="write", key="a", value="small") + with pytest.raises(InputError, match="size limit"): + _as("ann", func, _attr_name="write", key="big", value="y" * 20_000) + assert "big" in store # the object is kept, not dropped: it is the endpoint + with pytest.raises(InputError, match="size limit"): # even a read, the object is full + _as("bob", func, _attr_name="read", key="a") + del store["big"] # shrunk outside the service + assert _as("bob", func, _attr_name="read", key="a") == "small" + + +def test_a_call_that_grows_the_object_then_raises_is_still_measured(): + store = Store() + func = _instance(store, max_obj_size=10_000) + with pytest.raises(InputError, match="size limit"): + _as("ann", func, _attr_name="write_then_fail", key="big", value="y" * 20_000) + with pytest.raises(InputError, match="size limit"): + _as("ann", func, _attr_name="write", key="more", value="y" * 20_000) + assert "more" not in store # refused before running + + +def test_custom_sizeof(): + func = _instance(max_obj_size=2, obj_sizeof=lambda obj, limit: len(obj)) + _as("ann", func, _attr_name="write", key="a", value="y" * 20_000) + _as("ann", func, _attr_name="write", key="b", value=1) + with pytest.raises(InputError, match="size limit"): + _as("ann", func, _attr_name="write", key="c", value=1) + + +def _post(app, path, payload, user): + body = json.dumps(payload).encode() + environ = { + "REQUEST_METHOD": "POST", + "PATH_INFO": path, + "CONTENT_TYPE": "application/json", + "CONTENT_LENGTH": str(len(body)), + "HTTP_X_USER": user, + "wsgi.input": io.BytesIO(body), + } + setup_testing_defaults(environ) + statuses = [] + out = b"".join(app(environ, lambda status, headers, *a: statuses.append(status))) + return statuses[0], json.loads(out) + + +def test_http_statuses(): + store = Store() + app = mk_app( + [ + dict( + endpoint=store, + name="store", + attr_names=["write"], + obj_owner=lambda req: req.headers.get("X-User"), + max_obj_size=10_000, + ) + ] + ) + payload = {"_attr_name": "write", "key": "k", "value": 1} + status, n = _post(app, "/store", payload, "ann") + assert status.startswith("200") and n == 1 + status, _ = _post(app, "/store", payload, "") + assert status.startswith("401") + payload = {"_attr_name": "write", "key": "big", "value": "y" * 20_000} + status, _ = _post(app, "/store", payload, "ann") + assert status.startswith("400") From 3d2634b9f81c84e8e483c6acb01bb341124a8c8e Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 13 Oct 2026 05:07:25 +0000 Subject: [PATCH 2/3] fix: review of #31: obj_shrinkers, sizeof follows bound methods/partials/closures - obj_shrinkers: names still callable while an instance endpoint is over its limit, so a client can bring it back under (one client could otherwise lock everyone out). - bounded_deep_sizeof follows what bound methods, partials and closures hold. - Document what is not measured and the per-call cost; fix the 401/403 comment. Co-Authored-By: Claude Opus 5.5 --- py2http/default_configs.py | 6 +-- py2http/service.py | 68 ++++++++++++++++++------ py2http/tests/test_instance_endpoints.py | 51 ++++++++++++++++-- 3 files changed, 103 insertions(+), 22 deletions(-) diff --git a/py2http/default_configs.py b/py2http/default_configs.py index 8f5139c..ff9d8cd 100644 --- a/py2http/default_configs.py +++ b/py2http/default_configs.py @@ -60,9 +60,9 @@ def _raise_http_client_error(error, message, reason=None): ) -# HTTP status of each client error, most specific class first. ``AuthorizationError`` -# itself means the request did not say who it is (401); its subclasses mean it did and -# may not do this (403). +# HTTP status of each client error, most specific class first. ``ForbiddenError`` and +# ``OverwritesNotAllowed`` mean the client may not do this (403); any other +# ``AuthorizationError`` means the request did not say who it is (401). _CLIENT_ERROR_STATUSES = ( ((ForbiddenError, OverwritesNotAllowed), 403), (AuthorizationError, 401), diff --git a/py2http/service.py b/py2http/service.py index fa3750e..a11f3d9 100644 --- a/py2http/service.py +++ b/py2http/service.py @@ -28,7 +28,7 @@ from inspect import isawaitable import json from typing import Any, Callable, Dict, Iterable, Optional, TypedDict, Union -from types import FunctionType +from types import BuiltinMethodType, FunctionType, MethodType, MethodWrapperType import logging import os import sys @@ -230,14 +230,17 @@ def owner_bytes(self, owner) -> int: return sum(self._sizes.get((owner, obj_id), 0) for obj_id in objects) -_SHARED_SIZE_LEAVES = (type, ModuleType, FunctionType) +_SHARED_SIZE_LEAVES = (type, ModuleType) +_BOUND_METHODS = (MethodType, BuiltinMethodType, MethodWrapperType) def bounded_deep_sizeof(obj, limit: Optional[int] = None) -> int: """Approximate memory held by ``obj`` and everything it references (containers, - instance ``__dict__`` and ``__slots__``), counting each object once. Stops as soon - as the total exceeds ``limit``, so measuring costs at most about ``limit`` bytes' - worth of objects. Classes, modules and functions are shared, so not counted. + instance ``__dict__`` and ``__slots__``, what a bound method, a ``partial`` or a + closure holds), counting each object once. Stops as soon as the total exceeds + ``limit``, so measuring costs at most about ``limit`` bytes' worth of objects. + Classes, modules and functions themselves are shared, so not counted, and neither + is what they hold (class attributes, module globals). >>> small, big = bounded_deep_sizeof([1, 2]), bounded_deep_sizeof(list(range(1000))) >>> small < big @@ -251,6 +254,14 @@ def bounded_deep_sizeof(obj, limit: Optional[int] = None) -> int: if id(o) in seen or isinstance(o, _SHARED_SIZE_LEAVES): continue seen.add(id(o)) + if isinstance(o, FunctionType): # shared code, but its closure is its own + todo.extend(cell.cell_contents for cell in o.__closure__ or () + if _cell_is_set(cell)) + continue + if isinstance(o, _BOUND_METHODS): # what it is bound to (a module is a leaf) + todo.append(o.__self__) + elif isinstance(o, partial): + todo.extend((o.func, o.args, o.keywords)) total += sys.getsizeof(o, 0) if limit is not None and total > limit: break @@ -269,6 +280,14 @@ def bounded_deep_sizeof(obj, limit: Optional[int] = None) -> int: return total +def _cell_is_set(cell) -> bool: + try: + cell.cell_contents + except ValueError: # an empty cell (a free variable not yet assigned) + return False + return True + + def public_attr(obj, attr_name: str) -> bool: """The attribute policy ``attr_names='*'`` stands for: only public methods. @@ -1019,12 +1038,20 @@ def _instance_endpoint_dispatcher(obj, handler, name, is_dispatchable): service is built (an object already over it is refused then) and after every dispatched call, failed ones included. The call that takes it over the limit has already run: its result is withheld, the object is kept, and every later call is - refused until it is back under (shrunk outside the service). So memory is bounded - by the limit plus what the calls running at that moment allocate. ``None`` - disables the check. Raise ``max_obj_size``, or pass an ``obj_sizeof`` that - skips what is not request-driven (a loaded model, a database connection), for a - large object. Data the object keeps outside the process (files, a database) is - not measured. + refused until it is back under, except calls to ``obj_shrinkers``. So memory is + bounded by the limit plus what the calls running at that moment allocate. + ``None`` disables the check. Raise ``max_obj_size``, or pass an ``obj_sizeof`` + that skips what is not request-driven (a loaded model, a database connection), + for a large object. Not measured: data the object keeps outside the process + (files, a database), and state held in its class or in module globals. + Measuring walks the object (up to ``max_obj_size`` bytes of it) after every + call, reads included, so a large limit costs time on every request. A method + returning a generator or coroutine does its work after the measure: what it + adds is caught by the next call. + - ``obj_shrinkers``: names of attributes that only ever remove (``__delitem__``, + ``pop``, ``clear``...), still callable while the object is over its limit so a + client can bring it back under. Empty by default: py2http cannot tell which + methods shrink an object. """ owner_of = handler.get("obj_owner") if owner_of is None: @@ -1047,26 +1074,35 @@ def is_too_big(): f"{max_obj_size} bytes (as measured by obj_sizeof): raise max_obj_size, pass " "an obj_sizeof that skips what clients cannot grow, or max_obj_size=None." ) + shrinkers = frozenset(handler.get("obj_shrinkers", ())) over_limit = threading.Event() # set while the object is known to be too big def refuse(): over_limit.set() + allowed = f" (except {', '.join(sorted(shrinkers))})" if shrinkers else "" raise InputError( - f"{name} is over its size limit ({max_obj_size} bytes): calls are refused " - "until it is back under" + f"{name} is over its size limit ({max_obj_size} bytes): calls{allowed} " + "are refused until it is back under" ) def func(_attr_name=None, **kwargs): owner() + shrinking = isinstance(_attr_name, str) and _attr_name in shrinkers if over_limit.is_set(): - if is_too_big(): + if not is_too_big(): + over_limit.clear() + elif not shrinking: refuse() - over_limit.clear() try: return _get_attr_value(obj, _attr_name, is_dispatchable, **kwargs) finally: # a call that grows the object and then raises is measured too if is_too_big(): - refuse() + if shrinking: # it ran, and removed what it could: stay refused + over_limit.set() + else: + refuse() + else: + over_limit.clear() return func diff --git a/py2http/tests/test_instance_endpoints.py b/py2http/tests/test_instance_endpoints.py index c0059d4..1a3715b 100644 --- a/py2http/tests/test_instance_endpoints.py +++ b/py2http/tests/test_instance_endpoints.py @@ -26,6 +26,9 @@ def write(self, key, value): def read(self, key): return self[key] + def remove(self, key): + del self[key] + def write_then_fail(self, key, value): self[key] = value raise ValueError("invalid") @@ -33,9 +36,9 @@ def write_then_fail(self, key, value): def _instance(endpoint=None, **extras): extras.setdefault("obj_owner", lambda req: req) # the "request" is the owner here - return _get_func_to_dispatch_handler( - dict(endpoint=Store() if endpoint is None else endpoint, name="store", attr_names="*", **extras) - ) + extras.setdefault("attr_names", "*") + endpoint = Store() if endpoint is None else endpoint + return _get_func_to_dispatch_handler(dict(endpoint=endpoint, name="store", **extras)) def _as(owner, func, **kwargs): @@ -159,3 +162,45 @@ def test_http_statuses(): payload = {"_attr_name": "write", "key": "big", "value": "y" * 20_000} status, _ = _post(app, "/store", payload, "ann") assert status.startswith("400") + + +def test_shrinkers_run_while_over_the_limit(): + store = Store() + func = _instance( + store, + max_obj_size=10_000, + attr_names=["write", "read", "remove"], + obj_shrinkers=["remove"], + ) + for key in ("a", "b"): + with pytest.raises(InputError) if key == "b" else _no_error(): + _as("ann", func, _attr_name="write", key=key, value="y" * 6_000) + with pytest.raises(InputError, match="except remove"): + _as("ann", func, _attr_name="read", key="a") + assert _as("ann", func, _attr_name="remove", key="b") is None + assert _as("ann", func, _attr_name="read", key="a") == "y" * 6_000 + + +def _no_error(): + from contextlib import nullcontext + + return nullcontext() + + +def test_state_behind_methods_partials_and_closures_is_measured(): + from functools import partial + + from py2http.service import bounded_deep_sizeof + + big = [str(i) * 1000 for i in range(100)] # distinct strings, not one constant + + class Holder: + pass + + def closure(): + return big + + for held in (big.append, partial(print, big), closure): + holder = Holder() + holder.held = held + assert bounded_deep_sizeof(holder) > 100_000, held From a184a1596dca9416cbfd5b8455bdc52780868540 Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 13 Oct 2026 05:12:41 +0000 Subject: [PATCH 3/3] fix: second review of #31: checked obj_shrinkers, atomic measure+flag, guarded traversal - obj_shrinkers: a str is refused, every name must be dispatched. - Measuring and setting/clearing the over-limit flag happen under one lock. - bounded_deep_sizeof follows MethodType.__func__, function attributes and defaults, static/class methods; an object whose attributes raise is measured plainly instead of failing the call. Co-Authored-By: Claude Opus 5.5 --- py2http/service.py | 113 +++++++++++++++-------- py2http/tests/test_instance_endpoints.py | 30 ++++++ 2 files changed, 107 insertions(+), 36 deletions(-) diff --git a/py2http/service.py b/py2http/service.py index a11f3d9..2ed492d 100644 --- a/py2http/service.py +++ b/py2http/service.py @@ -237,10 +237,12 @@ def owner_bytes(self, owner) -> int: def bounded_deep_sizeof(obj, limit: Optional[int] = None) -> int: """Approximate memory held by ``obj`` and everything it references (containers, instance ``__dict__`` and ``__slots__``, what a bound method, a ``partial`` or a - closure holds), counting each object once. Stops as soon as the total exceeds - ``limit``, so measuring costs at most about ``limit`` bytes' worth of objects. - Classes, modules and functions themselves are shared, so not counted, and neither - is what they hold (class attributes, module globals). + function's closure, attributes and defaults hold), counting each object once. Stops + as soon as the total exceeds ``limit``, so measuring costs at most about ``limit`` + bytes' worth of objects. Classes, modules and the code of functions are shared, so + not counted, and neither is what classes and modules hold (class attributes, module + globals), nor caches kept in C (``lru_cache``). Holding a bound method of another + object counts that whole object, as holding the object itself would. >>> small, big = bounded_deep_sizeof([1, 2]), bounded_deep_sizeof(list(range(1000))) >>> small < big @@ -254,14 +256,14 @@ def bounded_deep_sizeof(obj, limit: Optional[int] = None) -> int: if id(o) in seen or isinstance(o, _SHARED_SIZE_LEAVES): continue seen.add(id(o)) - if isinstance(o, FunctionType): # shared code, but its closure is its own - todo.extend(cell.cell_contents for cell in o.__closure__ or () - if _cell_is_set(cell)) + try: + held = _held_by_callable(o) + except Exception: # an exotic object (a mock, a proxy): measure it plainly + held = None + if isinstance(o, FunctionType): # the code is shared; only what it holds counts + todo.extend(held or ()) continue - if isinstance(o, _BOUND_METHODS): # what it is bound to (a module is a leaf) - todo.append(o.__self__) - elif isinstance(o, partial): - todo.extend((o.func, o.args, o.keywords)) + todo.extend(held or ()) total += sys.getsizeof(o, 0) if limit is not None and total > limit: break @@ -270,16 +272,41 @@ def bounded_deep_sizeof(obj, limit: Optional[int] = None) -> int: todo.extend(o.values()) elif isinstance(o, (list, tuple, set, frozenset, deque)): todo.extend(o) - if hasattr(o, "__dict__") and isinstance(vars(o), dict): - todo.append(vars(o)) - for klass in type(o).__mro__: - slots = getattr(klass, "__slots__", ()) - for slot in (slots,) if isinstance(slots, str) else slots: - if slot not in ("__dict__", "__weakref__") and hasattr(o, slot): - todo.append(getattr(o, slot)) + try: # a __getattr__ may raise anything + todo.extend(_instance_state(o)) + except Exception: + pass return total +def _instance_state(o): + """An object's ``__dict__`` and the values of its ``__slots__``.""" + if hasattr(o, "__dict__") and isinstance(vars(o), dict): + yield vars(o) + for klass in type(o).__mro__: + slots = getattr(klass, "__slots__", ()) + for slot in (slots,) if isinstance(slots, str) else slots: + if slot not in ("__dict__", "__weakref__") and hasattr(o, slot): + yield getattr(o, slot) + + +def _held_by_callable(o) -> Optional[list]: + """What a function (closure, attributes, defaults), bound method, static or class + method, or ``partial`` holds; ``None`` for anything else.""" + if isinstance(o, FunctionType): + cells = [c.cell_contents for c in o.__closure__ or () if _cell_is_set(c)] + return [*cells, o.__dict__, o.__defaults__, o.__kwdefaults__] + if isinstance(o, MethodType): + return [o.__self__, o.__func__] + if isinstance(o, _BOUND_METHODS): # a builtin's: what it is bound to + return [o.__self__] + if isinstance(o, (staticmethod, classmethod)): + return [o.__func__] + if isinstance(o, partial): + return [o.func, o.args, o.keywords] + return None + + def _cell_is_set(cell) -> bool: try: cell.cell_contents @@ -1048,10 +1075,12 @@ def _instance_endpoint_dispatcher(obj, handler, name, is_dispatchable): call, reads included, so a large limit costs time on every request. A method returning a generator or coroutine does its work after the measure: what it adds is caught by the next call. - - ``obj_shrinkers``: names of attributes that only ever remove (``__delitem__``, - ``pop``, ``clear``...), still callable while the object is over its limit so a - client can bring it back under. Empty by default: py2http cannot tell which - methods shrink an object. + - ``obj_shrinkers``: names of dispatched attributes that only ever remove + (``pop``, ``clear``, ``__delitem__`` if ``attr_names`` lists it...), still + callable while the object is over its limit so a client can bring it back under. + Empty by default: py2http cannot tell which methods shrink an object, and the + bound holds only if every name listed really only removes (one that also adds is + never refused). """ owner_of = handler.get("obj_owner") if owner_of is None: @@ -1074,11 +1103,30 @@ def is_too_big(): f"{max_obj_size} bytes (as measured by obj_sizeof): raise max_obj_size, pass " "an obj_sizeof that skips what clients cannot grow, or max_obj_size=None." ) - shrinkers = frozenset(handler.get("obj_shrinkers", ())) + shrinkers = handler.get("obj_shrinkers", ()) + if isinstance(shrinkers, str): + raise TypeError(f"obj_shrinkers must be a collection of names: {shrinkers!r}") + shrinkers = frozenset(shrinkers) + for attr_name in shrinkers: + if not is_dispatchable(obj, attr_name): + raise ValueError( + f"obj_shrinkers of {name!r}: {attr_name!r} is not dispatched " + "(see attr_names)" + ) over_limit = threading.Event() # set while the object is known to be too big + measuring = threading.Lock() # so a stale measure cannot clear a newer one's flag + + def measure() -> bool: + """Whether the object is over its limit, recorded in ``over_limit``.""" + with measuring: + too_big = is_too_big() + if too_big: + over_limit.set() + else: + over_limit.clear() + return too_big def refuse(): - over_limit.set() allowed = f" (except {', '.join(sorted(shrinkers))})" if shrinkers else "" raise InputError( f"{name} is over its size limit ({max_obj_size} bytes): calls{allowed} " @@ -1088,21 +1136,14 @@ def refuse(): def func(_attr_name=None, **kwargs): owner() shrinking = isinstance(_attr_name, str) and _attr_name in shrinkers - if over_limit.is_set(): - if not is_too_big(): - over_limit.clear() - elif not shrinking: - refuse() + if over_limit.is_set() and measure() and not shrinking: + refuse() try: return _get_attr_value(obj, _attr_name, is_dispatchable, **kwargs) finally: # a call that grows the object and then raises is measured too - if is_too_big(): - if shrinking: # it ran, and removed what it could: stay refused - over_limit.set() - else: - refuse() - else: - over_limit.clear() + # a shrinker that ran but left it over the limit keeps its result + if measure() and not shrinking: + refuse() return func diff --git a/py2http/tests/test_instance_endpoints.py b/py2http/tests/test_instance_endpoints.py index 1a3715b..c25e9ae 100644 --- a/py2http/tests/test_instance_endpoints.py +++ b/py2http/tests/test_instance_endpoints.py @@ -204,3 +204,33 @@ def closure(): holder = Holder() holder.held = held assert bounded_deep_sizeof(holder) > 100_000, held + + +def test_shrinkers_are_checked_at_build(): + with pytest.raises(TypeError, match="collection"): + _instance(attr_names=["remove"], obj_shrinkers="remove") + with pytest.raises(ValueError, match="not dispatched"): + _instance(attr_names=["remove"], obj_shrinkers=["remvoe"]) + + +def test_more_held_state_is_measured_and_odd_objects_do_not_break_it(): + from types import MethodType + from unittest.mock import Mock + + from py2http.service import bounded_deep_sizeof + + big = [str(i) * 1000 for i in range(100)] + + def closure(self): + return big + + class Holder: + pass + + holder = Holder() + holder.held = MethodType(closure, object()) + assert bounded_deep_sizeof(holder) > 100_000 + holder.held = staticmethod(lambda: big) + assert bounded_deep_sizeof(holder) > 100_000 + holder.held = Mock(spec=type(closure)) + assert bounded_deep_sizeof(holder) > 0