Broken link email sent on redirect by i18n_patterns

[Aymeric Augustin]

With the following settings:

MIDDLEWARE_CLASSES = {
        # ...,
        'django.middleware.locale.LocaleMiddleware',
        'django.middleware.common.CommonMiddleware',
        # ...,
}
SEND_BROKEN_LINKS_EMAIL = True

when a redirect to an i18n'd URL occurs, a broken link email is sent.

I'm attaching a failing test case.

This may be the expected behavior, but it's hard to diagnose. I started receiving broken link emails for '/', with various referrers and UAs. But going to that URL worked and didn't send an email, because there's no referrer when you go straight to the URL.

The solution might be documenting that CommonMiddleware must come before LocaleMiddleware. The commit logs say that they used to be in that order, but that we swapped them because APPEND_SLASH didn't work (I haven't re-verified this yet). And then I started receiving the broken link emails.

[Aymeric Augustin]

After some more tests, here's what I've found.

Currently, ​the documentation says that LocaleMiddleware should come before CommonMiddleware. It means that during response processing, CommonMiddleware.process_response runs first, and it sends the broken link email before LocaleMiddleware.process_response gets a chance to redirect to the expected URL. (That's very similar to #1859.)

Putting CommonMiddleware before LocaleMiddleware solves the problem with broken link emails, but creates other bugs; notably, missing trailing slashes aren't appended to URLs when the prefix is a non-default language.

These results can be reproduced easily with:

• a URLconf with i18n_patterns, such as the following:

  urlpatterns = i18n_patterns('views',
      url(r'^contact/$', 'contact'),
  )
  

• APPEND_SLASH = True
• LANGUAGE_CODE = 'en'
• LANGUAGES = (('en', _('English')), ('fr', _('French')))

So, in the current state of things, i18n_patterns and SEND_BROKEN_LINK_EMAILS are incompatible.

---

The best solution I can think of is to move the broken link emails feature to its own middleware, say, BrokenLinksMiddleware, that should come before LocaleMiddleware.

Besides, 404 errors reporting by email sounds a bit archaic these days; does it really belong in CommonMiddleware?

It's a bit difficult to create a clean deprecation path for this change. I think CommonMiddleware.process_response could raise warnings if SEND_BROKEN_LINK_EMAILS is True but BrokenLinksMiddleware isn't in MIDDLEWARE_CLASSES. In this case, during the deprecation period, it would call BrokenLinksMiddleware.process_response.

[Aymeric Augustin]

​https://github.com/django/django/pull/652

[Claude Paroz]

Looks good, thanks.

[Aymeric Augustin <aymeric.augustin@…>]

In 50a985b09b439a0d52aad8694d377a3483cb02e1:

Fixed #19099 -- Split broken link emails out of common middleware.