Skip to content

Ensure skip_defaults doesn't cause extra fields to be serialized#485

Merged
tiangolo merged 1 commit intofastapi:masterfrom
dmontagu:skip-defaults-fix
Aug 30, 2019
Merged

Ensure skip_defaults doesn't cause extra fields to be serialized#485
tiangolo merged 1 commit intofastapi:masterfrom
dmontagu:skip-defaults-fix

Conversation

@dmontagu
Copy link
Collaborator

@dmontagu dmontagu commented Aug 29, 2019

Currently, if skip_defaults is true, the secure cloned field is not used when serializing the response. This can lead to extra information leaking out if the response_model differs in type from the returned model.

This pull request fixes this bug, and updates the relevant unit test to check for it.

(The bug was introduced in #422 -- sorry about that!)


I believe this pull request also improves performance in the case where skip_defaults is True, as the response will now only be validated once in the serialize_response call, instead of twice.

@codecov
Copy link

codecov bot commented Aug 29, 2019

Codecov Report

Merging #485 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #485   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         243    243           
  Lines        5696   5698    +2     
=====================================
+ Hits         5696   5698    +2
Impacted Files Coverage Δ
fastapi/routing.py 100% <100%> (ø) ⬆️
tests/test_skip_defaults.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 417a3ab...2393a30. Read the comment docs.

@tiangolo tiangolo merged commit 6c6bdb6 into fastapi:master Aug 30, 2019
@tiangolo
Copy link
Member

Good catch! 🔍 🕵️‍♂️ 🚀

Thanks a lot! 🎉 🍰

@skaaptjop
Copy link

Just wonder if line 49 in routing.py shouldn't read:

response = response.dict(skip_defaults=skip_defaults, by_alias=by_alias)

When using a response model that uses field aliases, the validation step following that conversion always fails.

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