Skip to content

Conversation

@adhami3310
Copy link
Member

found by @HO-9

this is not a vuln because granian/uvicorn impose a max header count of 100-130

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 4, 2025

CodSpeed Performance Report

Merging #6021 will not alter performance

Comparing optimize-frozen-dict-get-item (c1b17c2) with main (e4300ad)

Summary

✅ 8 untouched

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 4, 2025

Greptile Overview

Greptile Summary

Optimized _FrozenDictStrStr.__getitem__ from O(n) to O(1) by replacing tuple storage with MappingProxyType, preventing potential performance issues when accessing headers.

  • Changed internal _data storage from tuple[tuple[str, str], ...] to MappingProxyType[str, str]
  • Optimized __getitem__ to use direct lookup instead of creating a dict on every access
  • Added __hash__, __getstate__, and __setstate__ methods for proper immutability and serialization
  • Critical issue: dataclasses.asdict() will fail because MappingProxyType cannot be deep-copied without registering a pickle function via copyreg

Confidence Score: 1/5

  • This PR will cause runtime errors when dataclasses.asdict() is called on HeaderData objects
  • The optimization approach is sound, but MappingProxyType cannot be pickled/deep-copied by default, causing dataclasses.asdict() to fail. Test at tests/units/test_state.py:942-944 will fail, and any production code using dataclasses.asdict(HeaderData) will break
  • reflex/istate/data.py requires either registering MappingProxyType with copyreg or using a different approach for the _data field

Important Files Changed

File Analysis

Filename Score Overview
reflex/istate/data.py 3/5 Optimized _FrozenDictStrStr.__getitem__ from O(n) to O(1) by using MappingProxyType instead of tuple, but test at tests/units/test_state.py:942-944 will break

Sequence Diagram

sequenceDiagram
    participant Client
    participant RouterData
    participant HeaderData
    participant FrozenDict as _FrozenDictStrStr
    participant MappingProxy as MappingProxyType

    Client->>RouterData: from_router_data(router_data)
    RouterData->>HeaderData: from_router_data(router_data)
    HeaderData->>FrozenDict: __init__(**headers)
    FrozenDict->>FrozenDict: sort headers dict
    FrozenDict->>MappingProxy: wrap in MappingProxyType
    Note over FrozenDict,MappingProxy: Old: stored as tuple<br/>New: stored as MappingProxyType
    
    Client->>FrozenDict: __getitem__(key)
    Note over FrozenDict: Old: dict(self._data)[key] - O(n)<br/>New: self._data[key] - O(1)
    FrozenDict-->>Client: value
    
    Client->>HeaderData: dataclasses.asdict()
    HeaderData->>FrozenDict: process as dataclass
    FrozenDict->>MappingProxy: copy.deepcopy(_data)
    MappingProxy-->>FrozenDict: TypeError: cannot pickle mappingproxy
    FrozenDict-->>Client: Error!
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@adhami3310 adhami3310 merged commit e6b899a into main Dec 5, 2025
47 checks passed
@adhami3310 adhami3310 deleted the optimize-frozen-dict-get-item branch December 5, 2025 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants