BrokenLinkEmailsMiddleware shouldn't report 404s when URL = Referer + "/"

[Jon Ribbens]

CommonMiddleware with APPEND_SLASH redirects "/foo" to "/foo/".

BrokenLinkEmailsMiddleware goes "omg /foo/ doesn't exist and it was referred to by /foo, that's an INTERNAL BROKEN LINK!"

This means the "don't alert about links with no referer" check basically doesn't work at all.

If APPEND_SLASH is set and the referer is the URL with a trailing '/' removed then BrokenLinkEmailsMiddleware shouldn't send an email.

[Emre Yılmaz]

APPEND_SLASH should redirect if only /foo/ is valid a view. (check CommonMiddleware.should_redirect_with_slash)

Can you provide a test case?

[Aymeric Augustin]

I believe you can still hit this case if the URL resolves, but the view raises Http404 -- for example because it uses get_object_or_404.

[Jon Ribbens]

Indeed. Consider for example if you are using django-cms, where almost any URL will end up matching the view that then goes on to see if a page has been published with that name - so urlresolvers will basically always say "yes that URL might exist".

[Chris Maiden]

Hello and Happy New Year!

I've made an attempt at creating a failing test against 1.8.x as a first step - ​https://github.com/django/django/compare/stable/1.8.x...matason:ticket_25971?expand=1

Does it look like it captures the bug correctly?

[Jon Ribbens]

As far as my limited understanding of the test framework goes, it looks probably correct to me ;-)

[Chris Maiden]

Thanks jribbens, I'll see if I can come up with a pull request that fixes the issue!

[Tim Graham]

Unless this is a regression in Django 1.8, please send the pull request against master. The ticket "Version" only indicates what version the issue was reported in.

[Chris Maiden]

Thanks timgraham, noted.

[HARI KRISHNA KANCHI]

Sent a pull request, along with the patch. ​https://github.com/django/django/pull/6008. Can anybody check this pull request? And let me know if there are any improvements.

[Tim Graham]

The test doesn't seem to act as a regression test as it passes without the fix.

[HARI KRISHNA KANCHI]

Fixed the test case now.

[Tim Graham]

I'm not sure if the added logic is correct. Should it consider settings.APPEND_SLASH as mentioned in the original report?

[HARI KRISHNA KANCHI]

Yah, it should consider settings.APPEND_SLASH. I will change the code accordingly.

[HARI KRISHNA KANCHI]

If settings.APPEND_SLASH is unset and referrer url equal to requested url without trailing slash, then it should send mail right?

[Tim Graham]

Sounds correct to me.

[HARI KRISHNA KANCHI]

Hey TIm, Changed the code accordingly. Can you look into once. Thanks.

[Tim Graham <timograham@…>]

In 7467049:

Fixed #25971 -- Made BrokenLinkEmailsMiddleware ignore APPEND_SLASH redirects.

If APPEND_SLASH=True and the referer is the URL without a trailing '/', then
BrokenLinkEmailsMiddleware shouldn't send an email.