[Django] #29606: Validate the type of ALLOWED_HOSTS to prevent possible issues and wasted time

Django

unread,

Jul 28, 2018, 3:51:41 AM7/28/18

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS to prevent possible issues and wasted
time
-------------------------------------+-------------------------------------
Reporter: rafis | Owner: nobody
Type: New | Status: new
feature |
Component: Core | Version: 2.0
(System checks) |
Severity: Normal | Keywords: allowed_hosts
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
Python has soft behavior for conducting iteration process over lists and
over strings making them look the same:
{{{#!python
for char_or_item in str_or_list:
-- `char_or_item` can be character or list item
}}}
It would be better if it would have more strict behavior, for example,
{{{#!python
for char in some_str.chars():
-- now `char` can be only of string type and `list` class would
not have `chars` method
}}}
and for list
{{{#!python
for item in some_list.list_items():
-- `string` class would not have `list_items` method
}}}
This soft behavior usually leads to many nasty bugs to appear. Our two
software engineers from our team wasted about 1 hour debugging the issue
with ALLOWED_HOSTS being initialized with string in `local_settings.py`
which is included at the end of `settings.py`. Django was matching each
separate character of ALLOWED_HOSTS string against the "Host:" header from
an incoming HTTP request.

An obvious self-suggesting solution is to add a new system check that will
check the type of ALLOWED_HOSTS if it is string or not and notify the
developer about possible improper configuration. I think blacklist
checking (string or not) is more appropiate here, but I can be wrong.

--
Ticket URL: <https://code.djangoproject.com/ticket/29606>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Jul 28, 2018, 9:11:37 AM7/28/18

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

--------------------------------------+------------------------------------
Reporter: rafis | Owner: nobody
Type: New feature | Status: new
Component: Core (System checks) | Version: master
Severity: Normal | Resolution:
Keywords: allowed_hosts | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Tim Graham):

* needs_better_patch: 0 => 1
* version: 2.0 => master
* easy: 1 => 0
* has_patch: 0 => 1
* stage: Unreviewed => Accepted

Comment:

[https://github.com/django/django/pull/10151 PR]

I'm not sure if a system check is the way to go. The deployment checks
must be run manually `manage.py check --deploy`. The developer might not
do that as part of a debugging workflow.

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:1>

Django

unread,

Aug 20, 2018, 4:39:29 PM8/20/18

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

--------------------------------------+------------------------------------
Reporter: rafis | Owner: nobody

Changes (by Adam (Chainz) Johnson):

* cc: Adam (Chainz) Johnson (added)

Comment:

Although it may or may not have helped, I believe in general we should
check as much configuration as possible up front. Another error I myself
have made with ALLOWED_HOSTS is trying "*.example.com" instead of
".example.com" (confusion with "*" being an allowed value) - this (or in
general valid/invalid characters?) could also be checked for.

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:2>

Django

unread,

Aug 20, 2018, 5:01:28 PM8/20/18

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

--------------------------------------+------------------------------------
Reporter: rafis | Owner: nobody

Comment (by Tim Graham):

I'm not opposed to some validation, my suggestion was merely that maybe we
could be more helpful than a system check.

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:3>

Django

unread,

Sep 5, 2018, 1:30:22 PM9/5/18

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

--------------------------------------+------------------------------------
Reporter: rafis | Owner: nobody

Changes (by Herbert Fortes):

* cc: Herbert Fortes (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:4>

Django

unread,

Nov 13, 2018, 2:21:09 PM11/13/18

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

--------------------------------------+------------------------------------
Reporter: rafis | Owner: nobody

Comment (by Nick Mavrakis):

Sorry for the interfere but I would like to add my opinion on this. If
this New Feature is added, then proportionate validations should apply on
other `settings.py` variables (such as `INTERNAL_IPS` to be a list of
valid IPs, `STATIC_ROOT` to be a string of valid pathname etc). See where
it's going? I am not opposed on this improvement but if it's going to
happen, IMO it should happen if not on the whole
[https://docs.djangoproject.com/en/dev/ref/settings/ settings list], at
least on most of them.
Best regards, Nick

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:5>

Django

unread,

Nov 14, 2018, 3:40:51 PM11/14/18

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

-------------------------------------+-------------------------------------
Changes (by Tomasz Kluczkowski):

* owner: nobody => Tomasz Kluczkowski
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:6>

Django

unread,

Oct 20, 2020, 2:10:53 PM10/20/20

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

--------------------------------------+------------------------------------
Reporter: rafis | Owner: (none)

Type: New feature | Status: new
Component: Core (System checks) | Version: master

Severity: Normal | Resolution:
Keywords: allowed_hosts | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Mariusz Felisiak):

* owner: Tomasz Kluczkowski => (none)
* status: assigned => new
* easy: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:7>

Django

unread,

Oct 20, 2020, 11:59:15 PM10/20/20

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

-------------------------------------+-------------------------------------
Reporter: rafis | Owner: Octavio
| Peri

Type: New feature | Status: assigned
Component: Core (System | Version: master
checks) |

Severity: Normal | Resolution:
Keywords: allowed_hosts | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Octavio Peri):

* owner: (none) => Octavio Peri

* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:8>

Django

unread,

Nov 1, 2020, 10:37:06 PM11/1/20

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Octavio Peri):

Hi, I've assigned this ticket to myself and would like to check my idea
with you before writing any code.

I'm not sure about what Nick Mavrakis stated, maybe that'd be a good idea.

But sticking to this ticket, I believe the best way to proceed is checking
the setting is both iterable and not a str.
I would do so in two places:
1 - In `runserver`, where there's already one check related to
ALLOWED_HOSTS
2 - As a deployment check, that would raise an error if the constraints
are not satisfied.

If you could confirm this would be the way to go, I'd be more than happy
to add them in a PR.

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:9>

Django

unread,

Nov 2, 2020, 3:58:03 AM11/2/20

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam (Chainz) Johnson):

It needs only be in one place, as a system check. `runserver` runs system
checks at startup already. IMO it should not be a deployment check, as
`ALLOWED_HOSTS` affects use of `runserver` locally too if `DEBUG = False`.

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:10>

Django

unread,

Dec 25, 2020, 1:54:23 AM12/25/20

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Akshat1Nar):

Hello, I am Akshat, new to this organisation and I would like to
contribute on this bug. I do understand what rafis is trying to say here,
but I don't understand how to proceed with fixing with this issue. Can
someone tell me what files need to be modified or how to figure that out.

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:11>

Django

unread,

Jan 20, 2021, 12:04:21 PM1/20/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Marius Räsener):

Wouldn't it be best to check if the entries in `ALLOWED_HOSTS` are
actually valid hosts?
So either IPv4/IPv6 Addresses or some dot separated string with valid
characters.

Maybe I didn't understand how the bug appeared. Or was it something like:
{{{
# wrong
ALLOWED_HOSTS="127.0.0.1"
}}}
vs.
{{{
ALLOWED_HOSTS=["127.0.0.1",]
}}}
also, doesn't work a tuple too?

thx for any hints giving me a better understanding...

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:12>

Django

unread,

Jan 20, 2021, 12:48:57 PM1/20/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam Johnson):

The validation basically only needs to check it's an iterable of strings.

Checking that everything looks like a hostname is very complicated - we
also support wildcards. And it would probably break for someone out there
using a proxy server or middleware that adds something not-quite-host-
looking in the Host header.

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:13>

Django

unread,

Jan 20, 2021, 3:25:37 PM1/20/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Marius Räsener):

ok, so here's what I came up with for the check:
{{{
def check_allowedhosts(ALLOWED_HOSTS):
if isinstance(ALLOWED_HOSTS,(list,tuple)):
return all(isinstance(element,str) for element in ALLOWED_HOSTS)
else:
return False
}}}
sadly in this case `all([])` returns True so the if else statement is
needed I think ... but of course I'm happy to get hints for a better
implementation.

Besides, where would I put the check and the test?

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:14>

Django

unread,

Jan 20, 2021, 3:47:23 PM1/20/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam Johnson):

See the PR linked in comment #1 for an intial implementation.

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:15>

Django

unread,

Jan 21, 2021, 4:08:32 AM1/21/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Marius Räsener):

I didn't see the PR earlier, thank you for pointing to it.

Alright, so I could recreate or copy the PR, but there are things still
open to discuss. Can someone point me in the right direction what Tim
Graham is referring to?
Like how could this be more helpful than a system check - maybe something
that's hooked into a test run?

(for a better understanding, I'm trying since yesterday my luck with "easy
picking" issues to contribute something to Django - so please keep in mind
I'm a beginner.)

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:16>

Django

unread,

Jan 21, 2021, 1:08:35 PM1/21/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam Johnson):

Personally I think a system check would be fine. We point users to run
them during deployment in the deplyoment checklist:
https://docs.djangoproject.com/en/3.1/howto/deployment/checklist/

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:17>

Django

unread,

Jan 21, 2021, 4:10:32 PM1/21/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Marius Räsener):

well, having a app running in production for several years now I never
knew this exists :P
but, whatever - that's my responsibility.
Anyways, I don't have a strong opinion I just have an interest in closing
this issue since I want to be some kind of contributor and head to the
next issue.

So, what needs to be done to merge this and close the issue?

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:18>

Django

unread,

Jan 22, 2021, 12:37:47 AM1/22/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

Replying to [comment:18 Marius Räsener]:

> So, what needs to be done to merge this and close the issue?

The second step is to send PR via GitHub.

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:19>

Django

unread,

Jan 22, 2021, 7:25:14 AM1/22/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Marius Räsener):

Alright, here is my PR :) https://github.com/django/django/pull/13927

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:20>

Django

unread,

Jan 23, 2021, 8:40:59 AM1/23/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Marius Räsener):

Should I ping this every ~24 hrs or what could be a good strategy?

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:21>

Django

unread,

Jan 23, 2021, 9:24:58 AM1/23/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

Please refer to
[https://docs.djangoproject.com/en/3.1/internals/contributing/triaging-
tickets/#triage-workflow the contributing documentation] to make sure your
patch appears in the review queue.

In this case you'll want to uncheck ''Patch needs improvement''

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:22>

Django

unread,

Jan 27, 2021, 12:52:20 AM1/27/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

-------------------------------------+-------------------------------------
Reporter: rafis | Owner: Marius
| Räsener

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Marius Räsener):

* cc: Marius Räsener (added)
* needs_better_patch: 1 => 0
* owner: Octavio Peri => Marius Räsener

Comment:

I hope that’s the right thing to do...

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:23>

Django

unread,

Jan 28, 2021, 11:50:08 AM1/28/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Adam Johnson):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:24>

Django

unread,

Mar 19, 2021, 3:07:35 AM3/19/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS

-------------------------------------+-------------------------------------
Reporter: rafis | Owner: Marius
| Räsener
Type: New feature | Status: assigned

Component: Core (System | Version: dev

checks) |
Severity: Normal | Resolution:
Keywords: allowed_hosts | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by AdamDonna):

I've recreated the PR and incorporated the feedback from the reviewers.
It's ready for another review at
https://github.com/django/django/pull/14149.
Functionally i think it's good; the docs will need some review (especially
if they needed to be included in other places)

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:25>

Django

unread,

Mar 24, 2021, 4:24:50 AM3/24/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS
-------------------------------------+-------------------------------------
Reporter: rafis | Owner: Marius
| Räsener
Type: New feature | Status: assigned

Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: allowed_hosts | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* needs_better_patch: 1 => 0

* component: Core (System checks) => Core (Other)
* stage: Accepted => Ready for checkin

Comment:

#26709

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:26>

Django

unread,

Mar 24, 2021, 6:12:42 AM3/24/21

to django-...@googlegroups.com

#29606: Validate the type of ALLOWED_HOSTS
-------------------------------------+-------------------------------------
Reporter: rafis | Owner: Marius
| Räsener

Type: New feature | Status: closed

Component: Core (Other) | Version: dev

Severity: Normal | Resolution: fixed

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"cdd0b213a825fcfe90ae93dcc554fba8c1e5ff5d" cdd0b21]:
{{{
#!CommitTicketReference repository=""
revision="cdd0b213a825fcfe90ae93dcc554fba8c1e5ff5d"
Fixed #29606 -- Added type check for ALLOWED_HOSTS setting.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29606#comment:27>

Reply all

Reply to author

Forward