Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#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 kimsia)

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

Change History (6)

comment:1 by Mariusz Felisiak, 3 years ago

Cc: Florian Apolloner added
Component: Error reportingHTTP handling
Owner: set to nobody
Resolution: wontfix
Status: newclosed

Thanks for this ticket, however I see no reason to treat the underscore differently from the other forbidden chars.

Version 0, edited 3 years ago by Mariusz Felisiak (next)

comment:2 by kimsia, 3 years ago

Description: modified (diff)
Has patch: set

comment:3 by kimsia, 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.

comment:4 by Florian Apolloner, 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.

in reply to:  4 ; comment:5 by kimsia, 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.

in reply to:  5 comment:6 by Mariusz Felisiak, 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.

Note: See TracTickets for help on using tickets.
Back to Top