Opened 2 years ago
Closed 2 years ago
#34920 closed Bug (fixed)
FileExtensionValidator.__eq__() should ignore allowed_extensions order.
| Reported by: | ksg | Owned by: | ksg |
|---|---|---|---|
| Component: | Core (Other) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
django.core.validators.FileExtensionValidator had an __eq__ method to compare the validator class. However, comparing arrays is not accurate when the order of elements in the arrays is different.
def __eq__(self, other):
return (
isinstance(other, self.__class__)
and sorted(self.allowed_extensions) == sorted(other.allowed_extensions)
and self.message == other.message
and self.code == other.code
)
This test case failed:
self.assertEqual(
FileExtensionValidator(["jpg", "png", "txt"]),
FileExtensionValidator(["txt", "jpg", "png"]),
)
So I suggest comparing two extension arrays after sorting them.
Change History (10)
comment:1 by , 2 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 2 years ago
comment:3 by , 2 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
follow-up: 7 comment:4 by , 2 years ago
| Description: | modified (diff) |
|---|
I'd think that validators that behave identically should be considered equal. Did you run into a real-world bug with the current behavior?
comment:5 by , 2 years ago
| Summary: | Reordered file extensions for improved validation → FileExtensionValidator.__eq__() should ignore allowed_extensions order. |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:6 by , 2 years ago
| Patch needs improvement: | set |
|---|
comment:7 by , 2 years ago
Replying to Tim Graham:
I'd think that validators that behave identically should be considered equal. Did you run into a real-world bug with the current behavior?
No, it's just an enhancement to make eq work better!
comment:8 by , 2 years ago
| Patch needs improvement: | unset |
|---|
comment:9 by , 2 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
PR: https://github.com/django/django/pull/17398