[Django] #33456: Make underscore in hostname error more explicit
Django
------------------------------------------------+------------------------
Reporter: kimsia | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: Error reporting | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
Currently, the error message is simply
"The domain name provided is not valid according to RFC 1034/1035."
Most tickets filed against this topic is about how underscores should be
allowed. I agreed with Django's choice to invalidate underscores.
https://github.com/django/django/pull/594 explains this clearly.
However, the error message can be clearer.
I recommend when underscore is detected, simply make it more explicit
" %r contains _ and that is not valid according to RFC 1034/1035." %
domain
Otherwise, "The domain name provided is not valid according to RFC
1034/1035."
--
Ticket URL: <https://code.djangoproject.com/ticket/33456>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: kimsia | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: HTTP handling | Version: 3.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: Florian Apolloner (added)
* owner: (none) => nobody
* status: new => closed
* component: Error reporting => HTTP handling
* resolution: => wontfix
Comment:
Thanks for this ticket, however I see no reason to treat the underscore
differently from the other forbidden chars.
--
Ticket URL: <https://code.djangoproject.com/ticket/33456#comment:1>
Django
Reporter: kimsia | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: HTTP handling | Version: 3.2
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by kimsia):
* has_patch: 0 => 1
Old description:
> Currently, the error message is simply
>
> "The domain name provided is not valid according to RFC 1034/1035."
>
> Most tickets filed against this topic is about how underscores should be
> allowed. I agreed with Django's choice to invalidate underscores.
>
> https://github.com/django/django/pull/594 explains this clearly.
>
> However, the error message can be clearer.
>
> I recommend when underscore is detected, simply make it more explicit
>
> " %r contains _ and that is not valid according to RFC 1034/1035." %
> domain
>
> Otherwise, "The domain name provided is not valid according to RFC
> 1034/1035."
New description:
Currently, the error message is simply
"The domain name provided is not valid according to RFC 1034/1035."
Most tickets filed against this topic is about how underscores should be
allowed. I agreed with Django's choice to invalidate underscores.
https://github.com/django/django/pull/594 explains this clearly.
However, the error message can be clearer.
I recommend when underscore is detected, simply make it more explicit
" %r contains _ and that is not valid according to RFC 1034/1035." %
domain
Otherwise, "The domain name provided is not valid according to RFC
1034/1035."
Patch: https://github.com/django/django/pull/15343
--
--
Ticket URL: <https://code.djangoproject.com/ticket/33456#comment:2>
Django
Reporter: kimsia | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: HTTP handling | Version: 3.2
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by kimsia):
> Is this really Django's choice? 🤔 As far as I'm aware they are
forbidden in RFC 1035 and we're not doing anything unusual here.
I mean yes, it's Django's choice to follow RFC 1035 strictly. I have no
issues with that.
The more explicit message for underscore is to help silly people like me
to realize the error faster.
To address the issue of not treating underscore any more special, would
you mind if I change the error message to highlight which character
doesn't conform to the hostname regardless whether it's underscore or
something else.
--
Ticket URL: <https://code.djangoproject.com/ticket/33456#comment:3>
Django
Reporter: kimsia | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: HTTP handling | Version: 3.2
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
Given that we already have an if/else there and it seems to be a somewhat
common issue that can also reduce the number of tickets created I am a
slight +0 for accepting, but I do not feel strongly.
--
Ticket URL: <https://code.djangoproject.com/ticket/33456#comment:4>
Django
Reporter: kimsia | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: HTTP handling | Version: 3.2
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by kimsia):
Replying to [comment:4 Florian Apolloner]:
> Given that we already have an if/else there and it seems to be a
somewhat common issue that can also reduce the number of tickets created I
am a slight +0 for accepting, but I do not feel strongly.
I will try to rewrite in a way that respects the existing if/else
statement and yes i also note the number of tickets talking about this
issue. Personally i just wasted a few hrs of my time because i didn't read
the RFC close enough.
--
Ticket URL: <https://code.djangoproject.com/ticket/33456#comment:5>
Django
Reporter: kimsia | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: HTTP handling | Version: 3.2
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
> I will try to rewrite in a way that respects the existing if/else
statement and yes i also note the number of tickets talking about this
issue. Personally i just wasted a few hrs of my time because i didn't read
the RFC close enough.
I'm not sure that fixing this won't add more complexity than it's worth,
but we could evaluate a patch. Please take into account that code added in
[https://github.com/django/django/pull/15343 PR] is unreachable.
--
Ticket URL: <https://code.djangoproject.com/ticket/33456#comment:6>