#33456 closed Cleanup/optimization (wontfix)
Make underscore in hostname error more explicit
Reported by: | kimsia | Owned by: | nobody |
---|---|---|---|
Component: | HTTP handling | Version: | 3.2 |
Severity: | Normal | Keywords: | |
Cc: | Florian Apolloner | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
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."
Change History (6)
comment:1 by , 3 years ago
Cc: | added |
---|---|
Component: | Error reporting → HTTP handling |
Owner: | set to |
Resolution: | → wontfix |
Status: | new → closed |
comment:2 by , 3 years ago
Description: | modified (diff) |
---|---|
Has patch: | set |
comment:3 by , 3 years ago
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.
follow-up: 5 comment:4 by , 3 years ago
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.
follow-up: 6 comment:5 by , 3 years ago
Replying to 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.
comment:6 by , 3 years ago
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 PR is unreachable.
Thanks for this ticket, however I see no reason to treat the underscore differently from the other forbidden chars.