Opened 2 years ago

Last modified 21 months ago

#29606 assigned New feature

Validate the type of ALLOWED_HOSTS

Reported by: rafis Owned by: Tomasz Kluczkowski
Component: Core (System checks) Version: master
Severity: Normal Keywords: allowed_hosts
Cc: Adam (Chainz) Johnson, Herbert Fortes Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Python has soft behavior for conducting iteration process over lists and over strings making them look the same:

    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,

    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

    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.

Change History (6)

comment:1 Changed 2 years ago by Tim Graham

Easy pickings: unset
Has patch: set
Patch needs improvement: set
Summary: Validate the type of ALLOWED_HOSTS to prevent possible issues and wasted timeValidate the type of ALLOWED_HOSTS
Triage Stage: UnreviewedAccepted
Version: 2.0master

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.

comment:2 Changed 2 years ago by Adam (Chainz) Johnson

Cc: Adam (Chainz) Johnson added

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.

comment:3 Changed 2 years ago 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.

comment:4 Changed 2 years ago by Herbert Fortes

Cc: Herbert Fortes added

comment:5 Changed 21 months ago 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 settings list, at least on most of them.
Best regards, Nick

comment:6 Changed 21 months ago by Tomasz Kluczkowski

Owner: changed from nobody to Tomasz Kluczkowski
Status: newassigned
Note: See TracTickets for help on using tickets.
Back to Top