Stop as_dict() from modifying the ValidatedEmail it is called on - #174
Merged
Merged
Conversation
as_dict() handed back the instance's live __dict__ and then wrote repr() of domain_address into it, so reading the dict replaced the documented ipaddress.IPv4Address object on the object itself. Every later call repr'd the string again, and writing to the returned dict mutated the ValidatedEmail. as_constructor() does the same "make it printable" job with repr(getattr(self, key)) and never mutates, so take a copy instead. The existing test asserted the doubly repr'd value, which only existed because of the mutation; it now asserts the single repr and a new test pins that the attribute survives an as_dict() call.
Owner
|
Thanks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
as_dict()hands back the instance's live__dict__and then writesrepr()ofdomain_addressinto it, so reading the dict modifies the object it was read from:domain_addressis documented to hold the parsedipaddress.IPv4Address/IPv6Addressobject, so anything downstream that uses it — comparing addresses, or passing it toipaddress/sockethelpers — breaks after a single read, and each further call compounds the damage. Because the dict is the live namespace,v.as_dict()["domain"] = "x"also writes through to the object.as_constructor()in the same class does the same "make it printable" job withrepr(getattr(self, key))and never mutates, so the fix is to copy first:Worth flagging: the existing
test_dict_accessor_with_domain_addressasserted'"IPv4Address(\'127.0.0.1\')"'— the doubly repr'd value, which only ever existed because of this mutation. That test now asserts the singlerepr, and a newtest_dict_accessor_does_not_modify_validated_emailpins that the attribute is still the original object after a call and that a second call returns the same dict.Test:
pytest tests/test_main.py -k dict_accessorfails on0552069withassert "IPv4Address('127.0.0.1')" is IPv4Address('127.0.0.1')and passes here. The suite is 317 passed / 1 deselected (the deselected one needs network),flake8 --ignore=E501,E126,W503 email_validator testsis clean, andmypyreports no issues across the 13 source files.I did not add a CHANGELOG entry under "In Development" — happy to if you would like one.