LocaleMiddleware unexpectedly causes messages to be consumed under certain circumstances

[Sylvain Fankhauser]

Adding a message (with the messages contrib module) to the request and then redirecting the user to an internationalized route without the language prefix and a custom 404 template that consumes messages will consume messages from the request, even though no 404 response is sent back to the client.

I have set up a minimal reproduction project here: ​https://github.com/sephii/django-localemiddleware-bug

To reproduce this problem, you’ll need:

1. An internationalized route (eg. urlpatterns += i18n_patterns(path("foo/", views.foo)))
2. A view that adds a message to the request (eg. messages.success(request, "Hello world")) and then redirects to the internationalized route without a language prefix
3. A 404.html template that consumes the request messages (eg. {% for message in messages %}{{ message }}{% endfor %})

Visiting the view defined in point 2 will result in a 302 to /foo/, which, because of the 404.html template, will consume the message added by the view in point 2 before redirecting to /en/foo/ (which won’t have any message left to consume).

The bug is not in LocaleMiddleware per se, but is related to the way LocaleMiddleware works: by catching any 404 and checking if it matches an internationalized route, and if so redirecting to the correct route, but at this point the 404 response has been rendered.

Ideally the only response Django should create is the redirect, and no 404 should be rendered.

[Natalia Bidart]

Hello Sylvain Fankhauser!

Thank you for your interest in making Django better. I took a close look at this report, I cloned the reproducer repo and followed your steps. I do see what you are describing here, but I also noted a few cases that raise concerns:

• In my experience, 404 templates should ideally be as static as possible. Consuming/showing the messages from the request feels like a code smell.
• Redirecting to a hard-coded URL is an anti-pattern; ideally, the code would use proper calls to reverse.
• The URL /foobar/ is clearly a 404, so manually redirecting to this URL is another unexpected aspect of the code. If hard-coding is essential, why not redirecting to /<lang>/foobar/?

I made this slight change to the given code:

testdjango/urls.py

@@ -17 +17 @@
 from django.contrib import messages
 from django.http import HttpResponse
 from django.shortcuts import redirect
-from django.urls import path
+from django.urls import path, reverse
 
 
 def home(request):
     messages.error(request, "Hello world")
-    return redirect("/foobar/")
+    return redirect(reverse("foobar"))
 
 
 def foobar(request):
@@ -35 +35 @@
 ]
 
 urlpatterns += i18n_patterns(
-    path("foobar/", foobar),
+    path("foobar/", foobar, name="foobar"),
 )

and messages are not consumed any longer, because there is no 404 being raised anywhere in the call chain. Honestly I don't see where Django is at fault here, the way the i18n URLs are resolved involves raising a Resolver404 exception which triggers the creation of a HttpResponseNotFound instance (which needs the template rendered to be created).

I'll closing as invalid but if you have more information or if you disagree with the resolution, please reach out to the ​Django forum.

[Sylvain Fankhauser]

Hello Natalia,

Thank you for taking the time to review the issue.

The reproduction code is indeed not using reverse on purpose, to mimic the behaviour of redirect_to_login (which I’m actually using in my code, instead of the redirect(reverse("foobar")) I put in the reproduction code). redirect_to_login redirects to settings.LOGIN_URL, which doesn’t have any language prefix and causes the initial 404. Perhaps the proper fix in my case is to use a named URL for settings.LOGIN_URL instead of the default, so that the language prefix is included in the URL and no 404 is rendered.

I’m fine with keeping this as invalid as I don’t really see what else could be done (except maybe change the default settings.LOGIN_URL to use a named URL?).