#17734 closed Bug (fixed)
302 redirect to standard language for non-i18n URL's when returning 404.
Reported by: | Owned by: | Jannis Leidel | |
---|---|---|---|
Component: | Internationalization | Version: | dev |
Severity: | Release blocker | Keywords: | urlconf, i18n |
Cc: | Orne Brocaar, Jannis Leidel | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I'm using [Tastypie] to supply an API for my project. This API should be a language independent URI, while some other applications have language dependent URI's. This leaves me with the following URLConf:
from django.utils.translation import ugettext_lazy as _ from django.conf.urls.defaults import patterns, include, url from django.conf.urls.i18n import i18n_patterns urlpatterns = patterns('', url(r'^api/', include(v1_api.urls)), ) urlpatterns += i18n_patterns('', url(_(r'^something/'), include('something.urls')), )
When I raise a 404
in the API, the LocaleMiddleware
detects this and tries to redirect to /en/api/[..]
. I think this happens because is_language_prefix_patterns_used
always returns True
because it loops through all the URL's in the URLConf and finds a locale URL. However, the URL for the API is not an Locale URL.
[Tastypie]: https://github.com/toastdriven/django-tastypie
Attachments (1)
Change History (11)
comment:1 by , 13 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 13 years ago
When implementing this feature during the DjangoCon sprints, jezdez and I discussed this issue / side-effect. From what I can remember, we discussed different approaches to fix this side-effect. In the end, we decided to leave it as it was (I forgot the exact reason, but probably since the solutions we found made it too complex).
The most simple approach would be (I think, but feedback is welcome) to use the same logic as the CommonMiddleware
uses for the APPEND_SLASH
logic. Before redirecting the user to the language-prefixed URL, it will then first test if the prefixed URL version is resolvable, if not, it doesn't redirect the user. This would then mean that the LocaleMiddleware
would share some logic with the CommonMiddleware
because it should know about the APPEND_SLASH
setting. If this setting is set to True
, it should test for both /en/something
and /en/something/
. I'm not sure if that is desirable...
What happens now when the user requests /something
, the LocaleMiddleware
will redirect the user (assuming it detects the language as en) to /en/something
, then the CommonMiddleware
will redirect the user to /en/something/
.
What do you guys think?
comment:3 by , 13 years ago
The problem isn't about the /something pattern. It is about treating the /api pattern as locale aware while it isn't one. I might be missing the point in comment:2, but I think it is about a different issue?
comment:4 by , 13 years ago
Yes, but what happens now, when you request /api/raising/404/
, it is redirecting the user to /en/api/raising/404/
(assuming it detects the language as en). If it tests before redirecting the user if /en/api/raising/404/
is resolvable, it would find out that this would raise a 404 as well and thus would not redirect the user.
comment:5 by , 13 years ago
Owner: | changed from | to
---|
comment:6 by , 13 years ago
Nah, @akaariai, @brocaar is right, the LocaleMiddleware
does the redirection when it detects that a i18n enabled URL is used so we better check if that URL exists before redirecting to the URL path -- in that sense pretty much what APPEND_SLASH does, except we need to check if the full path matches instead of just appended an /
.
@brocaar I think this might change a bunch of existing tests, am I right?
comment:7 by , 13 years ago
Has patch: | set |
---|
comment:9 by , 11 years ago
I'm not sure if this should be the default behavior, but I feel that process_response()
should also check:
- If the non-language path is valid
- If the non-language path resolves to the same view as the language path
If the non-language path is valid, the redirect should only be performed if the redirect goes to the same view.
I'm running into the same problem as the original poster when my URL configuration contains a "catch-all" regex, as is required by Django CMS:
from django.conf.urls.defaults import patterns, include, url from django.conf.urls.i18n import i18n_patterns from cms.views import details urlpatterns = patterns('', url(r'^api/', include(v1_api.urls)), ) urlpatterns += i18n_patterns('', url(r'^something/', include('something.urls')), url(r'^(?P<slug>[0-9A-Za-z-_.//]+)/$', details, name='pages-details-by-slug') )
So, a request to an API endpoint (for example "/api/endpoint/NOT-FOUND-ID/") that returns a 404, will be redirected to "/en/api/endpoint/NOT-FOUND-ID/" because is_valid()
will return true for the language path because it matches the regex for
pages-details-by-slug
.
comment:10 by , 6 years ago
For what it's worth (this is a old ticket), I ran into the same problem as @geoffhing, where I had a API which on 404 would redirect to a language-prefixed URL because I was using a catch-all url pattern:
urlpatterns = [ url(r'^api/', include(api.urls)), ] urlpatterns += i18n_patterns( url(r'^something/', include('something.urls')), url(r'^', include(wagtail_urls)), )
I fixed it by extending the default Django LocaleMiddleware:
from django.middleware.locale import LocaleMiddleware as DjangoLocaleMiddleware class LocaleMiddleware(DjangoLocaleMiddleware): def process_response(self, request, response): if request.path.startswith('/api/'): # Prevent `/api/` URLs from redirecting to language-prefixed URLs, # because the API client will get a 302 redirect response instead # of 404, which is not what we want. return response return super().process_response(request, response)
This was the best solution I could come up with. I preferrably don't override default django classes but in this case I couldn't think of any other solution. It works quite fine though.
Marking as a release blocker since this is a significant bug in a new feature.