Skip to content

fix: isolate type decoders from shared signature annotation state#4639

Open
maksimzayats wants to merge 1 commit intolitestar-org:mainfrom
maksimzayats:fix-type-decorators
Open

fix: isolate type decoders from shared signature annotation state#4639
maksimzayats wants to merge 1 commit intolitestar-org:mainfrom
maksimzayats:fix-type-decorators

Conversation

@maksimzayats
Copy link
Copy Markdown
Contributor

@maksimzayats maksimzayats commented Mar 19, 2026

Description

This removes the shared _decoder mutation from signature parsing.

Previously, SignatureModel.create() could attach a decoder directly to the annotated type, which made decoder behavior process-global. That meant one handler, app, or DI provider could accidentally affect another if they used the same annotation class.

Now, signature parsing no longer mutates user-defined or stdlib classes. Request parsing stays scoped to the current handler or provider through the existing default_deserializer(type_decoders=...) path.

class UserId:
    def __init__(self, value: str) -> None:
        self.value = value


@get("/decoded", type_decoders=[(lambda t: t is UserId, lambda t, v: t(f"decoded:{v}"))])
def decoded(user_id: UserId) -> str:
    return user_id.value


@get("/plain")
def plain(user_id: UserId) -> str:
    return user_id.value

Old behavior:

# Building /decoded mutated the shared UserId type:
UserId._decoder = lambda t, v: t(f"decoded:{v}")

# So /plain could start decoding too, even without its own decoder.
# /decoded?user_id=1 -> "decoded:1"
# /plain?user_id=1   -> "decoded:1"   # leaked

New behavior:

# No mutation on UserId.
# Decoding stays scoped to the current handler.

# /decoded?user_id=1 -> "decoded:1"
# /plain?user_id=1   -> 400 validation error

📚 Documentation preview 📚: https://litestar-org.github.io/litestar-docs-preview/4639

@maksimzayats maksimzayats requested review from a team as code owners March 19, 2026 19:14
@github-actions github-actions bot added area/private-api This PR involves changes to the privatized API area/signature size: small labels Mar 19, 2026
@maksimzayats maksimzayats changed the title Isolate type decoders from shared signature annotation state fix: isolate type decoders from shared signature annotation state Mar 19, 2026
Copy link
Copy Markdown
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

I would categorize this as a tricky optimization instead of just a bug. Current behavior looks more correct to me. However, we also have to measure the perf impact of this change.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.87%. Comparing base (9587ba1) to head (9ab8b99).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4639      +/-   ##
==========================================
- Coverage   97.87%   97.87%   -0.01%     
==========================================
  Files         292      292              
  Lines       15077    15067      -10     
  Branches     1703     1701       -2     
==========================================
- Hits        14757    14747      -10     
  Misses        184      184              
  Partials      136      136              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@maksimzayats
Copy link
Copy Markdown
Contributor Author

Hi @sobolevn ! Hmm, the old behavior looks quite strange to me tbh... and it looks like more a bug, but I'd love to hear other maintainers opinion on this

@sobolevn
Copy link
Copy Markdown
Member

Context: jcrist/msgspec#497 and jcrist/msgspec#483

@provinzkraut
Copy link
Copy Markdown
Member

Agree with @sobolevn. Yes, it's a bit iffy to store things globally like this, but it's an optimisation we deemed worth it, as the lookup process is quite expensive. If you want to remove this, I'd need comprehensive benchmarks to show there's no negative impact on performance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/private-api This PR involves changes to the privatized API area/signature pr/external size: small Triage Required 🏥 This requires triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants