Fix erroneous empty pattern matching when PARENT_HOST is defined

[LeMinaw]

Rationale for this PR

The current regex host pattern matching does not take PARENT_HOST into account.

This leads to very misleading behavior of the r'' pattern, in particular when PARENT_HOST contains a . (eg. is set to example.com, see test case below).

This is basically the problem hilighted in PR https://github.com/jazzband/django-hosts/pull/121, and probably the cause of issues https://github.com/jazzband/django-hosts/issues/77, https://github.com/jazzband/django-hosts/issues/97.

As this issue were not detected by the test suite, I added a testcase that fails on master ATM. For reference, this is what the new test outputs without the fix (of course, it passes when the fix is applied):

    @override_settings(
        ALLOWED_HOSTS=['example.com'],
        ROOT_HOSTCONF='tests.hosts.blank_wildcard',
        PARENT_HOST='example.com',
        DEFAULT_HOST='root')
    def test_request_blank_urlconf_module(self):
        rf = RequestFactory(HTTP_HOST='example.com')
        request = rf.get('/')
        middleware = HostsRequestMiddleware()
        middleware.process_request(request)

>       self.assertEqual(request.urlconf, 'tests.urls.root')
E       AssertionError: 'tests.urls.simple' != 'tests.urls.root'

tests\test_middleware.py:52: AssertionError

Here are the host patterns for reference.

See how the middleware incorrectly sets example.com/'s urlconf to tests.urls.simple, because the first r'' pattern is not matched.

It is worth noting if PARENT_HOST does not contains a ., the issue vanishes. This leads to unexpected behavior when switching from, say, localhost to example.com.

The fix

It should be quite straightforward to understand. The idea is to suffix the compiled regex pattern with PARENT_HOST if it exists, beffore adding the delimiters identifiers to it (currently . or end of string). This way, r'', r'(|www)' and the like match consistently.

Additionnaly, this PR adds : to the set of host delimiters (like mentionned in issue https://github.com/jazzband/django-hosts/issues/85), to work nicely with HOST_PORT and dev servers without any need to tweak the host patterns.

[codecov[bot]]

Codecov Report

Merging #128 (743e330) into master (7f858cc) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #128   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines          398       400    +2     
  Branches        67        67           
=========================================
+ Hits           398       400    +2     

Impacted Files	Coverage Δ
django_hosts/defaults.py	100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[LeMinaw]

It would be nice to have feedback from the maintainers / users of this lib (don't be shy!) as it alters core functionnality.

@jezdez, it seems to me you're rather busy and don't have much time for Jazzband stuff at the moment, but as the main contributor of this repository, any thoughs?

If there's room for improvement just let me know.

[ddabble]

@LeMinaw I'm not familiar with the "notification culture" among the maintainers of this repo, but it might be better to @ the current project leads (found on the project page), which is currently @timgraham 😊

[timgraham]

I only had an interest in this project because I was paid as a Django fellow to be a maintainer of djangoproject.com which uses django-hosts. I would like to step down if we can find a new leader.

[ddabble]

[Slightly off-topic] @timgraham Ah, I see 😅 Do you know of any appropriate places for announcing that we're looking for a new project lead? (Also, do you know if that would be how things are usually done, or does Jazzband purely rely on people taking the initiative by themselves..?)

[timgraham]

We can discuss at https://github.com/jazzband/django-hosts/issues/144.

[dot-legal]

Wanted to bump this one up as I'm encountering it now. Seems to me like a pretty critical issue because the assumption throughout is that the r'' pattern in virtually every tutorial would be matching the no subdomain case. And, as pointed out, it's compounded by the fact that it behaves differently depending on localhost or a standard domain.

Originally my hope was to do a catchall to show an error to all erroneous subdomains, but the root domain is being caught. Took a long while to understand why because of the odd behavior. host(r'(\w+)', 'homepage.urls_redirect', name='redirect'),

[sltong]

I ran into the same issue as well. It seems that the FQDN and the host/URL parameters sections of the documentation support the idea that respecting PARENT_HOST is the expected behavior.

[AndreaGiardini]

Just faced the same issue here. Installing the fork from @LeMinaw worked like a charm. Can we please merge this?

Maybe @hramezani can help?

[hramezani]

Thanks @AndreaGiardini for mentioning me. Unfortunately, I am not a lead in this project and just helping them sometimes. Maybe @timgraham can help

[LeMinaw]

Tim was already mentionned on this issue, and he shared with us his wish to step down (see https://github.com/jazzband/django-hosts/issues/144). I don't see how pinging him again will help.

It's been a year and a half since I made this PR. I this at this point, we could:

• Close it and consider this project archived / read only
• Merge it and release a new major version, as the few (3) comments here were positive and no one complained

[AndreaGiardini]

It would be great if somebody from Jazzband could pick up the open PRs and publish a new release. 🙏

[hramezani]

FYI, I asked on the issue to be a lead for this project Let's wait, maybe I can help here.

[hramezani]

Ok, Now I am a lead on this project. I will take a look at this PR probably later this week and will bake a release

[hramezani]

@LeMinaw Could you please add a changelog entry in https://github.com/jazzband/django-hosts/blob/master/docs/changelog.rst

[hramezani]

Also, could you please add a test for : as well or point me to the existing test?

[hramezani]

It's available in django-hosts 5.2