Fixed #18119 -- Added a DomainNameValidator validator. #18037
Conversation
|
Thank you for this Nina ⭐
All new features need some docs and a release note, so this is roughly what I am expecting:
|
nmenezes0
force-pushed
the
feature/18119-domain-name-validation
branch
from
fc3bd96
to
f77f5f3
Compare
|
Thanks Sarah - docs now updated. |
|
Thanks @felixxm - docs now updated following comments. |
There was a problem hiding this comment.
Thank you @nmenezes0 ⭐ I have a few more comments.
Please also squash this to a single commit 👍
django/core/validators.py
Outdated
| ipv6_re = r"\[[0-9a-f:.]+\]" # (simple regex, validated later) | ||
|
|
||
| # Host patterns | ||
| # Max length for domain name labels is 63 characters per RFC 1034 sec. 3.1 |
There was a problem hiding this comment.
The comment # Max length for domain name labels is 63 characters per RFC 1034 sec. 3.1 should be above domain_re
I also think as this is best placed above the regex, we can remove the duplicate comment above domain_re = DomainNameValidator.domain_re in URLValidator
With a similar logic, add the # Host patterns comment here and remove the # Host patterns comment from above hostname_re = DomainNameValidator.hostname_re in URLValidator
Let's also add full stops to the end of the comments which are not inline (so # Host patterns. but maybe don't bother with the # can't end with a dash comments)
There was a problem hiding this comment.
Thanks - comments in DomainNameValidator updated and relevant comments in URLValidator.
django/core/validators.py
Outdated
| # Host patterns | ||
| hostname_re = DomainNameValidator.hostname_re | ||
| # Max length for domain name labels is 63 characters per RFC 1034 sec. 3.1 |
There was a problem hiding this comment.
| # Host patterns | |
| hostname_re = DomainNameValidator.hostname_re | |
| # Max length for domain name labels is 63 characters per RFC 1034 sec. 3.1 | |
| hostname_re = DomainNameValidator.hostname_re |
As per the above comment which hopefully makes sense
docs/ref/validators.txt
Outdated
| @@ -370,3 +370,28 @@ to, or in lieu of custom ``field.clean()`` methods. | |||
| .. versionchanged:: 5.0 | |||
|
|
|||
| The ``offset`` argument was added. | |||
|
|
|||
| ``DomainNameValidator`` | |||
There was a problem hiding this comment.
I think let's move the docs to be either under or above URLValidator to keep them close together
There was a problem hiding this comment.
Now moved to above the URLValidator section.
nmenezes0
force-pushed
the
feature/18119-domain-name-validation
branch
from
b2b5e90
to
151845f
Compare
Override __init__ function to add accept IDNA. Updates to DomainNameValidator to add logic around accept_idna flag Remove outdated details from docs. Made changes to domain name validation and tests. Reused regex in URLValidator. Added more changes to the DomainNameValidator so all tests pass. Removed rogue space. Blacken. Update documentation for DomainNameValidator. Remove unused text. Tidy docs. Update docs - fix underline. Update docs/ref/validators.txt Remove unnecessary blank lines. Co-authored-by: Mariusz Felisiak <felisiak.mariusz@gmail.com> Update docs/releases/5.1.txt Co-authored-by: Mariusz Felisiak <felisiak.mariusz@gmail.com> Changes to docs for DomainNameValidator. Change name of the parameter. Co-authored-by: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Update docs for new naming. Co-authored-by: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Update django/core/validators.py Co-authored-by: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Update comments for validators. Reorder docs to have DomainNameValidator next to URLNameValidator. blacken
nmenezes0
force-pushed
the
feature/18119-domain-name-validation
branch
from
151845f
to
dd9816e
Compare
|
Thanks @sarahboyce - I've made those changes and squashed to a single commit. |
|
I'm not sure why some of the checks are failing. The error is "commit changed" - potentially due to me pushing the squashed commit? |
Trac ticket number
ticket-18119
Branch description
Provide a concise overview of the issue or rationale behind the proposed changes.
This ticket adds a DomainNameValidator to validate International domain names. This validator takes a flag
is_idna(defaults toTrue) - to determine whether or not we should use non-ASCII international domain names.This builds on the existing PR with some changes: not using IP addresses (don't think they are part of a domain name), removing changes to docs as out of date (not sure what updates are needed here now?), changes to the tests and how they were written, reusing the regex in URLValidator.
The ticket in Trac suggests that the regex could be reused elsewhere in the codebase. I have reused it in the URLValidator, but I couldn't see how to use it in
django/core/mail/message.pyordjango/utils/html.py.Checklist
mainbranch.I wasn't sure what changes I should make to the documentation.