Opened 13 months ago

Closed 13 months ago

Last modified 12 months ago

#34455 closed Bug (fixed)

i18n_patterns() not respecting prefix_default_language=False

Reported by: Oussama Jarrousse Owned by: Sarah Boyce
Component: Internationalization Version: 4.2
Severity: Release blocker Keywords: internationalization, i18n, prefix_default_language
Cc: sergioisidoro, Claude Paroz, Mohit Singh Sinsniwal Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Oussama Jarrousse)

In my django project urls.py file I have the following setup:

from django.conf.urls.i18n import i18n_patterns
from django.contrib import admin
from django.urls import include
from django.urls import path

urlpatterns = []

# as an example... include the admin.site.urls 
urlpatterns += i18n_patterns(
    path("admin/", admin.site.urls), prefix_default_language=False
)

In versions Django==4.1.7 (or prior), I was able to navigating to /admin/ without having to add the language prefix.
Django==4.2.0, navigating to /admin/ will cause a HTTP 302 and only /en/admin/ works... although prefix_default_language=False is explicitly defined.

This change broke my API upon backend packages upgrade from 4.1.7 to 4.2.0

Change History (26)

comment:1 by Oussama Jarrousse, 13 months ago

Description: modified (diff)

comment:2 by Oussama Jarrousse, 13 months ago

Description: modified (diff)

comment:3 by Mariusz Felisiak, 13 months ago

Cc: sergioisidoro added
Resolution: needsinfo
Status: newclosed

Thanks for the ticket, however I'm not able to reproduce this issue. Can you provide a small sample project that reproduces this? (it seems to be related with 94e7f471c4edef845a4fe5e3160132997b4cca81.)

comment:4 by Oussama Jarrousse, 13 months ago

I will provide the project shortly on github...
In the meanwhile, I assume you were not able to reproduce the issue because you did not include: django.middleware.locale.LocaleMiddleware

here is MIDDLEWARE list in settings.py

MIDDLEWARE = [
    "django.middleware.security.SecurityMiddleware",
    "django.contrib.sessions.middleware.SessionMiddleware",
    "django.middleware.locale.LocaleMiddleware", # This line is important
    "django.middleware.common.CommonMiddleware",
    "django.middleware.csrf.CsrfViewMiddleware",
    "django.contrib.auth.middleware.AuthenticationMiddleware",
    "django.contrib.messages.middleware.MessageMiddleware",
    "django.middleware.clickjacking.XFrameOptionsMiddleware",
]

comment:5 by Oussama Jarrousse, 13 months ago

Resolution: needsinfo
Status: closednew

I prepared a simple (pytest) test that navigates to a path that should return HTTP status_code == 200.
I prepare a tox.ini file that runs pytest in two different Django environments once with Django==4.1.7 and another with Django==4.2.0
Tox will run the test twice consecutively. The first will pass. the second will fail.

https://github.com/oussjarrousse/djangoproject-ticket-34455

comment:6 by Mariusz Felisiak, 13 months ago

Cc: Claude Paroz added
Component: Core (URLs)Internationalization
Triage Stage: UnreviewedAccepted

comment:7 by Mohit Singh Sinsniwal, 13 months ago

Owner: changed from nobody to Mohit Singh Sinsniwal
Status: newassigned

I want to work on this bug, I think the real problem is in getting the language from request. Sending the pull request.

in reply to:  6 comment:8 by Mohit Singh Sinsniwal, 13 months ago

I have created a pull request. Can you please review it? https://github.com/django/django/pull/16727

Replying to Mariusz Felisiak:

Oussama, thanks!

Regression in 94e7f471c4edef845a4fe5e3160132997b4cca81.
Reproduced at 0e1aae7a5f51408b73c5a29e18bd1803dd030930.

comment:9 by Mohit Singh Sinsniwal, 13 months ago

Has patch: set

comment:10 by Mariusz Felisiak, 13 months ago

Has patch: unset

I want to work on this bug, I think the real problem is in getting the language from request. Sending the pull request.

I don't think think there is an issue. I'd rather suspect this line.

comment:11 by Mohit Singh Sinsniwal, 13 months ago

Resolution: worksforme
Status: assignedclosed

Unable to replicate the bug. For me, it works for both version 4.2 and 4.1.7.
I used LocaleMiddleware

in reply to:  11 ; comment:12 by Mariusz Felisiak, 13 months ago

Resolution: worksforme
Status: closednew

Replying to Mohit Singh Sinsniwal:

Unable to replicate the bug. For me, it works for both version 4.2 and 4.1.7.
I used LocaleMiddleware

Please don't close already accepted tickets. I'm still able to reproduce the issue.

in reply to:  11 comment:13 by Oussama Jarrousse, 13 months ago

Replying to Mohit Singh Sinsniwal:

Unable to replicate the bug. For me, it works for both version 4.2 and 4.1.7.
I used LocaleMiddleware

here is a project to replicate the issue... it uses tox to setup two different environments and run a simple test in each environment.

https://github.com/oussjarrousse/djangoproject-ticket-34455

comment:14 by Mariusz Felisiak, 13 months ago

Oussama, thanks, would you like to prepare a patch?

in reply to:  14 comment:15 by Oussama Jarrousse, 13 months ago

Replying to Mariusz Felisiak:

Oussama, thanks, would you like to prepare a patch?

In theory, I would love to.
However, I am not familiar enough with the core source code.

in reply to:  12 comment:16 by Mohit Singh Sinsniwal, 13 months ago

Cc: Mohit Singh Sinsniwal added
Status: newassigned

Replying to Mariusz Felisiak:

Replying to Mohit Singh Sinsniwal:

Unable to replicate the bug. For me, it works for both version 4.2 and 4.1.7.
I used LocaleMiddleware

Please don't close already accepted tickets. I'm still able to reproduce the issue.

Mariusz, sorry for closing it, I went on a different track while solving the issue, and now I can replicate.
I need your help in understanding the middleware. Locale class, what should be done with /admin/login/?next=/admin ?

  1. When /admin/login/?next=/admin is requested, it calls get_fallback_lanuage and redirects afterward to /en/admin/login/?next=/en/admin/

get_faalback_language is taking the prefixed language. If we dont want that, then we can update the process_request function:
OLD:

def process_request(self, request):
        urlconf = getattr(request, "urlconf", settings.ROOT_URLCONF)
        i18n_patterns_used, _ = is_language_prefix_patterns_used(urlconf)
        language = translation.get_language_from_request(
            request, check_path=i18n_patterns_used
        )
        if not language:
            language = self.get_fallback_language(request)
        translation.activate(language)
        request.LANGUAGE_CODE = translation.get_language()


New:

    def process_request(self, request):
        urlconf = getattr(request, "urlconf", settings.ROOT_URLCONF)
        (
            i18n_patterns_used,
            prefixed_default_language,
        ) = is_language_prefix_patterns_used(urlconf)
        language = translation.get_language_from_request(
            request, check_path=i18n_patterns_used
        )
        language_from_path = translation.get_language_from_path(request.path_info)
        if (
            not language_from_path
            and i18n_patterns_used
            and prefixed_default_language
        ):
            language = settings.LANGUAGE_CODE
        translation.activate(language)
        request.LANGUAGE_CODE = translation.get_language()

I want to work on this issue and need your suggestion if I am on right track.

Last edited 13 months ago by Mohit Singh Sinsniwal (previous) (diff)

comment:17 by Sarah Boyce, 13 months ago

Hi Mohit 👋 sorry I started to have a look and should have said so on the ticket

You can see here where I think the issue could be solved (doesn't mean it's where it should be https://github.com/django/django/pull/16735).
I think it's something around the whole en-us falling back to en and then that is different to the setting value for LANGUAGE_CODE 🤔

In general how I would approach a ticket is to write a regression test first and then see if you can get that passing without breaking things in other places

Last edited 13 months ago by Sarah Boyce (previous) (diff)

comment:18 by Sarah Boyce, 13 months ago

Has patch: set
Owner: changed from Mohit Singh Sinsniwal to Sarah Boyce

comment:19 by Mariusz Felisiak, 13 months ago

I think it's something around the whole en-us falling back to en and then that is different to the setting value for LANGUAGE_CODE 🤔

Brilliant! Thanks, that's what I've been missing for the whole time.

in reply to:  17 comment:20 by Mohit Singh Sinsniwal, 13 months ago

Replying to Sarah Boyce:

Hi Mohit 👋 sorry I started to have a look and should have said so on the ticket

You can see here where I think the issue could be solved (doesn't mean it's where it should be https://github.com/django/django/pull/16735).
I think it's something around the whole en-us falling back to en and then that is different to the setting value for LANGUAGE_CODE 🤔

In general how I would approach a ticket is to write a regression test first and then see if you can get that passing without breaking things in other places

Hi Sarah,
Thank you for solving the issue.
No problem, I understand that it would have taken me a lot of time to solve as this was my first ticket on djangoproject. Thank you for your guidance. I will try to write tests first in the future. Also, thank you to Mariusz for being patient with me and bearing with my absurd mistakes.

comment:21 by Mariusz Felisiak, 13 months ago

Triage Stage: AcceptedReady for checkin

comment:22 by Mariusz Felisiak <felisiak.mariusz@…>, 13 months ago

Resolution: fixed
Status: assignedclosed

In 3b47283:

Fixed #34455 -- Restored i18n_patterns() respect of prefix_default_language argument when fallback language is used.

Regression in 94e7f471c4edef845a4fe5e3160132997b4cca81.

Thanks Oussama Jarrousse for the report.

comment:23 by Mariusz Felisiak <felisiak.mariusz@…>, 13 months ago

In facc153:

[4.2.x] Fixed #34455 -- Restored i18n_patterns() respect of prefix_default_language argument when fallback language is used.

Regression in 94e7f471c4edef845a4fe5e3160132997b4cca81.

Thanks Oussama Jarrousse for the report.

Backport of 3b4728310a7a64f8fcc548163b0aa5f98a5c78f5 from main

comment:24 by ab, 12 months ago

Dear all,

Thank you for the great work on this issue. Unfortunately, I think it is not completely solved. We upgraded django 4.1.3 to django 4.2 and got a similar problem.

The patch in this thread is good to find the fallback language but is still no good if LANGUAGE_CODE is not 'en' AND the urls are translated.

The i18n_patterns decorated patterns with prefix_default_language=False, and LANGUAGE_CODE = 'fr' return 404. Actually, if LANGUAGE_CODE is not 'en'.
I am tracing the error, it seems to come from get_language_from_path that receives exposition (the french translation for exhibition instead of a language).
Consequently it returns False and get_language_from_request also (I shouldn't have to use cookies or HTTP headers, right?).

This patch does the trick (django/utils/translation/trans_real.py:548), replace None by settings.LANGUAGE_CODE:

def get_language_from_path(path, strict=False):
    """
    Return the language code if there's a valid language code found in `path`.

    If `strict` is False (the default), look for a country-specific variant
    when neither the language code nor its generic variant is found.
    """
    regex_match = language_code_prefix_re.match(path)
    if not regex_match:
        return None
    lang_code = regex_match[1]
    try:
        return get_supported_language_variant(lang_code, strict=strict)
    except LookupError:
        return settings.LANGUAGE_CODE

This post is not really useful as it is: it does not provide a correct bug report but it might be better now than before 4.2.1 is released.
I am willing to create a real bug report and to work on it, let me know what you think.

Best,
Anthony

comment:25 by Mariusz Felisiak, 12 months ago

Anthony, if you believe there is an issue in Django, then please create a new ticket in Trac and follow our bug reporting guidelines. Thanks.

comment:26 by ab, 12 months ago

Thank you Mariusz, here is the bug report: https://code.djangoproject.com/ticket/34515

Note: See TracTickets for help on using tickets.
Back to Top